Message ID | 20221130153204.2085591-1-kajetan.puchalski@arm.com |
---|---|
Headers | show |
Series | cpuidle: teo: Introduce util-awareness | expand |
On Wed, Nov 30, 2022 at 03:32:02PM +0000, Kajetan Puchalski wrote: Hi Rafael, As it's been a while since the last email I wanted to bump this thread and ask what you think about the last changes. Additionally, I got some emails from the kernel test robot and noticed that sched_cpu_util is contingent on CONFIG_SMP so in the current form there's build errors on !SMP machines. The following change should fix the problem, do you think it's all right to add? @@ -207,10 +207,17 @@ static DEFINE_PER_CPU(struct teo_cpu, teo_cpus); * @dev: Target CPU * @cpu_data: Governor CPU data for the target CPU */ +#ifdef CONFIG_SMP static void teo_get_util(struct cpuidle_device *dev, struct teo_cpu *cpu_data) { cpu_data->utilized = sched_cpu_util(dev->cpu) > cpu_data->util_threshold; } +#else +static void teo_get_util(struct cpuidle_device *dev, struct teo_cpu *cpu_data) +{ + cpu_data->utilized = false; +} +#endif Thanks in advance for your time, Kajetan > v4 -> v5: > - remove the restriction to only apply the mechanism for C1 candidate state > - clarify some code comments, fix comment style > - refactor the fast-exit path loop implementation > - move some cover letter information into the commit description > > v3 -> v4: > - remove the chunk of code skipping metrics updates when the CPU was utilized > - include new test results and more benchmarks in the cover letter > > v2 -> v3: > - add a patch adding an option to skip polling states in teo_find_shallower_state() > - only reduce the state if the candidate state is C1 and C0 is not a polling state > - add a check for polling states in the 2-states fast-exit path > - remove the ifdefs and Kconfig option > > v1 -> v2: > - rework the mechanism to reduce selected state by 1 instead of directly selecting C0 (suggested by Doug Smythies) > - add a fast-exit path for systems with 2 idle states to not waste cycles on metrics when utilized > - fix typos in comments > - include a missing header > > Kajetan Puchalski (2): > cpuidle: teo: Optionally skip polling states in teo_find_shallower_state() > cpuidle: teo: Introduce util-awareness > > drivers/cpuidle/governors/teo.c | 93 +++++++++++++++++++++++++++++++-- > 1 file changed, 89 insertions(+), 4 deletions(-) > > -- > 2.37.1 >
On Tue, Jan 3, 2023 at 3:22 PM Kajetan Puchalski <kajetan.puchalski@arm.com> wrote: > > On Wed, Nov 30, 2022 at 03:32:02PM +0000, Kajetan Puchalski wrote: > > Hi Rafael, > > As it's been a while since the last email I wanted to bump this thread > and ask what you think about the last changes. Right, I'll send my comments on the last version of the patch separately. > Additionally, I got some emails from the kernel test robot and noticed > that sched_cpu_util is contingent on CONFIG_SMP so in the current form > there's build errors on !SMP machines. > > The following change should fix the problem, do you think it's all right to add? > > @@ -207,10 +207,17 @@ static DEFINE_PER_CPU(struct teo_cpu, teo_cpus); > * @dev: Target CPU > * @cpu_data: Governor CPU data for the target CPU > */ > +#ifdef CONFIG_SMP > static void teo_get_util(struct cpuidle_device *dev, struct teo_cpu *cpu_data) > { > cpu_data->utilized = sched_cpu_util(dev->cpu) > cpu_data->util_threshold; > } > +#else > +static void teo_get_util(struct cpuidle_device *dev, struct teo_cpu *cpu_data) > +{ > + cpu_data->utilized = false; > +} > +#endif > IMV it would be better to use teo_cpu_is_utilized() that would be called to update cpu_data->utilized this way cpu_data->utilized = teo_cpu_is_utilized(dev->cpu, cpu_data->util_threshold); and define it as an empty stab in the !SMP case.