| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAJZ5v0jYxcsBv2OJdkJcvJK_HZDxuUEUq75Dg7LdvWcU0fV5tA@mail.gmail.com>
Date: Thu, 20 Nov 2025 12:07:19 +0100
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Christian Loehle <christian.loehle@....com>
Cc: "Rafael J. Wysocki" <rafael@...nel.org>, Linux PM <linux-pm@...r.kernel.org>,
LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v1 2/2] cpuidle: governors: teo: Simplify intercepts-based
state lookup
On Thu, Nov 20, 2025 at 9:45 AM Christian Loehle
<christian.loehle@....com> wrote:
>
> On 11/16/25 12:35, Rafael J. Wysocki 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.
>
> 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.
I'll figure out something suitable.
> >
> > 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.
> > + */
>
> NIT: s/not below/above
> or just use >= in the comment?
Well, I can just say "equal to or greater than" or "at least equal to"
(slightly preferred).
> > + 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!
No worries.
> Reviewed-by: Christian Loehle <christian.loehle@....com>
Thanks!
Powered by blists - more mailing lists