Message-ID: <CAJZ5v0jpaNcTyzviv2qSNUT5nvDV2wjw9OC4ZAUtUz8YJFhkKg@mail.gmail.com>
Date: Wed, 19 Nov 2025 22:13:55 +0100
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Christian Loehle <christian.loehle@....com>, Linux PM <linux-pm@...r.kernel.org>
Cc: LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v1 2/2] cpuidle: governors: teo: Simplify intercepts-based
 state lookup

On Sun, Nov 16, 2025 at 1:35 PM Rafael J. Wysocki <rafael@...nel.org> wrote:
>
> From: Rafael J. Wysocki <rafael.j.wysocki@...el.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.
>
> 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@...el.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.
> +                        */
> +                       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;
>                 }
>         }

I'd appreciate feedback on this one.

I regard it as a significant improvement even though it is not
expected to change functionality.

Powered by blists - more mailing lists