Re: [PATCH v1 2/2] cpuidle: governors: teo: Simplify intercepts-based state lookup

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

On 11/16/25 12:35, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> Simplify the loop looking up a candidate idle state in the case when an
> intercept is likely to occur by adding a search for the state index limit
> if the tick is stopped before it.
> 
> First, call tick_nohz_tick_stopped() just once and if it returns true,
> look for the shallowest state index below the current candidate one with
> target residency at least equal to the tick period length.
> 
> Next, simply look for a state that is not shallower than the one found
> in the previous step and, ideally, satisfies the intercepts majority
> condition.

NIT: The ideally is a bit handwavy, maybe:
Next, look for the deepest state that satisfies the intercepts majority
condition but select no shallower state than the one from the previous step.

Sounds a bit verbose I guess.

> 
> Since teo_state_ok() has no callers any more after the above changes,
> drop it.
> 
> No intentional functional impact.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>  drivers/cpuidle/governors/teo.c |   62 ++++++++++------------------------------
>  1 file changed, 16 insertions(+), 46 deletions(-)
> 
> --- a/drivers/cpuidle/governors/teo.c
> +++ b/drivers/cpuidle/governors/teo.c
> @@ -256,12 +256,6 @@ static void teo_update(struct cpuidle_dr
>  	}
>  }
>  
> -static bool teo_state_ok(int i, struct cpuidle_driver *drv)
> -{
> -	return !tick_nohz_tick_stopped() ||
> -		drv->states[i].target_residency_ns >= TICK_NSEC;
> -}
> -
>  /**
>   * teo_find_shallower_state - Find shallower idle state matching given duration.
>   * @drv: cpuidle driver containing state data.
> @@ -383,7 +377,18 @@ static int teo_select(struct cpuidle_dri
>  	 * better choice.
>  	 */
>  	if (2 * idx_intercept_sum > cpu_data->total - idx_hit_sum) {
> -		int first_suitable_idx = idx;
> +		int min_idx = idx0;
> +
> +		if (tick_nohz_tick_stopped()) {
> +			/*
> +			 * Look for the shallowest idle state below the current
> +			 * candidate one whose target residency is not below the
> +			 * tick period length.
> +			 */

NIT: s/not below/above
or just use >= in the comment?

> +			while (min_idx < idx &&
> +			       drv->states[min_idx].target_residency_ns < TICK_NSEC)
> +				min_idx++;
> +		}
>  
>  		/*
>  		 * Look for the deepest idle state whose target residency had
> @@ -393,49 +398,14 @@ static int teo_select(struct cpuidle_dri
>  		 * Take the possible duration limitation present if the tick
>  		 * has been stopped already into account.
>  		 */
> -		intercept_sum = 0;
> -
> -		for (i = idx - 1; i >= 0; i--) {
> -			struct teo_bin *bin = &cpu_data->state_bins[i];
> -
> -			intercept_sum += bin->intercepts;
> -
> -			if (2 * intercept_sum > idx_intercept_sum) {
> -				/*
> -				 * Use the current state unless it is too
> -				 * shallow or disabled, in which case take the
> -				 * first enabled state that is deep enough.
> -				 */
> -				if (teo_state_ok(i, drv) &&
> -				    !dev->states_usage[i].disable) {
> -					idx = i;
> -					break;
> -				}
> -				idx = first_suitable_idx;
> -				break;
> -			}
> +		for (i = idx - 1, intercept_sum = 0; i >= min_idx; i--) {
> +			intercept_sum += cpu_data->state_bins[i].intercepts;
>  
>  			if (dev->states_usage[i].disable)
>  				continue;
>  
> -			if (teo_state_ok(i, drv)) {
> -				/*
> -				 * The current state is deep enough, but still
> -				 * there may be a better one.
> -				 */
> -				first_suitable_idx = i;
> -				continue;
> -			}
> -
> -			/*
> -			 * The current state is too shallow, so if no suitable
> -			 * states other than the initial candidate have been
> -			 * found, give up (the remaining states to check are
> -			 * shallower still), but otherwise the first suitable
> -			 * state other than the initial candidate may turn out
> -			 * to be preferable.
> -			 */
> -			if (first_suitable_idx == idx)
> +			idx = i;
> +			if (2 * intercept_sum > idx_intercept_sum)
>  				break;
>  		}
>  	}

Thanks, that is indeed a nice simplification. I'll get test results out on Monday,
sorry!

Reviewed-by: Christian Loehle <christian.loehle@arm.com>