Re: [PATCH v2 4/4] cpuidle: governors: teo: Decay metrics below DECAY_SHIFT threshold

[Christian Loehle <christian.loehle@arm.com>]

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)
> 
> 
>