* [PATCH v1 0/4] cpuidle: governors: teo: Assorted improvements
@ 2025-11-12 16:21 Rafael J. Wysocki
2025-11-12 16:22 ` [PATCH v1 1/4] cpuidle: governors: teo: Drop incorrect target residency check Rafael J. Wysocki
` (4 more replies)
0 siblings, 5 replies; 26+ messages in thread
From: Rafael J. Wysocki @ 2025-11-12 16:21 UTC (permalink / raw)
To: Linux PM, Christian Loehle; +Cc: LKML, Reka Norman
Hi,
This is a bunch of teo cpuidle governor improvements, some of which are related
to a bug report discussed recently:
https://lore.kernel.org/linux-pm/CAEmPcwsNMNnNXuxgvHTQ93Mx-q3Oz9U57THQsU_qdcCx1m4w5g@mail.gmail.com/
The first patch fixes a bug that may cause an overly deep idle state
to be selected when the scheduler tick has been already stopped.
Patch [2/4] removes an unnecessary function argument.
Patch [3/4] makes teo_update() to use s64 as the data type for its local
variables more consistently.
The last patch reworks the governor's decay implementation to also decay
metric values lower than 8.
Thanks!
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v1 1/4] cpuidle: governors: teo: Drop incorrect target residency check
2025-11-12 16:21 [PATCH v1 0/4] cpuidle: governors: teo: Assorted improvements Rafael J. Wysocki
@ 2025-11-12 16:22 ` Rafael J. Wysocki
2025-11-13 11:32 ` Christian Loehle
2025-11-13 13:24 ` [PATCH v2 1/4] cpuidle: governors: teo: Drop misguided " Rafael J. Wysocki
2025-11-12 16:23 ` [PATCH v1 2/4] cpuidle: governors: teo: Drop redundant function parameter Rafael J. Wysocki
` (3 subsequent siblings)
4 siblings, 2 replies; 26+ messages in thread
From: Rafael J. Wysocki @ 2025-11-12 16:22 UTC (permalink / raw)
To: Linux PM, Christian Loehle; +Cc: LKML, Reka Norman
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
When the target residency of the current candidate idle state is
greater than the expected time till the closest timer (the sleep
length), it does not matter whether or not the tick has already
been stopped or if it is going to be stopped. The closest timer
will trigger anyway at its due time, so it does not make sense to
select an idle state with target residency above the sleep length.
Accordingly, drop the teo_state_ok() check done in that case and
let the governor use the teo_find_shallower_state() return value
as the new candidate idle state index.
Fixes: 21d28cd2fa5f ("cpuidle: teo: Do not call tick_nohz_get_sleep_length() upfront")
Cc: All applicable <stable@vger.kernel.org>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
drivers/cpuidle/governors/teo.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
--- a/drivers/cpuidle/governors/teo.c
+++ b/drivers/cpuidle/governors/teo.c
@@ -458,11 +458,8 @@ static int teo_select(struct cpuidle_dri
* If the closest expected timer is before the target residency of the
* candidate state, a shallower one needs to be found.
*/
- if (drv->states[idx].target_residency_ns > duration_ns) {
- i = teo_find_shallower_state(drv, dev, idx, duration_ns, false);
- if (teo_state_ok(i, drv))
- idx = i;
- }
+ if (drv->states[idx].target_residency_ns > duration_ns)
+ idx = teo_find_shallower_state(drv, dev, idx, duration_ns, false);
/*
* If the selected state's target residency is below the tick length
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v1 2/4] cpuidle: governors: teo: Drop redundant function parameter
2025-11-12 16:21 [PATCH v1 0/4] cpuidle: governors: teo: Assorted improvements Rafael J. Wysocki
2025-11-12 16:22 ` [PATCH v1 1/4] cpuidle: governors: teo: Drop incorrect target residency check Rafael J. Wysocki
@ 2025-11-12 16:23 ` Rafael J. Wysocki
2025-11-13 11:46 ` Christian Loehle
2025-11-12 16:24 ` [PATCH v1 3/4] cpuidle: governors: teo: Use s64 consistently in teo_update() Rafael J. Wysocki
` (2 subsequent siblings)
4 siblings, 1 reply; 26+ messages in thread
From: Rafael J. Wysocki @ 2025-11-12 16:23 UTC (permalink / raw)
To: Linux PM, Christian Loehle; +Cc: LKML, Reka Norman
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
The last no_poll parameter of teo_find_shallower_state() is always
false, so drop it.
No intentional functional impact.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
drivers/cpuidle/governors/teo.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
--- a/drivers/cpuidle/governors/teo.c
+++ b/drivers/cpuidle/governors/teo.c
@@ -239,17 +239,15 @@ static bool teo_state_ok(int i, struct c
* @dev: Target CPU.
* @state_idx: Index of the capping idle state.
* @duration_ns: Idle duration value to match.
- * @no_poll: Don't consider polling states.
*/
static int teo_find_shallower_state(struct cpuidle_driver *drv,
struct cpuidle_device *dev, int state_idx,
- s64 duration_ns, bool no_poll)
+ s64 duration_ns)
{
int i;
for (i = state_idx - 1; i >= 0; i--) {
- if (dev->states_usage[i].disable ||
- (no_poll && drv->states[i].flags & CPUIDLE_FLAG_POLLING))
+ if (dev->states_usage[i].disable)
continue;
state_idx = i;
@@ -459,7 +457,7 @@ static int teo_select(struct cpuidle_dri
* candidate state, a shallower one needs to be found.
*/
if (drv->states[idx].target_residency_ns > duration_ns)
- idx = teo_find_shallower_state(drv, dev, idx, duration_ns, false);
+ idx = teo_find_shallower_state(drv, dev, idx, duration_ns);
/*
* If the selected state's target residency is below the tick length
@@ -487,7 +485,7 @@ end:
*/
if (idx > idx0 &&
drv->states[idx].target_residency_ns > delta_tick)
- idx = teo_find_shallower_state(drv, dev, idx, delta_tick, false);
+ idx = teo_find_shallower_state(drv, dev, idx, delta_tick);
out_tick:
*stop_tick = false;
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v1 3/4] cpuidle: governors: teo: Use s64 consistently in teo_update()
2025-11-12 16:21 [PATCH v1 0/4] cpuidle: governors: teo: Assorted improvements Rafael J. Wysocki
2025-11-12 16:22 ` [PATCH v1 1/4] cpuidle: governors: teo: Drop incorrect target residency check Rafael J. Wysocki
2025-11-12 16:23 ` [PATCH v1 2/4] cpuidle: governors: teo: Drop redundant function parameter Rafael J. Wysocki
@ 2025-11-12 16:24 ` Rafael J. Wysocki
2025-11-13 11:48 ` Christian Loehle
2025-11-12 16:25 ` [PATCH v1 4/4] cpuidle: governors: teo: Decay metrics below DECAY_SHIFT threshold Rafael J. Wysocki
2025-11-13 15:21 ` [PATCH v1 0/4] cpuidle: governors: teo: Assorted improvements Christian Loehle
4 siblings, 1 reply; 26+ messages in thread
From: Rafael J. Wysocki @ 2025-11-12 16:24 UTC (permalink / raw)
To: Linux PM, Christian Loehle; +Cc: LKML, Reka Norman
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Two local variables in teo_update() are defined as u64, but their
values are then compared with s64 values, so it is more consistent
to use s64 as their data type.
No intentional functional impact.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
drivers/cpuidle/governors/teo.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
--- a/drivers/cpuidle/governors/teo.c
+++ b/drivers/cpuidle/governors/teo.c
@@ -157,8 +157,7 @@ static void teo_update(struct cpuidle_dr
{
struct teo_cpu *cpu_data = per_cpu_ptr(&teo_cpus, dev->cpu);
int i, idx_timer = 0, idx_duration = 0;
- s64 target_residency_ns;
- u64 measured_ns;
+ s64 target_residency_ns, measured_ns;
cpu_data->short_idles -= cpu_data->short_idles >> DECAY_SHIFT;
@@ -167,9 +166,9 @@ static void teo_update(struct cpuidle_dr
* If one of the safety nets has triggered, assume that this
* might have been a long sleep.
*/
- measured_ns = U64_MAX;
+ measured_ns = S64_MAX;
} else {
- u64 lat_ns = drv->states[dev->last_state_idx].exit_latency_ns;
+ s64 lat_ns = drv->states[dev->last_state_idx].exit_latency_ns;
measured_ns = dev->last_residency_ns;
/*
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v1 4/4] cpuidle: governors: teo: Decay metrics below DECAY_SHIFT threshold
2025-11-12 16:21 [PATCH v1 0/4] cpuidle: governors: teo: Assorted improvements Rafael J. Wysocki
` (2 preceding siblings ...)
2025-11-12 16:24 ` [PATCH v1 3/4] cpuidle: governors: teo: Use s64 consistently in teo_update() Rafael J. Wysocki
@ 2025-11-12 16:25 ` Rafael J. Wysocki
2025-11-12 17:29 ` Christian Loehle
2025-11-12 18:03 ` [PATCH v2 " Rafael J. Wysocki
2025-11-13 15:21 ` [PATCH v1 0/4] cpuidle: governors: teo: Assorted improvements Christian Loehle
4 siblings, 2 replies; 26+ messages in thread
From: Rafael J. Wysocki @ 2025-11-12 16:25 UTC (permalink / raw)
To: Linux PM, Christian Loehle; +Cc: LKML, Reka Norman
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
If a given governor metric falls below a certain value (8 for
DECAY_SHIFT equal to 3), it will not decay any more due to the
simplistic decay implementation. This may in some cases lead to
subtle inconsistencies in the governor behavior, so change the
decay implementation to take it into account and set the metric
at hand to 0 in that case.
Suggested-by: Christian Loehle <christian.loehle@arm.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
drivers/cpuidle/governors/teo.c | 20 +++++++++++++++-----
1 file changed, 15 insertions(+), 5 deletions(-)
--- a/drivers/cpuidle/governors/teo.c
+++ b/drivers/cpuidle/governors/teo.c
@@ -148,6 +148,16 @@ struct teo_cpu {
static DEFINE_PER_CPU(struct teo_cpu, teo_cpus);
+static void teo_decay(unsigned int *metric)
+{
+ unsigned int delta = *metric >> DECAY_SHIFT;
+
+ if (delta)
+ *metric -= delta;
+ else
+ *metric = 0;
+}
+
/**
* teo_update - Update CPU metrics after wakeup.
* @drv: cpuidle driver containing state data.
@@ -159,7 +169,7 @@ static void teo_update(struct cpuidle_dr
int i, idx_timer = 0, idx_duration = 0;
s64 target_residency_ns, measured_ns;
- cpu_data->short_idles -= cpu_data->short_idles >> DECAY_SHIFT;
+ teo_decay(&cpu_data->short_idles);
if (cpu_data->artificial_wakeup) {
/*
@@ -195,8 +205,8 @@ static void teo_update(struct cpuidle_dr
for (i = 0; i < drv->state_count; i++) {
struct teo_bin *bin = &cpu_data->state_bins[i];
- bin->hits -= bin->hits >> DECAY_SHIFT;
- bin->intercepts -= bin->intercepts >> DECAY_SHIFT;
+ teo_decay(&bin->hits);
+ teo_decay(&bin->intercepts);
target_residency_ns = drv->states[i].target_residency_ns;
@@ -207,7 +217,7 @@ static void teo_update(struct cpuidle_dr
}
}
- cpu_data->tick_intercepts -= cpu_data->tick_intercepts >> DECAY_SHIFT;
+ teo_decay(&cpu_data->tick_intercepts);
/*
* If the measured idle duration falls into the same bin as the sleep
* length, this is a "hit", so update the "hits" metric for that bin.
@@ -222,7 +232,7 @@ static void teo_update(struct cpuidle_dr
cpu_data->tick_intercepts += PULSE;
}
- cpu_data->total -= cpu_data->total >> DECAY_SHIFT;
+ teo_decay(&cpu_data->total);
cpu_data->total += PULSE;
}
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v1 4/4] cpuidle: governors: teo: Decay metrics below DECAY_SHIFT threshold
2025-11-12 16:25 ` [PATCH v1 4/4] cpuidle: governors: teo: Decay metrics below DECAY_SHIFT threshold Rafael J. Wysocki
@ 2025-11-12 17:29 ` Christian Loehle
2025-11-12 17:51 ` Rafael J. Wysocki
2025-11-12 18:03 ` [PATCH v2 " Rafael J. Wysocki
1 sibling, 1 reply; 26+ messages in thread
From: Christian Loehle @ 2025-11-12 17:29 UTC (permalink / raw)
To: Rafael J. Wysocki, Linux PM; +Cc: LKML, Reka Norman
On 11/12/25 16:25, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> If a given governor metric falls below a certain value (8 for
> DECAY_SHIFT equal to 3), it will not decay any more due to the
> simplistic decay implementation. This may in some cases lead to
> subtle inconsistencies in the governor behavior, so change the
> decay implementation to take it into account and set the metric
> at hand to 0 in that case.
>
> Suggested-by: Christian Loehle <christian.loehle@arm.com>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
> drivers/cpuidle/governors/teo.c | 20 +++++++++++++++-----
> 1 file changed, 15 insertions(+), 5 deletions(-)
>
> --- a/drivers/cpuidle/governors/teo.c
> +++ b/drivers/cpuidle/governors/teo.c
> @@ -148,6 +148,16 @@ struct teo_cpu {
>
> static DEFINE_PER_CPU(struct teo_cpu, teo_cpus);
>
> +static void teo_decay(unsigned int *metric)
> +{
> + unsigned int delta = *metric >> DECAY_SHIFT;
> +
> + if (delta)
> + *metric -= delta;
> + else
> + *metric = 0;
> +}
> +
> /**
> * teo_update - Update CPU metrics after wakeup.
> * @drv: cpuidle driver containing state data.
> @@ -159,7 +169,7 @@ static void teo_update(struct cpuidle_dr
> int i, idx_timer = 0, idx_duration = 0;
> s64 target_residency_ns, measured_ns;
>
> - cpu_data->short_idles -= cpu_data->short_idles >> DECAY_SHIFT;
> + teo_decay(&cpu_data->short_idles);
>
> if (cpu_data->artificial_wakeup) {
> /*
> @@ -195,8 +205,8 @@ static void teo_update(struct cpuidle_dr
> for (i = 0; i < drv->state_count; i++) {
> struct teo_bin *bin = &cpu_data->state_bins[i];
>
> - bin->hits -= bin->hits >> DECAY_SHIFT;
> - bin->intercepts -= bin->intercepts >> DECAY_SHIFT;
> + teo_decay(&bin->hits);
> + teo_decay(&bin->intercepts);
>
> target_residency_ns = drv->states[i].target_residency_ns;
>
> @@ -207,7 +217,7 @@ static void teo_update(struct cpuidle_dr
> }
> }
>
> - cpu_data->tick_intercepts -= cpu_data->tick_intercepts >> DECAY_SHIFT;
> + teo_decay(&cpu_data->tick_intercepts);
> /*
> * If the measured idle duration falls into the same bin as the sleep
> * length, this is a "hit", so update the "hits" metric for that bin.
> @@ -222,7 +232,7 @@ static void teo_update(struct cpuidle_dr
> cpu_data->tick_intercepts += PULSE;
> }
>
> - cpu_data->total -= cpu_data->total >> DECAY_SHIFT;
> + teo_decay(&cpu_data->total);
> cpu_data->total += PULSE;
This will result in total no longer being a strict sum of the bins.
Any reason not to do something like:
-----8<-----
diff --git a/drivers/cpuidle/governors/teo.c b/drivers/cpuidle/governors/teo.c
index e5b795cf3155..ff58d70ee80d 100644
--- a/drivers/cpuidle/governors/teo.c
+++ b/drivers/cpuidle/governors/teo.c
@@ -148,14 +148,19 @@ struct teo_cpu {
static DEFINE_PER_CPU(struct teo_cpu, teo_cpus);
-static void teo_decay(unsigned int *metric)
+static unsigned int teo_decay(unsigned int *metric)
{
unsigned int delta = *metric >> DECAY_SHIFT;
+ unsigned int decay;
- if (delta)
+ if (delta) {
*metric -= delta;
- else
- *metric = 0;
+ return delta;
+ }
+
+ decay = *metric;
+ *metric = 0;
+ return decay;
}
/**
@@ -168,6 +173,7 @@ static void teo_update(struct cpuidle_driver *drv, struct cpuidle_device *dev)
struct teo_cpu *cpu_data = per_cpu_ptr(&teo_cpus, dev->cpu);
int i, idx_timer = 0, idx_duration = 0;
s64 target_residency_ns, measured_ns;
+ unsigned int total_decay = 0;
teo_decay(&cpu_data->short_idles);
@@ -205,8 +211,8 @@ static void teo_update(struct cpuidle_driver *drv, struct cpuidle_device *dev)
for (i = 0; i < drv->state_count; i++) {
struct teo_bin *bin = &cpu_data->state_bins[i];
- teo_decay(&bin->hits);
- teo_decay(&bin->intercepts);
+ total_decay += teo_decay(&bin->hits);
+ total_decay += teo_decay(&bin->intercepts);
target_residency_ns = drv->states[i].target_residency_ns;
@@ -232,7 +238,7 @@ static void teo_update(struct cpuidle_driver *drv, struct cpuidle_device *dev)
cpu_data->tick_intercepts += PULSE;
}
- teo_decay(&cpu_data->total);
+ cpu_data->total -= total_decay;
cpu_data->total += PULSE;
}
^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH v1 4/4] cpuidle: governors: teo: Decay metrics below DECAY_SHIFT threshold
2025-11-12 17:29 ` Christian Loehle
@ 2025-11-12 17:51 ` Rafael J. Wysocki
2025-11-12 18:00 ` Christian Loehle
0 siblings, 1 reply; 26+ messages in thread
From: Rafael J. Wysocki @ 2025-11-12 17:51 UTC (permalink / raw)
To: Christian Loehle; +Cc: Rafael J. Wysocki, Linux PM, LKML, Reka Norman
On Wed, Nov 12, 2025 at 6:29 PM Christian Loehle
<christian.loehle@arm.com> wrote:
>
> On 11/12/25 16:25, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >
> > If a given governor metric falls below a certain value (8 for
> > DECAY_SHIFT equal to 3), it will not decay any more due to the
> > simplistic decay implementation. This may in some cases lead to
> > subtle inconsistencies in the governor behavior, so change the
> > decay implementation to take it into account and set the metric
> > at hand to 0 in that case.
> >
> > Suggested-by: Christian Loehle <christian.loehle@arm.com>
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > ---
> > drivers/cpuidle/governors/teo.c | 20 +++++++++++++++-----
> > 1 file changed, 15 insertions(+), 5 deletions(-)
> >
> > --- a/drivers/cpuidle/governors/teo.c
> > +++ b/drivers/cpuidle/governors/teo.c
> > @@ -148,6 +148,16 @@ struct teo_cpu {
> >
> > static DEFINE_PER_CPU(struct teo_cpu, teo_cpus);
> >
> > +static void teo_decay(unsigned int *metric)
> > +{
> > + unsigned int delta = *metric >> DECAY_SHIFT;
> > +
> > + if (delta)
> > + *metric -= delta;
> > + else
> > + *metric = 0;
> > +}
> > +
> > /**
> > * teo_update - Update CPU metrics after wakeup.
> > * @drv: cpuidle driver containing state data.
> > @@ -159,7 +169,7 @@ static void teo_update(struct cpuidle_dr
> > int i, idx_timer = 0, idx_duration = 0;
> > s64 target_residency_ns, measured_ns;
> >
> > - cpu_data->short_idles -= cpu_data->short_idles >> DECAY_SHIFT;
> > + teo_decay(&cpu_data->short_idles);
> >
> > if (cpu_data->artificial_wakeup) {
> > /*
> > @@ -195,8 +205,8 @@ static void teo_update(struct cpuidle_dr
> > for (i = 0; i < drv->state_count; i++) {
> > struct teo_bin *bin = &cpu_data->state_bins[i];
> >
> > - bin->hits -= bin->hits >> DECAY_SHIFT;
> > - bin->intercepts -= bin->intercepts >> DECAY_SHIFT;
> > + teo_decay(&bin->hits);
> > + teo_decay(&bin->intercepts);
> >
> > target_residency_ns = drv->states[i].target_residency_ns;
> >
> > @@ -207,7 +217,7 @@ static void teo_update(struct cpuidle_dr
> > }
> > }
> >
> > - cpu_data->tick_intercepts -= cpu_data->tick_intercepts >> DECAY_SHIFT;
> > + teo_decay(&cpu_data->tick_intercepts);
> > /*
> > * If the measured idle duration falls into the same bin as the sleep
> > * length, this is a "hit", so update the "hits" metric for that bin.
> > @@ -222,7 +232,7 @@ static void teo_update(struct cpuidle_dr
> > cpu_data->tick_intercepts += PULSE;
> > }
> >
> > - cpu_data->total -= cpu_data->total >> DECAY_SHIFT;
> > + teo_decay(&cpu_data->total);
> > cpu_data->total += PULSE;
>
> This will result in total no longer being a strict sum of the bins.
Ah, good point.
> Any reason not to do something like:
Well, it would be more straightforward to just compute "total" from
scratch instead of using total_decay (it would be the same amount of
computation minus the teo_decay() changes AFAICS).
I'll send an update of this patch.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v1 4/4] cpuidle: governors: teo: Decay metrics below DECAY_SHIFT threshold
2025-11-12 17:51 ` Rafael J. Wysocki
@ 2025-11-12 18:00 ` Christian Loehle
0 siblings, 0 replies; 26+ messages in thread
From: Christian Loehle @ 2025-11-12 18:00 UTC (permalink / raw)
To: Rafael J. Wysocki; +Cc: Linux PM, LKML, Reka Norman
On 11/12/25 17:51, Rafael J. Wysocki wrote:
> On Wed, Nov 12, 2025 at 6:29 PM Christian Loehle
> <christian.loehle@arm.com> wrote:
>>
>> On 11/12/25 16:25, Rafael J. Wysocki wrote:
>>> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>>>
>>> If a given governor metric falls below a certain value (8 for
>>> DECAY_SHIFT equal to 3), it will not decay any more due to the
>>> simplistic decay implementation. This may in some cases lead to
>>> subtle inconsistencies in the governor behavior, so change the
>>> decay implementation to take it into account and set the metric
>>> at hand to 0 in that case.
>>>
>>> Suggested-by: Christian Loehle <christian.loehle@arm.com>
>>> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>>> ---
>>> drivers/cpuidle/governors/teo.c | 20 +++++++++++++++-----
>>> 1 file changed, 15 insertions(+), 5 deletions(-)
>>>
>>> --- a/drivers/cpuidle/governors/teo.c
>>> +++ b/drivers/cpuidle/governors/teo.c
>>> @@ -148,6 +148,16 @@ struct teo_cpu {
>>>
>>> static DEFINE_PER_CPU(struct teo_cpu, teo_cpus);
>>>
>>> +static void teo_decay(unsigned int *metric)
>>> +{
>>> + unsigned int delta = *metric >> DECAY_SHIFT;
>>> +
>>> + if (delta)
>>> + *metric -= delta;
>>> + else
>>> + *metric = 0;
>>> +}
>>> +
>>> /**
>>> * teo_update - Update CPU metrics after wakeup.
>>> * @drv: cpuidle driver containing state data.
>>> @@ -159,7 +169,7 @@ static void teo_update(struct cpuidle_dr
>>> int i, idx_timer = 0, idx_duration = 0;
>>> s64 target_residency_ns, measured_ns;
>>>
>>> - cpu_data->short_idles -= cpu_data->short_idles >> DECAY_SHIFT;
>>> + teo_decay(&cpu_data->short_idles);
>>>
>>> if (cpu_data->artificial_wakeup) {
>>> /*
>>> @@ -195,8 +205,8 @@ static void teo_update(struct cpuidle_dr
>>> for (i = 0; i < drv->state_count; i++) {
>>> struct teo_bin *bin = &cpu_data->state_bins[i];
>>>
>>> - bin->hits -= bin->hits >> DECAY_SHIFT;
>>> - bin->intercepts -= bin->intercepts >> DECAY_SHIFT;
>>> + teo_decay(&bin->hits);
>>> + teo_decay(&bin->intercepts);
>>>
>>> target_residency_ns = drv->states[i].target_residency_ns;
>>>
>>> @@ -207,7 +217,7 @@ static void teo_update(struct cpuidle_dr
>>> }
>>> }
>>>
>>> - cpu_data->tick_intercepts -= cpu_data->tick_intercepts >> DECAY_SHIFT;
>>> + teo_decay(&cpu_data->tick_intercepts);
>>> /*
>>> * If the measured idle duration falls into the same bin as the sleep
>>> * length, this is a "hit", so update the "hits" metric for that bin.
>>> @@ -222,7 +232,7 @@ static void teo_update(struct cpuidle_dr
>>> cpu_data->tick_intercepts += PULSE;
>>> }
>>>
>>> - cpu_data->total -= cpu_data->total >> DECAY_SHIFT;
>>> + teo_decay(&cpu_data->total);
>>> cpu_data->total += PULSE;
>>
>> This will result in total no longer being a strict sum of the bins.
>
> Ah, good point.
>
>> Any reason not to do something like:
>
> Well, it would be more straightforward to just compute "total" from
> scratch instead of using total_decay (it would be the same amount of
> computation minus the teo_decay() changes AFAICS).
Duh, of course...
>
> I'll send an update of this patch.
Thanks!
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v2 4/4] cpuidle: governors: teo: Decay metrics below DECAY_SHIFT threshold
2025-11-12 16:25 ` [PATCH v1 4/4] cpuidle: governors: teo: Decay metrics below DECAY_SHIFT threshold Rafael J. Wysocki
2025-11-12 17:29 ` Christian Loehle
@ 2025-11-12 18:03 ` Rafael J. Wysocki
2025-11-13 11:49 ` Christian Loehle
1 sibling, 1 reply; 26+ messages in thread
From: Rafael J. Wysocki @ 2025-11-12 18:03 UTC (permalink / raw)
To: Linux PM, Christian Loehle; +Cc: LKML, Reka Norman
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
If a given governor metric falls below a certain value (8 for
DECAY_SHIFT equal to 3), it will not decay any more due to the
simplistic decay implementation. This may in some cases lead to
subtle inconsistencies in the governor behavior, so change the
decay implementation to take it into account and set the metric
at hand to 0 in that case.
Suggested-by: Christian Loehle <christian.loehle@arm.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
v1 -> v2:
* Ensure that cpu_data->total is always the sum of the intercepts and hits
metrics for all of the idle states (Christian).
---
drivers/cpuidle/governors/teo.c | 26 +++++++++++++++++++-------
1 file changed, 19 insertions(+), 7 deletions(-)
--- a/drivers/cpuidle/governors/teo.c
+++ b/drivers/cpuidle/governors/teo.c
@@ -148,6 +148,16 @@ struct teo_cpu {
static DEFINE_PER_CPU(struct teo_cpu, teo_cpus);
+static void teo_decay(unsigned int *metric)
+{
+ unsigned int delta = *metric >> DECAY_SHIFT;
+
+ if (delta)
+ *metric -= delta;
+ else
+ *metric = 0;
+}
+
/**
* teo_update - Update CPU metrics after wakeup.
* @drv: cpuidle driver containing state data.
@@ -158,8 +168,9 @@ static void teo_update(struct cpuidle_dr
struct teo_cpu *cpu_data = per_cpu_ptr(&teo_cpus, dev->cpu);
int i, idx_timer = 0, idx_duration = 0;
s64 target_residency_ns, measured_ns;
+ unsigned int total = 0;
- cpu_data->short_idles -= cpu_data->short_idles >> DECAY_SHIFT;
+ teo_decay(&cpu_data->short_idles);
if (cpu_data->artificial_wakeup) {
/*
@@ -195,8 +206,10 @@ static void teo_update(struct cpuidle_dr
for (i = 0; i < drv->state_count; i++) {
struct teo_bin *bin = &cpu_data->state_bins[i];
- bin->hits -= bin->hits >> DECAY_SHIFT;
- bin->intercepts -= bin->intercepts >> DECAY_SHIFT;
+ teo_decay(&bin->hits);
+ total += bin->hits;
+ teo_decay(&bin->intercepts);
+ total += bin->intercepts;
target_residency_ns = drv->states[i].target_residency_ns;
@@ -207,7 +220,9 @@ static void teo_update(struct cpuidle_dr
}
}
- cpu_data->tick_intercepts -= cpu_data->tick_intercepts >> DECAY_SHIFT;
+ cpu_data->total = total + PULSE;
+
+ teo_decay(&cpu_data->tick_intercepts);
/*
* If the measured idle duration falls into the same bin as the sleep
* length, this is a "hit", so update the "hits" metric for that bin.
@@ -221,9 +236,6 @@ static void teo_update(struct cpuidle_dr
if (TICK_NSEC <= measured_ns)
cpu_data->tick_intercepts += PULSE;
}
-
- cpu_data->total -= cpu_data->total >> DECAY_SHIFT;
- cpu_data->total += PULSE;
}
static bool teo_state_ok(int i, struct cpuidle_driver *drv)
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v1 1/4] cpuidle: governors: teo: Drop incorrect target residency check
2025-11-12 16:22 ` [PATCH v1 1/4] cpuidle: governors: teo: Drop incorrect target residency check Rafael J. Wysocki
@ 2025-11-13 11:32 ` Christian Loehle
2025-11-13 11:41 ` Rafael J. Wysocki
2025-11-13 13:24 ` [PATCH v2 1/4] cpuidle: governors: teo: Drop misguided " Rafael J. Wysocki
1 sibling, 1 reply; 26+ messages in thread
From: Christian Loehle @ 2025-11-13 11:32 UTC (permalink / raw)
To: Rafael J. Wysocki, Linux PM; +Cc: LKML, Reka Norman
On 11/12/25 16:22, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> When the target residency of the current candidate idle state is
> greater than the expected time till the closest timer (the sleep
> length), it does not matter whether or not the tick has already
> been stopped or if it is going to be stopped. The closest timer
> will trigger anyway at its due time, so it does not make sense to
> select an idle state with target residency above the sleep length.
>
> Accordingly, drop the teo_state_ok() check done in that case and
> let the governor use the teo_find_shallower_state() return value
> as the new candidate idle state index.
>
> Fixes: 21d28cd2fa5f ("cpuidle: teo: Do not call tick_nohz_get_sleep_length() upfront")
> Cc: All applicable <stable@vger.kernel.org>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
> drivers/cpuidle/governors/teo.c | 7 ++-----
> 1 file changed, 2 insertions(+), 5 deletions(-)
>
> --- a/drivers/cpuidle/governors/teo.c
> +++ b/drivers/cpuidle/governors/teo.c
> @@ -458,11 +458,8 @@ static int teo_select(struct cpuidle_dri
> * If the closest expected timer is before the target residency of the
> * candidate state, a shallower one needs to be found.
> */
> - if (drv->states[idx].target_residency_ns > duration_ns) {
> - i = teo_find_shallower_state(drv, dev, idx, duration_ns, false);
> - if (teo_state_ok(i, drv))
> - idx = i;
> - }
> + if (drv->states[idx].target_residency_ns > duration_ns)
> + idx = teo_find_shallower_state(drv, dev, idx, duration_ns, false);
>
> /*
> * If the selected state's target residency is below the tick length
>
>
>
AFAICT this check was to not be stuck in a shallow state when tick is already disabled.
There might be a timer armed in t+500us but that might still get cancelled, which
is why we didn't think a below TICK_NSEC 'shallow' state is acceptable?
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v1 1/4] cpuidle: governors: teo: Drop incorrect target residency check
2025-11-13 11:32 ` Christian Loehle
@ 2025-11-13 11:41 ` Rafael J. Wysocki
2025-11-13 11:47 ` Rafael J. Wysocki
0 siblings, 1 reply; 26+ messages in thread
From: Rafael J. Wysocki @ 2025-11-13 11:41 UTC (permalink / raw)
To: Christian Loehle; +Cc: Rafael J. Wysocki, Linux PM, LKML, Reka Norman
On Thu, Nov 13, 2025 at 12:32 PM Christian Loehle
<christian.loehle@arm.com> wrote:
>
> On 11/12/25 16:22, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >
> > When the target residency of the current candidate idle state is
> > greater than the expected time till the closest timer (the sleep
> > length), it does not matter whether or not the tick has already
> > been stopped or if it is going to be stopped. The closest timer
> > will trigger anyway at its due time, so it does not make sense to
> > select an idle state with target residency above the sleep length.
> >
> > Accordingly, drop the teo_state_ok() check done in that case and
> > let the governor use the teo_find_shallower_state() return value
> > as the new candidate idle state index.
> >
> > Fixes: 21d28cd2fa5f ("cpuidle: teo: Do not call tick_nohz_get_sleep_length() upfront")
> > Cc: All applicable <stable@vger.kernel.org>
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > ---
> > drivers/cpuidle/governors/teo.c | 7 ++-----
> > 1 file changed, 2 insertions(+), 5 deletions(-)
> >
> > --- a/drivers/cpuidle/governors/teo.c
> > +++ b/drivers/cpuidle/governors/teo.c
> > @@ -458,11 +458,8 @@ static int teo_select(struct cpuidle_dri
> > * If the closest expected timer is before the target residency of the
> > * candidate state, a shallower one needs to be found.
> > */
> > - if (drv->states[idx].target_residency_ns > duration_ns) {
> > - i = teo_find_shallower_state(drv, dev, idx, duration_ns, false);
> > - if (teo_state_ok(i, drv))
> > - idx = i;
> > - }
> > + if (drv->states[idx].target_residency_ns > duration_ns)
> > + idx = teo_find_shallower_state(drv, dev, idx, duration_ns, false);
> >
> > /*
> > * If the selected state's target residency is below the tick length
> >
> >
> >
>
> AFAICT this check was to not be stuck in a shallow state when tick is already disabled.
> There might be a timer armed in t+500us but that might still get cancelled, which
> is why we didn't think a below TICK_NSEC 'shallow' state is acceptable?
This is all about hrtimers which are not expected to be canceled too
often and real energy is wasted here by going too deep if the timer is
not canceled.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v1 2/4] cpuidle: governors: teo: Drop redundant function parameter
2025-11-12 16:23 ` [PATCH v1 2/4] cpuidle: governors: teo: Drop redundant function parameter Rafael J. Wysocki
@ 2025-11-13 11:46 ` Christian Loehle
0 siblings, 0 replies; 26+ messages in thread
From: Christian Loehle @ 2025-11-13 11:46 UTC (permalink / raw)
To: Rafael J. Wysocki, Linux PM; +Cc: LKML, Reka Norman
On 11/12/25 16:23, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> The last no_poll parameter of teo_find_shallower_state() is always
> false, so drop it.
>
> No intentional functional impact.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Reviewed-by: Christian Loehle <christian.loehle@arm.com>
> ---
> drivers/cpuidle/governors/teo.c | 10 ++++------
> 1 file changed, 4 insertions(+), 6 deletions(-)
>
> --- a/drivers/cpuidle/governors/teo.c
> +++ b/drivers/cpuidle/governors/teo.c
> @@ -239,17 +239,15 @@ static bool teo_state_ok(int i, struct c
> * @dev: Target CPU.
> * @state_idx: Index of the capping idle state.
> * @duration_ns: Idle duration value to match.
> - * @no_poll: Don't consider polling states.
> */
> static int teo_find_shallower_state(struct cpuidle_driver *drv,
> struct cpuidle_device *dev, int state_idx,
> - s64 duration_ns, bool no_poll)
> + s64 duration_ns)
> {
> int i;
>
> for (i = state_idx - 1; i >= 0; i--) {
> - if (dev->states_usage[i].disable ||
> - (no_poll && drv->states[i].flags & CPUIDLE_FLAG_POLLING))
> + if (dev->states_usage[i].disable)
> continue;
>
> state_idx = i;
> @@ -459,7 +457,7 @@ static int teo_select(struct cpuidle_dri
> * candidate state, a shallower one needs to be found.
> */
> if (drv->states[idx].target_residency_ns > duration_ns)
> - idx = teo_find_shallower_state(drv, dev, idx, duration_ns, false);
> + idx = teo_find_shallower_state(drv, dev, idx, duration_ns);
>
> /*
> * If the selected state's target residency is below the tick length
> @@ -487,7 +485,7 @@ end:
> */
> if (idx > idx0 &&
> drv->states[idx].target_residency_ns > delta_tick)
> - idx = teo_find_shallower_state(drv, dev, idx, delta_tick, false);
> + idx = teo_find_shallower_state(drv, dev, idx, delta_tick);
>
> out_tick:
> *stop_tick = false;
>
>
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v1 1/4] cpuidle: governors: teo: Drop incorrect target residency check
2025-11-13 11:41 ` Rafael J. Wysocki
@ 2025-11-13 11:47 ` Rafael J. Wysocki
2025-11-13 13:26 ` Rafael J. Wysocki
0 siblings, 1 reply; 26+ messages in thread
From: Rafael J. Wysocki @ 2025-11-13 11:47 UTC (permalink / raw)
To: Christian Loehle; +Cc: Linux PM, LKML, Reka Norman
On Thu, Nov 13, 2025 at 12:41 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
>
> On Thu, Nov 13, 2025 at 12:32 PM Christian Loehle
> <christian.loehle@arm.com> wrote:
> >
> > On 11/12/25 16:22, Rafael J. Wysocki wrote:
> > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > >
> > > When the target residency of the current candidate idle state is
> > > greater than the expected time till the closest timer (the sleep
> > > length), it does not matter whether or not the tick has already
> > > been stopped or if it is going to be stopped. The closest timer
> > > will trigger anyway at its due time, so it does not make sense to
> > > select an idle state with target residency above the sleep length.
> > >
> > > Accordingly, drop the teo_state_ok() check done in that case and
> > > let the governor use the teo_find_shallower_state() return value
> > > as the new candidate idle state index.
> > >
> > > Fixes: 21d28cd2fa5f ("cpuidle: teo: Do not call tick_nohz_get_sleep_length() upfront")
> > > Cc: All applicable <stable@vger.kernel.org>
> > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > > ---
> > > drivers/cpuidle/governors/teo.c | 7 ++-----
> > > 1 file changed, 2 insertions(+), 5 deletions(-)
> > >
> > > --- a/drivers/cpuidle/governors/teo.c
> > > +++ b/drivers/cpuidle/governors/teo.c
> > > @@ -458,11 +458,8 @@ static int teo_select(struct cpuidle_dri
> > > * If the closest expected timer is before the target residency of the
> > > * candidate state, a shallower one needs to be found.
> > > */
> > > - if (drv->states[idx].target_residency_ns > duration_ns) {
> > > - i = teo_find_shallower_state(drv, dev, idx, duration_ns, false);
> > > - if (teo_state_ok(i, drv))
> > > - idx = i;
> > > - }
> > > + if (drv->states[idx].target_residency_ns > duration_ns)
> > > + idx = teo_find_shallower_state(drv, dev, idx, duration_ns, false);
> > >
> > > /*
> > > * If the selected state's target residency is below the tick length
> > >
> > >
> > >
> >
> > AFAICT this check was to not be stuck in a shallow state when tick is already disabled.
> > There might be a timer armed in t+500us but that might still get cancelled, which
> > is why we didn't think a below TICK_NSEC 'shallow' state is acceptable?
>
> This is all about hrtimers which are not expected to be canceled too
> often and real energy is wasted here by going too deep if the timer is
> not canceled.
Overall, both teo and menu assume that the timers reported by
tick_nohz_get_sleep_length() will trigger. Otherwise, calling it
would be kind of pointless ...
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v1 3/4] cpuidle: governors: teo: Use s64 consistently in teo_update()
2025-11-12 16:24 ` [PATCH v1 3/4] cpuidle: governors: teo: Use s64 consistently in teo_update() Rafael J. Wysocki
@ 2025-11-13 11:48 ` Christian Loehle
0 siblings, 0 replies; 26+ messages in thread
From: Christian Loehle @ 2025-11-13 11:48 UTC (permalink / raw)
To: Rafael J. Wysocki, Linux PM; +Cc: LKML, Reka Norman
On 11/12/25 16:24, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> Two local variables in teo_update() are defined as u64, but their
> values are then compared with s64 values, so it is more consistent
> to use s64 as their data type.
>
> No intentional functional impact.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Reviewed-by: Christian Loehle <christian.loehle@arm.com>
> ---
> drivers/cpuidle/governors/teo.c | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> --- a/drivers/cpuidle/governors/teo.c
> +++ b/drivers/cpuidle/governors/teo.c
> @@ -157,8 +157,7 @@ static void teo_update(struct cpuidle_dr
> {
> struct teo_cpu *cpu_data = per_cpu_ptr(&teo_cpus, dev->cpu);
> int i, idx_timer = 0, idx_duration = 0;
> - s64 target_residency_ns;
> - u64 measured_ns;
> + s64 target_residency_ns, measured_ns;
>
> cpu_data->short_idles -= cpu_data->short_idles >> DECAY_SHIFT;
>
> @@ -167,9 +166,9 @@ static void teo_update(struct cpuidle_dr
> * If one of the safety nets has triggered, assume that this
> * might have been a long sleep.
> */
> - measured_ns = U64_MAX;
> + measured_ns = S64_MAX;
> } else {
> - u64 lat_ns = drv->states[dev->last_state_idx].exit_latency_ns;
> + s64 lat_ns = drv->states[dev->last_state_idx].exit_latency_ns;
>
> measured_ns = dev->last_residency_ns;
> /*
>
>
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 4/4] cpuidle: governors: teo: Decay metrics below DECAY_SHIFT threshold
2025-11-12 18:03 ` [PATCH v2 " Rafael J. Wysocki
@ 2025-11-13 11:49 ` Christian Loehle
0 siblings, 0 replies; 26+ messages in thread
From: Christian Loehle @ 2025-11-13 11:49 UTC (permalink / raw)
To: Rafael J. Wysocki, Linux PM; +Cc: LKML, Reka Norman
On 11/12/25 18:03, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> If a given governor metric falls below a certain value (8 for
> DECAY_SHIFT equal to 3), it will not decay any more due to the
> simplistic decay implementation. This may in some cases lead to
> subtle inconsistencies in the governor behavior, so change the
> decay implementation to take it into account and set the metric
> at hand to 0 in that case.
>
> Suggested-by: Christian Loehle <christian.loehle@arm.com>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Reviewed-by: Christian Loehle <christian.loehle@arm.com>
> ---
>
> v1 -> v2:
> * Ensure that cpu_data->total is always the sum of the intercepts and hits
> metrics for all of the idle states (Christian).
>
> ---
> drivers/cpuidle/governors/teo.c | 26 +++++++++++++++++++-------
> 1 file changed, 19 insertions(+), 7 deletions(-)
>
> --- a/drivers/cpuidle/governors/teo.c
> +++ b/drivers/cpuidle/governors/teo.c
> @@ -148,6 +148,16 @@ struct teo_cpu {
>
> static DEFINE_PER_CPU(struct teo_cpu, teo_cpus);
>
> +static void teo_decay(unsigned int *metric)
> +{
> + unsigned int delta = *metric >> DECAY_SHIFT;
> +
> + if (delta)
> + *metric -= delta;
> + else
> + *metric = 0;
> +}
> +
> /**
> * teo_update - Update CPU metrics after wakeup.
> * @drv: cpuidle driver containing state data.
> @@ -158,8 +168,9 @@ static void teo_update(struct cpuidle_dr
> struct teo_cpu *cpu_data = per_cpu_ptr(&teo_cpus, dev->cpu);
> int i, idx_timer = 0, idx_duration = 0;
> s64 target_residency_ns, measured_ns;
> + unsigned int total = 0;
>
> - cpu_data->short_idles -= cpu_data->short_idles >> DECAY_SHIFT;
> + teo_decay(&cpu_data->short_idles);
>
> if (cpu_data->artificial_wakeup) {
> /*
> @@ -195,8 +206,10 @@ static void teo_update(struct cpuidle_dr
> for (i = 0; i < drv->state_count; i++) {
> struct teo_bin *bin = &cpu_data->state_bins[i];
>
> - bin->hits -= bin->hits >> DECAY_SHIFT;
> - bin->intercepts -= bin->intercepts >> DECAY_SHIFT;
> + teo_decay(&bin->hits);
> + total += bin->hits;
> + teo_decay(&bin->intercepts);
> + total += bin->intercepts;
>
> target_residency_ns = drv->states[i].target_residency_ns;
>
> @@ -207,7 +220,9 @@ static void teo_update(struct cpuidle_dr
> }
> }
>
> - cpu_data->tick_intercepts -= cpu_data->tick_intercepts >> DECAY_SHIFT;
> + cpu_data->total = total + PULSE;
> +
> + teo_decay(&cpu_data->tick_intercepts);
> /*
> * If the measured idle duration falls into the same bin as the sleep
> * length, this is a "hit", so update the "hits" metric for that bin.
> @@ -221,9 +236,6 @@ static void teo_update(struct cpuidle_dr
> if (TICK_NSEC <= measured_ns)
> cpu_data->tick_intercepts += PULSE;
> }
> -
> - cpu_data->total -= cpu_data->total >> DECAY_SHIFT;
> - cpu_data->total += PULSE;
> }
>
> static bool teo_state_ok(int i, struct cpuidle_driver *drv)
>
>
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v2 1/4] cpuidle: governors: teo: Drop misguided target residency check
2025-11-12 16:22 ` [PATCH v1 1/4] cpuidle: governors: teo: Drop incorrect target residency check Rafael J. Wysocki
2025-11-13 11:32 ` Christian Loehle
@ 2025-11-13 13:24 ` Rafael J. Wysocki
2025-11-14 9:16 ` Christian Loehle
1 sibling, 1 reply; 26+ messages in thread
From: Rafael J. Wysocki @ 2025-11-13 13:24 UTC (permalink / raw)
To: Linux PM, Christian Loehle; +Cc: LKML, Reka Norman
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
When the target residency of the current candidate idle state is
greater than the expected time till the closest timer (the sleep
length), it does not matter whether or not the tick has already been
stopped or if it is going to be stopped. The closest timer will
trigger anyway at its due time, so if an idle state with target
residency above the sleep length is selected, energy will be wasted
and there may be excess latency.
Of course, if the closest timer were canceled before it could trigger,
a deeper idle state would be more suitable, but this is not expected
to happen (generally speaking, hrtimers are not expected to be
canceled as a rule).
Accordingly, the teo_state_ok() check done in that case causes energy to
be wasted more often than it allows any energy to be saved (if it allows
any energy to be saved at all), so drop it and let the governor use the
teo_find_shallower_state() return value as the new candidate idle state
index.
Fixes: 21d28cd2fa5f ("cpuidle: teo: Do not call tick_nohz_get_sleep_length() upfront")
Cc: All applicable <stable@vger.kernel.org>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
v1 -> v2: Subject and changelog modifications
---
drivers/cpuidle/governors/teo.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
--- a/drivers/cpuidle/governors/teo.c
+++ b/drivers/cpuidle/governors/teo.c
@@ -458,11 +458,8 @@ static int teo_select(struct cpuidle_dri
* If the closest expected timer is before the target residency of the
* candidate state, a shallower one needs to be found.
*/
- if (drv->states[idx].target_residency_ns > duration_ns) {
- i = teo_find_shallower_state(drv, dev, idx, duration_ns, false);
- if (teo_state_ok(i, drv))
- idx = i;
- }
+ if (drv->states[idx].target_residency_ns > duration_ns)
+ idx = teo_find_shallower_state(drv, dev, idx, duration_ns, false);
/*
* If the selected state's target residency is below the tick length
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v1 1/4] cpuidle: governors: teo: Drop incorrect target residency check
2025-11-13 11:47 ` Rafael J. Wysocki
@ 2025-11-13 13:26 ` Rafael J. Wysocki
0 siblings, 0 replies; 26+ messages in thread
From: Rafael J. Wysocki @ 2025-11-13 13:26 UTC (permalink / raw)
To: Christian Loehle; +Cc: Linux PM, LKML, Reka Norman
On Thu, Nov 13, 2025 at 12:47 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
>
> On Thu, Nov 13, 2025 at 12:41 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
> >
> > On Thu, Nov 13, 2025 at 12:32 PM Christian Loehle
> > <christian.loehle@arm.com> wrote:
> > >
> > > On 11/12/25 16:22, Rafael J. Wysocki wrote:
> > > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > > >
> > > > When the target residency of the current candidate idle state is
> > > > greater than the expected time till the closest timer (the sleep
> > > > length), it does not matter whether or not the tick has already
> > > > been stopped or if it is going to be stopped. The closest timer
> > > > will trigger anyway at its due time, so it does not make sense to
> > > > select an idle state with target residency above the sleep length.
> > > >
> > > > Accordingly, drop the teo_state_ok() check done in that case and
> > > > let the governor use the teo_find_shallower_state() return value
> > > > as the new candidate idle state index.
> > > >
> > > > Fixes: 21d28cd2fa5f ("cpuidle: teo: Do not call tick_nohz_get_sleep_length() upfront")
> > > > Cc: All applicable <stable@vger.kernel.org>
> > > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > > > ---
> > > > drivers/cpuidle/governors/teo.c | 7 ++-----
> > > > 1 file changed, 2 insertions(+), 5 deletions(-)
> > > >
> > > > --- a/drivers/cpuidle/governors/teo.c
> > > > +++ b/drivers/cpuidle/governors/teo.c
> > > > @@ -458,11 +458,8 @@ static int teo_select(struct cpuidle_dri
> > > > * If the closest expected timer is before the target residency of the
> > > > * candidate state, a shallower one needs to be found.
> > > > */
> > > > - if (drv->states[idx].target_residency_ns > duration_ns) {
> > > > - i = teo_find_shallower_state(drv, dev, idx, duration_ns, false);
> > > > - if (teo_state_ok(i, drv))
> > > > - idx = i;
> > > > - }
> > > > + if (drv->states[idx].target_residency_ns > duration_ns)
> > > > + idx = teo_find_shallower_state(drv, dev, idx, duration_ns, false);
> > > >
> > > > /*
> > > > * If the selected state's target residency is below the tick length
> > > >
> > > >
> > > >
> > >
> > > AFAICT this check was to not be stuck in a shallow state when tick is already disabled.
> > > There might be a timer armed in t+500us but that might still get cancelled, which
> > > is why we didn't think a below TICK_NSEC 'shallow' state is acceptable?
> >
> > This is all about hrtimers which are not expected to be canceled too
> > often and real energy is wasted here by going too deep if the timer is
> > not canceled.
>
> Overall, both teo and menu assume that the timers reported by
> tick_nohz_get_sleep_length() will trigger. Otherwise, calling it
> would be kind of pointless ...
Anyway, I've sent a v2 of the $subject patch with a more elaborate changelog:
https://lore.kernel.org/linux-pm/5955081.DvuYhMxLoT@rafael.j.wysocki/
Hopefully, it is more convincing.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v1 0/4] cpuidle: governors: teo: Assorted improvements
2025-11-12 16:21 [PATCH v1 0/4] cpuidle: governors: teo: Assorted improvements Rafael J. Wysocki
` (3 preceding siblings ...)
2025-11-12 16:25 ` [PATCH v1 4/4] cpuidle: governors: teo: Decay metrics below DECAY_SHIFT threshold Rafael J. Wysocki
@ 2025-11-13 15:21 ` Christian Loehle
2025-11-13 15:25 ` Rafael J. Wysocki
2025-11-19 22:52 ` Doug Smythies
4 siblings, 2 replies; 26+ messages in thread
From: Christian Loehle @ 2025-11-13 15:21 UTC (permalink / raw)
To: Rafael J. Wysocki, Linux PM; +Cc: LKML, Reka Norman
On 11/12/25 16:21, Rafael J. Wysocki wrote:
> Hi,
>
> This is a bunch of teo cpuidle governor improvements, some of which are related
> to a bug report discussed recently:
>
> https://lore.kernel.org/linux-pm/CAEmPcwsNMNnNXuxgvHTQ93Mx-q3Oz9U57THQsU_qdcCx1m4w5g@mail.gmail.com/
>
> The first patch fixes a bug that may cause an overly deep idle state
> to be selected when the scheduler tick has been already stopped.
>
> Patch [2/4] removes an unnecessary function argument.
>
> Patch [3/4] makes teo_update() to use s64 as the data type for its local
> variables more consistently.
>
> The last patch reworks the governor's decay implementation to also decay
> metric values lower than 8.
>
Tested-by: Christian Loehle <christian.loehle@arm.com>
Test results below, although there really isn't anything interesting in there.
teo-1 to teo-4 (patches 1 to 4 respectively are essentially indistinguishable from
teo-m = mainline)
device gov iter iops idles idle_miss ratio belows aboves
mmcblk1 teo-m 0 2330 644806 69 0.000 47 22
mmcblk1 teo-m 1 2328 642994 102 0.000 66 36
mmcblk1 teo-m 2 2352 644020 44 0.000 28 16
mmcblk1 teo-1 0 2324 582886 174 0.000 52 122
mmcblk1 teo-1 1 2350 584012 57 0.000 45 12
mmcblk1 teo-1 2 2329 642238 55 0.000 36 19
mmcblk1 teo-2 0 2327 627590 90 0.000 40 50
mmcblk1 teo-2 1 2328 640178 62 0.000 43 19
mmcblk1 teo-2 2 2327 590950 109 0.000 97 12
mmcblk1 teo-3 0 2328 644366 55 0.000 41 14
mmcblk1 teo-3 1 2327 642740 57 0.000 31 26
mmcblk1 teo-3 2 2327 638428 77 0.000 51 26
mmcblk1 teo-4 0 2326 614326 137 0.000 35 102
mmcblk1 teo-4 1 2328 639990 29 0.000 24 5
mmcblk1 teo-4 2 2327 643086 62 0.000 41 21
mmcblk1 teo-6-6 0 2329 596768 1753 0.003 1686 67
mmcblk1 teo-6-6 1 2330 600324 1527 0.003 1495 32
mmcblk1 teo-6-6 2 2331 600110 1439 0.002 1397 42
mmcblk1 teo-6-12 0 2347 741170 6 0.000 4 2
mmcblk1 teo-6-12 1 2329 782232 0 0.000 4 2
mmcblk1 teo-6-12 2 2328 775652 10 0.000 8 2
mmcblk2 teo-m 0 5758 860660 61 0.000 47 14
mmcblk2 teo-m 1 5731 827264 493 0.001 477 16
mmcblk2 teo-m 2 5700 861346 69 0.000 50 19
mmcblk2 teo-1 0 5663 848222 708 0.001 666 42
mmcblk2 teo-1 1 5648 704886 563 0.001 528 35
mmcblk2 teo-1 2 5655 705698 208 0.000 161 47
mmcblk2 teo-2 0 5697 908264 326 0.000 316 10
mmcblk2 teo-2 1 5656 720270 592 0.001 515 77
mmcblk2 teo-2 2 5641 673948 540 0.001 501 39
mmcblk2 teo-3 0 5675 724770 709 0.001 666 43
mmcblk2 teo-3 1 5675 792828 1144 0.001 892 252
mmcblk2 teo-3 2 5701 909848 660 0.001 332 328
mmcblk2 teo-4 0 5673 860736 798 0.001 791 7
mmcblk2 teo-4 1 5606 508886 1786 0.004 1727 59
mmcblk2 teo-4 2 5632 569238 104 0.000 73 31
mmcblk2 teo-6-6 0 5692 848824 1748 0.002 1685 63
mmcblk2 teo-6-6 1 5898 876180 1225 0.001 1215 10
mmcblk2 teo-6-6 2 5646 633846 2248 0.004 2199 49
mmcblk2 teo-6-12 0 5869 1009742 9 0.000 6 3
mmcblk2 teo-6-12 1 5644 775326 4 0.000 4 0
mmcblk2 teo-6-12 2 5671 904480 4 0.000 4 0
nvme0n1 teo-m 0 10342 765802 152 0.000 55 97
nvme0n1 teo-m 1 10703 789232 157 0.000 52 105
nvme0n1 teo-m 2 10405 770046 62 0.000 34 28
nvme0n1 teo-1 0 10687 782984 84 0.000 40 44
nvme0n1 teo-1 1 11326 829102 79 0.000 33 46
nvme0n1 teo-1 2 10636 780884 197 0.000 98 99
nvme0n1 teo-2 0 10951 803155 108 0.000 39 69
nvme0n1 teo-2 1 11468 837334 55 0.000 35 20
nvme0n1 teo-2 2 10410 773154 148 0.000 63 85
nvme0n1 teo-3 0 10701 787882 101 0.000 47 54
nvme0n1 teo-3 1 10403 767178 54 0.000 31 23
nvme0n1 teo-3 2 10749 785898 209 0.000 51 158
nvme0n1 teo-4 0 10462 768442 149 0.000 56 93
nvme0n1 teo-4 1 10642 777534 85 0.000 45 40
nvme0n1 teo-4 2 10486 775710 172 0.000 62 110
nvme0n1 teo-6-6 0 11507 798450 1564 0.002 1505 59
nvme0n1 teo-6-6 1 10499 735294 2009 0.003 1879 130
nvme0n1 teo-6-6 2 10598 740936 1756 0.002 1477 279
nvme0n1 teo-6-12 0 10542 922846 18 0.000 12 6
nvme0n1 teo-6-12 1 10482 922114 23 0.000 16 7
nvme0n1 teo-6-12 2 11046 940398 23 0.000 17 6
sda teo-m 0 1269 778958 111 0.000 108 3
sda teo-m 1 1268 831064 1064 0.001 1054 10
sda teo-m 2 1270 803714 2044 0.003 2034 10
sda teo-1 0 1273 776836 143 0.000 131 12
sda teo-1 1 1272 801800 2043 0.003 2043 0
sda teo-1 2 1270 788906 165 0.000 155 10
sda teo-2 0 1271 763694 79 0.000 68 11
sda teo-2 1 1270 753904 66 0.000 62 4
sda teo-2 2 1272 806804 45 0.000 38 7
sda teo-3 0 1271 812758 1497 0.002 1494 3
sda teo-3 1 1270 800006 49 0.000 48 1
sda teo-3 2 1272 829854 358 0.000 356 2
sda teo-4 0 1270 886646 243 0.000 239 4
sda teo-4 1 1269 820968 40 0.000 38 2
sda teo-4 2 1272 835716 124 0.000 109 15
sda teo-6-6 0 1257 591486 5450 0.009 5435 15
sda teo-6-6 1 1265 788536 4412 0.006 4404 8
sda teo-6-6 2 1254 788282 4470 0.006 4461 9
sda teo-6-12 0 1274 841398 0 0.000 4461 9
sda teo-6-12 1 1271 840824 0 0.000 4461 9
sda teo-6-12 2 1272 923816 4 0.000 4 0
nullb0 teo-m 0 101284 114406 178 0.002 70 108
nullb0 teo-m 1 101381 112804 496 0.004 303 193
nullb0 teo-m 2 101716 100370 380 0.004 225 155
nullb0 teo-1 0 101353 101198 366 0.004 202 164
nullb0 teo-1 1 100571 105004 168 0.002 87 81
nullb0 teo-1 2 101311 110644 56 0.001 26 30
nullb0 teo-2 0 100961 113126 261 0.002 159 102
nullb0 teo-2 1 101346 111776 463 0.004 261 202
nullb0 teo-2 2 101287 109772 277 0.003 137 140
nullb0 teo-3 0 101448 113264 239 0.002 141 98
nullb0 teo-3 1 101077 112294 301 0.003 47 254
nullb0 teo-3 2 100097 116756 423 0.004 267 156
nullb0 teo-4 0 99562 109606 52 0.000 30 22
nullb0 teo-4 1 101131 113982 136 0.001 45 91
nullb0 teo-4 2 100618 113698 144 0.001 41 103
nullb0 teo-6-6 0 101295 74346 2369 0.032 2139 230
nullb0 teo-6-6 1 101299 75350 1943 0.026 1803 140
nullb0 teo-6-6 2 100578 73924 2317 0.031 2089 228
nullb0 teo-6-12 0 100005 269180 7 0.000 4 3
nullb0 teo-6-12 1 100705 315302 0 0.000 4 3
nullb0 teo-6-12 2 100861 293350 6 0.000 3 3
mtdblock3 teo-m 0 262 312586 66 0.000 47 19
mtdblock3 teo-m 1 259 635682 100 0.000 97 3
mtdblock3 teo-m 2 258 771110 128 0.000 89 39
mtdblock3 teo-1 0 256 601566 7409 0.012 7396 13
mtdblock3 teo-1 1 260 535040 75 0.000 55 20
mtdblock3 teo-1 2 259 702118 1844 0.003 1820 24
mtdblock3 teo-2 0 261 312978 1813 0.006 1789 24
mtdblock3 teo-2 1 255 891548 3095 0.003 3090 5
mtdblock3 teo-2 2 259 680754 234 0.000 223 11
mtdblock3 teo-3 0 259 649474 102 0.000 94 8
mtdblock3 teo-3 1 260 552324 223 0.000 182 41
mtdblock3 teo-3 2 258 734378 1562 0.002 1551 11
mtdblock3 teo-4 0 257 762430 2438 0.003 2430 8
mtdblock3 teo-4 1 252 894318 9326 0.010 9314 12
mtdblock3 teo-4 2 258 663502 1437 0.002 1421 16
mtdblock3 teo-6-6 0 258 782434 1611 0.002 1572 39
mtdblock3 teo-6-6 1 259 591852 1658 0.003 1532 126
mtdblock3 teo-6-6 2 262 274192 1523 0.006 1432 91
mtdblock3 teo-6-12 0 256 986250 11 0.000 10 1
mtdblock3 teo-6-12 1 258 853982 11 0.000 9 2
mtdblock3 teo-6-12 2 260 654184 7 0.000 6 1
test gov i score %change idles idle_miss miss_rt belows aboves
schbench teo-m 0 196 0 26276 12 0.000 10 2
schbench teo-m 1 195 -0 25668 23 0.001 19 4
schbench teo-m 2 208 6 26180 24 0.001 21 3
schbench teo-m 3 191 -2 25384 4 0.000 3 1
schbench teo-m 4 194 -1 26060 31 0.001 21 10
schbench teo-1 0 195 -0 26476 10 0.000 9 1
schbench teo-1 1 194 -1 24880 9 0.000 9 0
schbench teo-1 2 194 -1 24540 10 0.000 8 2
schbench teo-1 3 195 -1 26246 30 0.001 23 7
schbench teo-1 4 194 -1 26060 15 0.001 11 4
schbench teo-2 0 198 1 27150 19 0.001 13 6
schbench teo-2 1 195 -0 25354 8 0.000 8 0
schbench teo-2 2 194 -1 24396 21 0.001 18 3
schbench teo-2 3 193 -1 26044 20 0.001 18 2
schbench teo-2 4 195 -0 26444 22 0.001 20 2
schbench teo-3 0 192 -2 24988 10 0.000 9 1
schbench teo-3 1 193 -1 25936 9 0.000 7 2
schbench teo-3 2 190 -3 25218 14 0.001 13 1
schbench teo-3 3 191 -2 25612 12 0.000 9 3
schbench teo-3 4 192 -2 24892 28 0.001 17 11
schbench teo-4 0 197 1 26768 16 0.001 14 2
schbench teo-4 1 193 -1 26352 15 0.001 13 2
schbench teo-4 2 194 -1 26158 15 0.001 14 1
schbench teo-4 3 203 4 27536 28 0.001 26 2
schbench teo-4 4 192 -2 24806 19 0.001 15 4
schbench teo-6-6 0 199 2 24676 128 0.005 121 7
schbench teo-6-6 1 211 8 26794 127 0.005 117 10
schbench teo-6-6 2 205 5 25026 140 0.006 125 15
schbench teo-6-6 3 198 1 24816 196 0.008 174 22
schbench teo-6-6 4 198 1 26604 236 0.009 218 18
schbench teo-6-12 0 190 -3 24464 0 0.000 218 18
schbench teo-6-12 1 190 -3 25274 0 0.000 218 18
schbench teo-6-12 2 190 -3 25256 0 0.000 218 18
schbench teo-6-12 3 199 2 26342 0 0.000 218 18
schbench teo-6-12 4 188 -4 25508 0 0.000 218 18
ebizzy teo-m 0 10733 0 1358 7 0.005 7 0
ebizzy teo-m 1 10750 0 1206 12 0.010 8 4
ebizzy teo-m 2 10699 -0 1950 24 0.012 23 1
ebizzy teo-m 3 10773 0 1384 18 0.013 16 2
ebizzy teo-m 4 10667 -1 2060 24 0.012 23 1
ebizzy teo-1 0 10770 0 1858 22 0.012 19 3
ebizzy teo-1 1 10719 -0 1258 4 0.003 4 0
ebizzy teo-1 2 10704 -0 1422 5 0.004 5 0
ebizzy teo-1 3 10719 -0 1568 16 0.010 14 2
ebizzy teo-1 4 10802 1 1288 11 0.009 11 0
ebizzy teo-2 0 10669 -1 1126 11 0.010 9 2
ebizzy teo-2 1 10696 -0 1733 18 0.010 14 4
ebizzy teo-2 2 10676 -1 1620 17 0.010 14 3
ebizzy teo-2 3 10737 0 1332 10 0.008 9 1
ebizzy teo-2 4 10696 -0 1290 8 0.006 8 0
ebizzy teo-3 0 10650 -1 1448 12 0.008 9 3
ebizzy teo-3 1 10689 -0 1470 12 0.008 12 0
ebizzy teo-3 2 10642 -1 1612 19 0.012 15 4
ebizzy teo-3 3 10705 -0 1462 10 0.007 8 2
ebizzy teo-3 4 10717 -0 1798 21 0.012 19 2
ebizzy teo-4 0 10625 -1 1324 12 0.009 8 4
ebizzy teo-4 1 10705 -0 1430 12 0.008 11 1
ebizzy teo-4 2 10671 -1 1818 19 0.010 16 3
ebizzy teo-4 3 10688 -0 1616 8 0.005 8 0
ebizzy teo-4 4 10724 -0 1882 11 0.006 9 2
ebizzy teo-6-6 0 10659 -1 1442 58 0.040 46 12
ebizzy teo-6-6 1 10653 -1 1144 43 0.038 41 2
ebizzy teo-6-6 2 10696 -0 1256 37 0.029 35 2
ebizzy teo-6-6 3 10710 -0 1168 43 0.037 38 5
ebizzy teo-6-6 4 10696 -0 1166 47 0.040 43 4
ebizzy teo-6-12 0 10709 -0 2186 0 0.000 43 4
ebizzy teo-6-12 1 10687 -0 1688 0 0.000 43 4
ebizzy teo-6-12 2 10689 -0 2806 0 0.000 43 4
ebizzy teo-6-12 3 10734 0 1426 0 0.000 43 4
ebizzy teo-6-12 4 10757 0 2346 0 0.000 43 4
adrestia teo-m 0 8 0 103680 21 0.000 19 2
adrestia teo-m 1 12 50 104026 31 0.000 16 15
adrestia teo-m 2 12 50 104063 32 0.000 24 8
adrestia teo-m 3 12 50 104768 44 0.000 22 22
adrestia teo-m 4 12 50 104388 18 0.000 16 2
adrestia teo-1 0 12 50 104152 26 0.000 15 11
adrestia teo-1 1 12 50 103556 12 0.000 9 3
adrestia teo-1 2 11 38 104202 36 0.000 24 12
adrestia teo-1 3 12 50 104014 25 0.000 17 8
adrestia teo-1 4 11 38 103388 12 0.000 11 1
adrestia teo-2 0 12 50 103492 25 0.000 12 13
adrestia teo-2 1 12 50 103924 29 0.000 17 12
adrestia teo-2 2 12 50 103674 19 0.000 13 6
adrestia teo-2 3 12 50 104464 41 0.000 28 13
adrestia teo-2 4 12 50 103758 16 0.000 8 8
adrestia teo-3 0 12 50 103984 25 0.000 12 13
adrestia teo-3 1 11 38 103556 23 0.000 14 9
adrestia teo-3 2 12 50 104088 22 0.000 17 5
adrestia teo-3 3 12 50 104430 48 0.000 26 22
adrestia teo-3 4 12 50 103968 26 0.000 18 8
adrestia teo-4 0 12 50 103924 19 0.000 14 5
adrestia teo-4 1 8 0 103228 14 0.000 10 4
adrestia teo-4 2 12 50 104124 15 0.000 10 5
adrestia teo-4 3 12 50 104472 22 0.000 16 6
adrestia teo-4 4 12 50 103554 30 0.000 16 14
adrestia teo-6-6 0 12 50 102832 61 0.001 51 10
adrestia teo-6-6 1 12 50 103136 52 0.001 38 14
adrestia teo-6-6 2 12 50 102882 37 0.000 29 8
adrestia teo-6-6 3 12 50 103102 51 0.000 40 11
adrestia teo-6-6 4 12 50 103260 60 0.001 43 17
adrestia teo-6-12 0 12 50 112320 0 0.000 43 17
adrestia teo-6-12 1 12 50 114412 0 0.000 43 17
adrestia teo-6-12 2 11 38 112220 0 0.000 43 17
adrestia teo-6-12 3 12 50 112028 0 0.000 43 17
adrestia teo-6-12 4 12 50 112750 0 0.000 43 17
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v1 0/4] cpuidle: governors: teo: Assorted improvements
2025-11-13 15:21 ` [PATCH v1 0/4] cpuidle: governors: teo: Assorted improvements Christian Loehle
@ 2025-11-13 15:25 ` Rafael J. Wysocki
2025-11-19 22:52 ` Doug Smythies
1 sibling, 0 replies; 26+ messages in thread
From: Rafael J. Wysocki @ 2025-11-13 15:25 UTC (permalink / raw)
To: Christian Loehle; +Cc: Rafael J. Wysocki, Linux PM, LKML, Reka Norman
On Thu, Nov 13, 2025 at 4:21 PM Christian Loehle
<christian.loehle@arm.com> wrote:
>
> On 11/12/25 16:21, Rafael J. Wysocki wrote:
> > Hi,
> >
> > This is a bunch of teo cpuidle governor improvements, some of which are related
> > to a bug report discussed recently:
> >
> > https://lore.kernel.org/linux-pm/CAEmPcwsNMNnNXuxgvHTQ93Mx-q3Oz9U57THQsU_qdcCx1m4w5g@mail.gmail.com/
> >
> > The first patch fixes a bug that may cause an overly deep idle state
> > to be selected when the scheduler tick has been already stopped.
> >
> > Patch [2/4] removes an unnecessary function argument.
> >
> > Patch [3/4] makes teo_update() to use s64 as the data type for its local
> > variables more consistently.
> >
> > The last patch reworks the governor's decay implementation to also decay
> > metric values lower than 8.
> >
>
> Tested-by: Christian Loehle <christian.loehle@arm.com>
Thank you!
> Test results below, although there really isn't anything interesting in there.
> teo-1 to teo-4 (patches 1 to 4 respectively are essentially indistinguishable from
> teo-m = mainline)
Good, that's how it should be.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 1/4] cpuidle: governors: teo: Drop misguided target residency check
2025-11-13 13:24 ` [PATCH v2 1/4] cpuidle: governors: teo: Drop misguided " Rafael J. Wysocki
@ 2025-11-14 9:16 ` Christian Loehle
0 siblings, 0 replies; 26+ messages in thread
From: Christian Loehle @ 2025-11-14 9:16 UTC (permalink / raw)
To: Rafael J. Wysocki, Linux PM; +Cc: LKML, Reka Norman
On 11/13/25 13:24, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> When the target residency of the current candidate idle state is
> greater than the expected time till the closest timer (the sleep
> length), it does not matter whether or not the tick has already been
> stopped or if it is going to be stopped. The closest timer will
> trigger anyway at its due time, so if an idle state with target
> residency above the sleep length is selected, energy will be wasted
> and there may be excess latency.
>
> Of course, if the closest timer were canceled before it could trigger,
> a deeper idle state would be more suitable, but this is not expected
> to happen (generally speaking, hrtimers are not expected to be
> canceled as a rule).
>
> Accordingly, the teo_state_ok() check done in that case causes energy to
> be wasted more often than it allows any energy to be saved (if it allows
> any energy to be saved at all), so drop it and let the governor use the
> teo_find_shallower_state() return value as the new candidate idle state
> index.
>
> Fixes: 21d28cd2fa5f ("cpuidle: teo: Do not call tick_nohz_get_sleep_length() upfront")
> Cc: All applicable <stable@vger.kernel.org>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>
> v1 -> v2: Subject and changelog modifications
>
> ---
> drivers/cpuidle/governors/teo.c | 7 ++-----
> 1 file changed, 2 insertions(+), 5 deletions(-)
>
> --- a/drivers/cpuidle/governors/teo.c
> +++ b/drivers/cpuidle/governors/teo.c
> @@ -458,11 +458,8 @@ static int teo_select(struct cpuidle_dri
> * If the closest expected timer is before the target residency of the
> * candidate state, a shallower one needs to be found.
> */
> - if (drv->states[idx].target_residency_ns > duration_ns) {
> - i = teo_find_shallower_state(drv, dev, idx, duration_ns, false);
> - if (teo_state_ok(i, drv))
> - idx = i;
> - }
> + if (drv->states[idx].target_residency_ns > duration_ns)
> + idx = teo_find_shallower_state(drv, dev, idx, duration_ns, false);
>
> /*
> * If the selected state's target residency is below the tick length
>
Reviewed-by: Christian Loehle <christian.loehle@arm.com>
^ permalink raw reply [flat|nested] 26+ messages in thread
* RE: [PATCH v1 0/4] cpuidle: governors: teo: Assorted improvements
2025-11-13 15:21 ` [PATCH v1 0/4] cpuidle: governors: teo: Assorted improvements Christian Loehle
2025-11-13 15:25 ` Rafael J. Wysocki
@ 2025-11-19 22:52 ` Doug Smythies
2025-11-20 11:02 ` Rafael J. Wysocki
1 sibling, 1 reply; 26+ messages in thread
From: Doug Smythies @ 2025-11-19 22:52 UTC (permalink / raw)
To: 'Christian Loehle', 'Rafael J. Wysocki',
'Linux PM'
Cc: 'LKML', 'Reka Norman', Doug Smythies,
'Marcelo Tosatti'
On 2025.11.13 07:22 Christian Loehle wrote:
> On 11/12/25 16:21, Rafael J. Wysocki wrote:
>> Hi,
>>
>> This is a bunch of teo cpuidle governor improvements, some of which are related
>> to a bug report discussed recently:
>>
>> https://lore.kernel.org/linux-pm/CAEmPcwsNMNnNXuxgvHTQ93Mx-q3Oz9U57THQsU_qdcCx1m4w5g@mail.gmail.com/
>>
>> The first patch fixes a bug that may cause an overly deep idle state
>> to be selected when the scheduler tick has been already stopped.
>>
>> Patch [2/4] removes an unnecessary function argument.
>>
>> Patch [3/4] makes teo_update() to use s64 as the data type for its local
>> variables more consistently.
>>
>> The last patch reworks the governor's decay implementation to also decay
>> metric values lower than 8.
>>
>
> Tested-by: Christian Loehle <christian.loehle@arm.com>
>
> Test results below, although there really isn't anything interesting in there.
> teo-1 to teo-4 (patches 1 to 4 respectively are essentially indistinguishable from
> teo-m = mainline)
I tested the 4 patch set also, and also found no differences in results above
repeatability noise levels.
Additionally, I added another patch (patch 5 of 4):
"cpuidle: governors: teo: Rework the handling of tick wakeups" [1]
Similar findings.
Additionally, I added another patch (patch 6 of 4):
"sched/idle: disable tick in idle=poll idle entry" [2]
And found only one significant improvement, for only one test,
but only for the TEO idle governor:
Kernel 6.18-rc4:
For a 6 pair fast ping-pong test (meaning no work per token stop):
teo: 5.53 uSec per loop, reference test
4 of 4 patches: 5.53 uSec per loop, 0%
5 of 4 patches: 5.54 uSec per loop, 0.2% (noise)
6 of 4 patches: 4.77 uSec per loop, 13% better
6 of 4 patches (again): 4.81 uSec per loop, 13% better
menu: 5.29 uSec per loop, 4.4% better
menu + patch 6 of 4: 5.28 uSec per loop, 4.5% better
Idle state 0 usage:
18% with patch 6, teo
11% with menu
~1% with mainline and not patch 6, teo.
Idle state 1 usage:
almost 0 with patch 6, teo
~6% with menu
27% with mainline and not patch 6, teo.
Power: About 100 watts. Patch 6 and teo does increase power use by about a watt or 2.
Processor: Intel(R) Core(TM) i5-10600K CPU @ 4.10GHz, 6 cores 12 CPUs.
For clarity my branch log:
3993913d7f81 (HEAD -> rjw-teo) sched/idle: disable tick in idle=poll idle entry
d9b12b8d62bf cpuidle: governors: teo: Rework the handling of tick wakeups
e47178c87272 cpuidle: governors: teo: Decay metrics below DECAY_SHIFT threshold
7fe32e411c2b cpuidle: governors: teo: Use s64 consistently in teo_update()
490e6118e45d cpuidle: governors: teo: Drop redundant function parameter
8f627f86062e cpuidle: governors: teo: Drop incorrect target residency check
6146a0f1dfae (tag: v6.18-rc4, origin/master, origin/HEAD, master) Linux 6.18-rc4
[1] https://lore.kernel.org/linux-pm/6228387.lOV4Wx5bFT@rafael.j.wysocki/
[2] https://lore.kernel.org/linux-pm/aQiWfnnSzxsnwa2o@tpad/
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v1 0/4] cpuidle: governors: teo: Assorted improvements
2025-11-19 22:52 ` Doug Smythies
@ 2025-11-20 11:02 ` Rafael J. Wysocki
2025-11-20 13:35 ` Christian Loehle
0 siblings, 1 reply; 26+ messages in thread
From: Rafael J. Wysocki @ 2025-11-20 11:02 UTC (permalink / raw)
To: Doug Smythies
Cc: Christian Loehle, Rafael J. Wysocki, Linux PM, LKML, Reka Norman,
Marcelo Tosatti
On Wed, Nov 19, 2025 at 11:52 PM Doug Smythies <dsmythies@telus.net> wrote:
>
> On 2025.11.13 07:22 Christian Loehle wrote:
> > On 11/12/25 16:21, Rafael J. Wysocki wrote:
> >> Hi,
> >>
> >> This is a bunch of teo cpuidle governor improvements, some of which are related
> >> to a bug report discussed recently:
> >>
> >> https://lore.kernel.org/linux-pm/CAEmPcwsNMNnNXuxgvHTQ93Mx-q3Oz9U57THQsU_qdcCx1m4w5g@mail.gmail.com/
> >>
> >> The first patch fixes a bug that may cause an overly deep idle state
> >> to be selected when the scheduler tick has been already stopped.
> >>
> >> Patch [2/4] removes an unnecessary function argument.
> >>
> >> Patch [3/4] makes teo_update() to use s64 as the data type for its local
> >> variables more consistently.
> >>
> >> The last patch reworks the governor's decay implementation to also decay
> >> metric values lower than 8.
> >>
> >
> > Tested-by: Christian Loehle <christian.loehle@arm.com>
> >
> > Test results below, although there really isn't anything interesting in there.
> > teo-1 to teo-4 (patches 1 to 4 respectively are essentially indistinguishable from
> > teo-m = mainline)
>
> I tested the 4 patch set also, and also found no differences in results above
> repeatability noise levels.
>
> Additionally, I added another patch (patch 5 of 4):
> "cpuidle: governors: teo: Rework the handling of tick wakeups" [1]
> Similar findings.
>
> Additionally, I added another patch (patch 6 of 4):
> "sched/idle: disable tick in idle=poll idle entry" [2]
> And found only one significant improvement, for only one test,
> but only for the TEO idle governor:
>
> Kernel 6.18-rc4:
> For a 6 pair fast ping-pong test (meaning no work per token stop):
> teo: 5.53 uSec per loop, reference test
> 4 of 4 patches: 5.53 uSec per loop, 0%
> 5 of 4 patches: 5.54 uSec per loop, 0.2% (noise)
> 6 of 4 patches: 4.77 uSec per loop, 13% better
> 6 of 4 patches (again): 4.81 uSec per loop, 13% better
> menu: 5.29 uSec per loop, 4.4% better
> menu + patch 6 of 4: 5.28 uSec per loop, 4.5% better
>
> Idle state 0 usage:
> 18% with patch 6, teo
> 11% with menu
> ~1% with mainline and not patch 6, teo.
>
> Idle state 1 usage:
> almost 0 with patch 6, teo
> ~6% with menu
> 27% with mainline and not patch 6, teo.
>
> Power: About 100 watts. Patch 6 and teo does increase power use by about a watt or 2.
>
> Processor: Intel(R) Core(TM) i5-10600K CPU @ 4.10GHz, 6 cores 12 CPUs.
>
> For clarity my branch log:
> 3993913d7f81 (HEAD -> rjw-teo) sched/idle: disable tick in idle=poll idle entry
> d9b12b8d62bf cpuidle: governors: teo: Rework the handling of tick wakeups
> e47178c87272 cpuidle: governors: teo: Decay metrics below DECAY_SHIFT threshold
> 7fe32e411c2b cpuidle: governors: teo: Use s64 consistently in teo_update()
> 490e6118e45d cpuidle: governors: teo: Drop redundant function parameter
> 8f627f86062e cpuidle: governors: teo: Drop incorrect target residency check
> 6146a0f1dfae (tag: v6.18-rc4, origin/master, origin/HEAD, master) Linux 6.18-rc4
>
> [1] https://lore.kernel.org/linux-pm/6228387.lOV4Wx5bFT@rafael.j.wysocki/
> [2] https://lore.kernel.org/linux-pm/aQiWfnnSzxsnwa2o@tpad/
Thanks for the feedback, much appreciated!
I will likely have some more teo updates in the next cycle.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v1 0/4] cpuidle: governors: teo: Assorted improvements
2025-11-20 11:02 ` Rafael J. Wysocki
@ 2025-11-20 13:35 ` Christian Loehle
2025-11-20 13:38 ` Rafael J. Wysocki
0 siblings, 1 reply; 26+ messages in thread
From: Christian Loehle @ 2025-11-20 13:35 UTC (permalink / raw)
To: Rafael J. Wysocki, Doug Smythies
Cc: Linux PM, LKML, Reka Norman, Marcelo Tosatti
On 11/20/25 11:02, Rafael J. Wysocki wrote:
> On Wed, Nov 19, 2025 at 11:52 PM Doug Smythies <dsmythies@telus.net> wrote:
>>
>> On 2025.11.13 07:22 Christian Loehle wrote:
>>> On 11/12/25 16:21, Rafael J. Wysocki wrote:
>>>> Hi,
>>>>
>>>> This is a bunch of teo cpuidle governor improvements, some of which are related
>>>> to a bug report discussed recently:
>>>>
>>>> https://lore.kernel.org/linux-pm/CAEmPcwsNMNnNXuxgvHTQ93Mx-q3Oz9U57THQsU_qdcCx1m4w5g@mail.gmail.com/
>>>>
>>>> The first patch fixes a bug that may cause an overly deep idle state
>>>> to be selected when the scheduler tick has been already stopped.
>>>>
>>>> Patch [2/4] removes an unnecessary function argument.
>>>>
>>>> Patch [3/4] makes teo_update() to use s64 as the data type for its local
>>>> variables more consistently.
>>>>
>>>> The last patch reworks the governor's decay implementation to also decay
>>>> metric values lower than 8.
>>>>
>>>
>>> Tested-by: Christian Loehle <christian.loehle@arm.com>
>>>
>>> Test results below, although there really isn't anything interesting in there.
>>> teo-1 to teo-4 (patches 1 to 4 respectively are essentially indistinguishable from
>>> teo-m = mainline)
>>
>> I tested the 4 patch set also, and also found no differences in results above
>> repeatability noise levels.
>>
>> Additionally, I added another patch (patch 5 of 4):
>> "cpuidle: governors: teo: Rework the handling of tick wakeups" [1]
>> Similar findings.
>>
>> Additionally, I added another patch (patch 6 of 4):
>> "sched/idle: disable tick in idle=poll idle entry" [2]
>> And found only one significant improvement, for only one test,
>> but only for the TEO idle governor:
>>
>> Kernel 6.18-rc4:
>> For a 6 pair fast ping-pong test (meaning no work per token stop):
>> teo: 5.53 uSec per loop, reference test
>> 4 of 4 patches: 5.53 uSec per loop, 0%
>> 5 of 4 patches: 5.54 uSec per loop, 0.2% (noise)
>> 6 of 4 patches: 4.77 uSec per loop, 13% better
>> 6 of 4 patches (again): 4.81 uSec per loop, 13% better
>> menu: 5.29 uSec per loop, 4.4% better
>> menu + patch 6 of 4: 5.28 uSec per loop, 4.5% better
>>
>> Idle state 0 usage:
>> 18% with patch 6, teo
>> 11% with menu
>> ~1% with mainline and not patch 6, teo.
>>
>> Idle state 1 usage:
>> almost 0 with patch 6, teo
>> ~6% with menu
>> 27% with mainline and not patch 6, teo.
>>
>> Power: About 100 watts. Patch 6 and teo does increase power use by about a watt or 2.
>>
>> Processor: Intel(R) Core(TM) i5-10600K CPU @ 4.10GHz, 6 cores 12 CPUs.
>>
>> For clarity my branch log:
>> 3993913d7f81 (HEAD -> rjw-teo) sched/idle: disable tick in idle=poll idle entry
>> d9b12b8d62bf cpuidle: governors: teo: Rework the handling of tick wakeups
>> e47178c87272 cpuidle: governors: teo: Decay metrics below DECAY_SHIFT threshold
>> 7fe32e411c2b cpuidle: governors: teo: Use s64 consistently in teo_update()
>> 490e6118e45d cpuidle: governors: teo: Drop redundant function parameter
>> 8f627f86062e cpuidle: governors: teo: Drop incorrect target residency check
>> 6146a0f1dfae (tag: v6.18-rc4, origin/master, origin/HEAD, master) Linux 6.18-rc4
>>
>> [1] https://lore.kernel.org/linux-pm/6228387.lOV4Wx5bFT@rafael.j.wysocki/
>> [2] https://lore.kernel.org/linux-pm/aQiWfnnSzxsnwa2o@tpad/
>
> Thanks for the feedback, much appreciated!
>
> I will likely have some more teo updates in the next cycle.
You're welcome, looking forward to reviewing them too.
I haven't tried to see what this would ideally look like for the -stable branches.
Just backport everything until the most recent applicable Fixes:?
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v1 0/4] cpuidle: governors: teo: Assorted improvements
2025-11-20 13:35 ` Christian Loehle
@ 2025-11-20 13:38 ` Rafael J. Wysocki
2025-11-20 13:57 ` Rafael J. Wysocki
0 siblings, 1 reply; 26+ messages in thread
From: Rafael J. Wysocki @ 2025-11-20 13:38 UTC (permalink / raw)
To: Christian Loehle
Cc: Rafael J. Wysocki, Doug Smythies, Linux PM, LKML, Reka Norman,
Marcelo Tosatti
On Thu, Nov 20, 2025 at 2:35 PM Christian Loehle
<christian.loehle@arm.com> wrote:
>
> On 11/20/25 11:02, Rafael J. Wysocki wrote:
> > On Wed, Nov 19, 2025 at 11:52 PM Doug Smythies <dsmythies@telus.net> wrote:
> >>
> >> On 2025.11.13 07:22 Christian Loehle wrote:
> >>> On 11/12/25 16:21, Rafael J. Wysocki wrote:
> >>>> Hi,
> >>>>
> >>>> This is a bunch of teo cpuidle governor improvements, some of which are related
> >>>> to a bug report discussed recently:
> >>>>
> >>>> https://lore.kernel.org/linux-pm/CAEmPcwsNMNnNXuxgvHTQ93Mx-q3Oz9U57THQsU_qdcCx1m4w5g@mail.gmail.com/
> >>>>
> >>>> The first patch fixes a bug that may cause an overly deep idle state
> >>>> to be selected when the scheduler tick has been already stopped.
> >>>>
> >>>> Patch [2/4] removes an unnecessary function argument.
> >>>>
> >>>> Patch [3/4] makes teo_update() to use s64 as the data type for its local
> >>>> variables more consistently.
> >>>>
> >>>> The last patch reworks the governor's decay implementation to also decay
> >>>> metric values lower than 8.
> >>>>
> >>>
> >>> Tested-by: Christian Loehle <christian.loehle@arm.com>
> >>>
> >>> Test results below, although there really isn't anything interesting in there.
> >>> teo-1 to teo-4 (patches 1 to 4 respectively are essentially indistinguishable from
> >>> teo-m = mainline)
> >>
> >> I tested the 4 patch set also, and also found no differences in results above
> >> repeatability noise levels.
> >>
> >> Additionally, I added another patch (patch 5 of 4):
> >> "cpuidle: governors: teo: Rework the handling of tick wakeups" [1]
> >> Similar findings.
> >>
> >> Additionally, I added another patch (patch 6 of 4):
> >> "sched/idle: disable tick in idle=poll idle entry" [2]
> >> And found only one significant improvement, for only one test,
> >> but only for the TEO idle governor:
> >>
> >> Kernel 6.18-rc4:
> >> For a 6 pair fast ping-pong test (meaning no work per token stop):
> >> teo: 5.53 uSec per loop, reference test
> >> 4 of 4 patches: 5.53 uSec per loop, 0%
> >> 5 of 4 patches: 5.54 uSec per loop, 0.2% (noise)
> >> 6 of 4 patches: 4.77 uSec per loop, 13% better
> >> 6 of 4 patches (again): 4.81 uSec per loop, 13% better
> >> menu: 5.29 uSec per loop, 4.4% better
> >> menu + patch 6 of 4: 5.28 uSec per loop, 4.5% better
> >>
> >> Idle state 0 usage:
> >> 18% with patch 6, teo
> >> 11% with menu
> >> ~1% with mainline and not patch 6, teo.
> >>
> >> Idle state 1 usage:
> >> almost 0 with patch 6, teo
> >> ~6% with menu
> >> 27% with mainline and not patch 6, teo.
> >>
> >> Power: About 100 watts. Patch 6 and teo does increase power use by about a watt or 2.
> >>
> >> Processor: Intel(R) Core(TM) i5-10600K CPU @ 4.10GHz, 6 cores 12 CPUs.
> >>
> >> For clarity my branch log:
> >> 3993913d7f81 (HEAD -> rjw-teo) sched/idle: disable tick in idle=poll idle entry
> >> d9b12b8d62bf cpuidle: governors: teo: Rework the handling of tick wakeups
> >> e47178c87272 cpuidle: governors: teo: Decay metrics below DECAY_SHIFT threshold
> >> 7fe32e411c2b cpuidle: governors: teo: Use s64 consistently in teo_update()
> >> 490e6118e45d cpuidle: governors: teo: Drop redundant function parameter
> >> 8f627f86062e cpuidle: governors: teo: Drop incorrect target residency check
> >> 6146a0f1dfae (tag: v6.18-rc4, origin/master, origin/HEAD, master) Linux 6.18-rc4
> >>
> >> [1] https://lore.kernel.org/linux-pm/6228387.lOV4Wx5bFT@rafael.j.wysocki/
> >> [2] https://lore.kernel.org/linux-pm/aQiWfnnSzxsnwa2o@tpad/
> >
> > Thanks for the feedback, much appreciated!
> >
> > I will likely have some more teo updates in the next cycle.
>
> You're welcome, looking forward to reviewing them too.
> I haven't tried to see what this would ideally look like for the -stable branches.
> Just backport everything until the most recent applicable Fixes:?
I've added a list to this commit:
https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git/commit/?h=linux-next&id=14c66155c4609f1a1207d4e716c5e722b8bf920e
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v1 0/4] cpuidle: governors: teo: Assorted improvements
2025-11-20 13:38 ` Rafael J. Wysocki
@ 2025-11-20 13:57 ` Rafael J. Wysocki
2025-11-20 15:21 ` Rafael J. Wysocki
0 siblings, 1 reply; 26+ messages in thread
From: Rafael J. Wysocki @ 2025-11-20 13:57 UTC (permalink / raw)
To: Christian Loehle
Cc: Doug Smythies, Linux PM, LKML, Reka Norman, Marcelo Tosatti
On Thu, Nov 20, 2025 at 2:38 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
>
> On Thu, Nov 20, 2025 at 2:35 PM Christian Loehle
> <christian.loehle@arm.com> wrote:
> >
> > On 11/20/25 11:02, Rafael J. Wysocki wrote:
> > > On Wed, Nov 19, 2025 at 11:52 PM Doug Smythies <dsmythies@telus.net> wrote:
> > >>
> > >> On 2025.11.13 07:22 Christian Loehle wrote:
> > >>> On 11/12/25 16:21, Rafael J. Wysocki wrote:
> > >>>> Hi,
> > >>>>
> > >>>> This is a bunch of teo cpuidle governor improvements, some of which are related
> > >>>> to a bug report discussed recently:
> > >>>>
> > >>>> https://lore.kernel.org/linux-pm/CAEmPcwsNMNnNXuxgvHTQ93Mx-q3Oz9U57THQsU_qdcCx1m4w5g@mail.gmail.com/
> > >>>>
> > >>>> The first patch fixes a bug that may cause an overly deep idle state
> > >>>> to be selected when the scheduler tick has been already stopped.
> > >>>>
> > >>>> Patch [2/4] removes an unnecessary function argument.
> > >>>>
> > >>>> Patch [3/4] makes teo_update() to use s64 as the data type for its local
> > >>>> variables more consistently.
> > >>>>
> > >>>> The last patch reworks the governor's decay implementation to also decay
> > >>>> metric values lower than 8.
> > >>>>
> > >>>
> > >>> Tested-by: Christian Loehle <christian.loehle@arm.com>
> > >>>
> > >>> Test results below, although there really isn't anything interesting in there.
> > >>> teo-1 to teo-4 (patches 1 to 4 respectively are essentially indistinguishable from
> > >>> teo-m = mainline)
> > >>
> > >> I tested the 4 patch set also, and also found no differences in results above
> > >> repeatability noise levels.
> > >>
> > >> Additionally, I added another patch (patch 5 of 4):
> > >> "cpuidle: governors: teo: Rework the handling of tick wakeups" [1]
> > >> Similar findings.
> > >>
> > >> Additionally, I added another patch (patch 6 of 4):
> > >> "sched/idle: disable tick in idle=poll idle entry" [2]
> > >> And found only one significant improvement, for only one test,
> > >> but only for the TEO idle governor:
> > >>
> > >> Kernel 6.18-rc4:
> > >> For a 6 pair fast ping-pong test (meaning no work per token stop):
> > >> teo: 5.53 uSec per loop, reference test
> > >> 4 of 4 patches: 5.53 uSec per loop, 0%
> > >> 5 of 4 patches: 5.54 uSec per loop, 0.2% (noise)
> > >> 6 of 4 patches: 4.77 uSec per loop, 13% better
> > >> 6 of 4 patches (again): 4.81 uSec per loop, 13% better
> > >> menu: 5.29 uSec per loop, 4.4% better
> > >> menu + patch 6 of 4: 5.28 uSec per loop, 4.5% better
> > >>
> > >> Idle state 0 usage:
> > >> 18% with patch 6, teo
> > >> 11% with menu
> > >> ~1% with mainline and not patch 6, teo.
> > >>
> > >> Idle state 1 usage:
> > >> almost 0 with patch 6, teo
> > >> ~6% with menu
> > >> 27% with mainline and not patch 6, teo.
> > >>
> > >> Power: About 100 watts. Patch 6 and teo does increase power use by about a watt or 2.
> > >>
> > >> Processor: Intel(R) Core(TM) i5-10600K CPU @ 4.10GHz, 6 cores 12 CPUs.
> > >>
> > >> For clarity my branch log:
> > >> 3993913d7f81 (HEAD -> rjw-teo) sched/idle: disable tick in idle=poll idle entry
> > >> d9b12b8d62bf cpuidle: governors: teo: Rework the handling of tick wakeups
> > >> e47178c87272 cpuidle: governors: teo: Decay metrics below DECAY_SHIFT threshold
> > >> 7fe32e411c2b cpuidle: governors: teo: Use s64 consistently in teo_update()
> > >> 490e6118e45d cpuidle: governors: teo: Drop redundant function parameter
> > >> 8f627f86062e cpuidle: governors: teo: Drop incorrect target residency check
> > >> 6146a0f1dfae (tag: v6.18-rc4, origin/master, origin/HEAD, master) Linux 6.18-rc4
> > >>
> > >> [1] https://lore.kernel.org/linux-pm/6228387.lOV4Wx5bFT@rafael.j.wysocki/
> > >> [2] https://lore.kernel.org/linux-pm/aQiWfnnSzxsnwa2o@tpad/
> > >
> > > Thanks for the feedback, much appreciated!
> > >
> > > I will likely have some more teo updates in the next cycle.
> >
> > You're welcome, looking forward to reviewing them too.
> > I haven't tried to see what this would ideally look like for the -stable branches.
> > Just backport everything until the most recent applicable Fixes:?
>
> I've added a list to this commit:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git/commit/?h=linux-next&id=14c66155c4609f1a1207d4e716c5e722b8bf920e
Which somehow got incorrect git commit hashes, so I need to regenerate
it. Sorry for the confusion.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v1 0/4] cpuidle: governors: teo: Assorted improvements
2025-11-20 13:57 ` Rafael J. Wysocki
@ 2025-11-20 15:21 ` Rafael J. Wysocki
0 siblings, 0 replies; 26+ messages in thread
From: Rafael J. Wysocki @ 2025-11-20 15:21 UTC (permalink / raw)
To: Christian Loehle
Cc: Doug Smythies, Linux PM, LKML, Reka Norman, Marcelo Tosatti
On Thu, Nov 20, 2025 at 2:57 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
>
> On Thu, Nov 20, 2025 at 2:38 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
> >
> > On Thu, Nov 20, 2025 at 2:35 PM Christian Loehle
> > <christian.loehle@arm.com> wrote:
> > >
> > > On 11/20/25 11:02, Rafael J. Wysocki wrote:
> > > > On Wed, Nov 19, 2025 at 11:52 PM Doug Smythies <dsmythies@telus.net> wrote:
> > > >>
> > > >> On 2025.11.13 07:22 Christian Loehle wrote:
> > > >>> On 11/12/25 16:21, Rafael J. Wysocki wrote:
> > > >>>> Hi,
> > > >>>>
> > > >>>> This is a bunch of teo cpuidle governor improvements, some of which are related
> > > >>>> to a bug report discussed recently:
> > > >>>>
> > > >>>> https://lore.kernel.org/linux-pm/CAEmPcwsNMNnNXuxgvHTQ93Mx-q3Oz9U57THQsU_qdcCx1m4w5g@mail.gmail.com/
> > > >>>>
> > > >>>> The first patch fixes a bug that may cause an overly deep idle state
> > > >>>> to be selected when the scheduler tick has been already stopped.
> > > >>>>
> > > >>>> Patch [2/4] removes an unnecessary function argument.
> > > >>>>
> > > >>>> Patch [3/4] makes teo_update() to use s64 as the data type for its local
> > > >>>> variables more consistently.
> > > >>>>
> > > >>>> The last patch reworks the governor's decay implementation to also decay
> > > >>>> metric values lower than 8.
> > > >>>>
> > > >>>
> > > >>> Tested-by: Christian Loehle <christian.loehle@arm.com>
> > > >>>
> > > >>> Test results below, although there really isn't anything interesting in there.
> > > >>> teo-1 to teo-4 (patches 1 to 4 respectively are essentially indistinguishable from
> > > >>> teo-m = mainline)
> > > >>
> > > >> I tested the 4 patch set also, and also found no differences in results above
> > > >> repeatability noise levels.
> > > >>
> > > >> Additionally, I added another patch (patch 5 of 4):
> > > >> "cpuidle: governors: teo: Rework the handling of tick wakeups" [1]
> > > >> Similar findings.
> > > >>
> > > >> Additionally, I added another patch (patch 6 of 4):
> > > >> "sched/idle: disable tick in idle=poll idle entry" [2]
> > > >> And found only one significant improvement, for only one test,
> > > >> but only for the TEO idle governor:
> > > >>
> > > >> Kernel 6.18-rc4:
> > > >> For a 6 pair fast ping-pong test (meaning no work per token stop):
> > > >> teo: 5.53 uSec per loop, reference test
> > > >> 4 of 4 patches: 5.53 uSec per loop, 0%
> > > >> 5 of 4 patches: 5.54 uSec per loop, 0.2% (noise)
> > > >> 6 of 4 patches: 4.77 uSec per loop, 13% better
> > > >> 6 of 4 patches (again): 4.81 uSec per loop, 13% better
> > > >> menu: 5.29 uSec per loop, 4.4% better
> > > >> menu + patch 6 of 4: 5.28 uSec per loop, 4.5% better
> > > >>
> > > >> Idle state 0 usage:
> > > >> 18% with patch 6, teo
> > > >> 11% with menu
> > > >> ~1% with mainline and not patch 6, teo.
> > > >>
> > > >> Idle state 1 usage:
> > > >> almost 0 with patch 6, teo
> > > >> ~6% with menu
> > > >> 27% with mainline and not patch 6, teo.
> > > >>
> > > >> Power: About 100 watts. Patch 6 and teo does increase power use by about a watt or 2.
> > > >>
> > > >> Processor: Intel(R) Core(TM) i5-10600K CPU @ 4.10GHz, 6 cores 12 CPUs.
> > > >>
> > > >> For clarity my branch log:
> > > >> 3993913d7f81 (HEAD -> rjw-teo) sched/idle: disable tick in idle=poll idle entry
> > > >> d9b12b8d62bf cpuidle: governors: teo: Rework the handling of tick wakeups
> > > >> e47178c87272 cpuidle: governors: teo: Decay metrics below DECAY_SHIFT threshold
> > > >> 7fe32e411c2b cpuidle: governors: teo: Use s64 consistently in teo_update()
> > > >> 490e6118e45d cpuidle: governors: teo: Drop redundant function parameter
> > > >> 8f627f86062e cpuidle: governors: teo: Drop incorrect target residency check
> > > >> 6146a0f1dfae (tag: v6.18-rc4, origin/master, origin/HEAD, master) Linux 6.18-rc4
> > > >>
> > > >> [1] https://lore.kernel.org/linux-pm/6228387.lOV4Wx5bFT@rafael.j.wysocki/
> > > >> [2] https://lore.kernel.org/linux-pm/aQiWfnnSzxsnwa2o@tpad/
> > > >
> > > > Thanks for the feedback, much appreciated!
> > > >
> > > > I will likely have some more teo updates in the next cycle.
> > >
> > > You're welcome, looking forward to reviewing them too.
> > > I haven't tried to see what this would ideally look like for the -stable branches.
> > > Just backport everything until the most recent applicable Fixes:?
> >
> > I've added a list to this commit:
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git/commit/?h=linux-next&id=14c66155c4609f1a1207d4e716c5e722b8bf920e
>
> Which somehow got incorrect git commit hashes, so I need to regenerate
> it. Sorry for the confusion.
Done, and it's this commit now:
https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git/commit/?h=linux-next&id=083654ded547238c70e0d4f57115cd1c91245b6e
^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2025-11-20 15:22 UTC | newest]
Thread overview: 26+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-12 16:21 [PATCH v1 0/4] cpuidle: governors: teo: Assorted improvements Rafael J. Wysocki
2025-11-12 16:22 ` [PATCH v1 1/4] cpuidle: governors: teo: Drop incorrect target residency check Rafael J. Wysocki
2025-11-13 11:32 ` Christian Loehle
2025-11-13 11:41 ` Rafael J. Wysocki
2025-11-13 11:47 ` Rafael J. Wysocki
2025-11-13 13:26 ` Rafael J. Wysocki
2025-11-13 13:24 ` [PATCH v2 1/4] cpuidle: governors: teo: Drop misguided " Rafael J. Wysocki
2025-11-14 9:16 ` Christian Loehle
2025-11-12 16:23 ` [PATCH v1 2/4] cpuidle: governors: teo: Drop redundant function parameter Rafael J. Wysocki
2025-11-13 11:46 ` Christian Loehle
2025-11-12 16:24 ` [PATCH v1 3/4] cpuidle: governors: teo: Use s64 consistently in teo_update() Rafael J. Wysocki
2025-11-13 11:48 ` Christian Loehle
2025-11-12 16:25 ` [PATCH v1 4/4] cpuidle: governors: teo: Decay metrics below DECAY_SHIFT threshold Rafael J. Wysocki
2025-11-12 17:29 ` Christian Loehle
2025-11-12 17:51 ` Rafael J. Wysocki
2025-11-12 18:00 ` Christian Loehle
2025-11-12 18:03 ` [PATCH v2 " Rafael J. Wysocki
2025-11-13 11:49 ` Christian Loehle
2025-11-13 15:21 ` [PATCH v1 0/4] cpuidle: governors: teo: Assorted improvements Christian Loehle
2025-11-13 15:25 ` Rafael J. Wysocki
2025-11-19 22:52 ` Doug Smythies
2025-11-20 11:02 ` Rafael J. Wysocki
2025-11-20 13:35 ` Christian Loehle
2025-11-20 13:38 ` Rafael J. Wysocki
2025-11-20 13:57 ` Rafael J. Wysocki
2025-11-20 15:21 ` Rafael J. Wysocki
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox