[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAJZ5v0h5PkU5t=uPVO2Evq0gNrX4vYkAJFVDPLsqzCGKXnb+_w@mail.gmail.com>
Date: Thu, 13 Nov 2025 12:41:56 +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>, Reka Norman <rekanorman@...omium.org>
Subject: Re: [PATCH v1 1/4] cpuidle: governors: teo: Drop incorrect target
residency check
On Thu, Nov 13, 2025 at 12:32 PM Christian Loehle
<christian.loehle@....com> wrote:
>
> On 11/12/25 16:22, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@...el.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@...r.kernel.org>
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@...el.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.
Powered by blists - more mailing lists