Re: [patch 2/2] sched/idle: Make default_idle_call() NOHZ aware

[Qais Yousef <qyousef@layalina.io>]

On 03/10/26 16:09, Rafael J. Wysocki wrote:
> On Tue, Mar 10, 2026 at 4:03 PM Qais Yousef <qyousef@layalina.io> wrote:
> >
> > On 03/10/26 09:18, Christian Loehle wrote:
> > > On 3/10/26 03:54, Qais Yousef wrote:
> > > > On 03/07/26 17:25, Rafael J. Wysocki wrote:
> > > >
> > > >> In the meantime I realized that if the .select() governor
> > > >> callback is skipped, its .reflect() callback should be skipped
> > > >> either, so I've posted this:
> > > >>
> > > >> https://lkml.org/lkml/2026/3/7/569
> > > >>
> > > >> and here's a fixed version of the last patch on top of the above (for
> > > >> completeness):
> > > >>
> > > >> ---
> > > >>  kernel/sched/idle.c |   25 ++++++++++++++++---------
> > > >>  1 file changed, 16 insertions(+), 9 deletions(-)
> > > >>
> > > >> --- a/kernel/sched/idle.c
> > > >> +++ b/kernel/sched/idle.c
> > > >> @@ -161,6 +161,14 @@ static int call_cpuidle(struct cpuidle_d
> > > >>    return cpuidle_enter(drv, dev, next_state);
> > > >>  }
> > > >>
> > > >> +static void idle_call_stop_or_retain_tick(bool stop_tick)
> > > >> +{
> > > >> +  if (stop_tick || tick_nohz_tick_stopped())
> > > >> +          tick_nohz_idle_stop_tick();
> > > >> +  else
> > > >> +          tick_nohz_idle_retain_tick();
> > > >> +}
> > > >> +
> > > >>  /**
> > > >>   * cpuidle_idle_call - the main idle function
> > > >>   *
> > > >> @@ -170,7 +178,7 @@ static int call_cpuidle(struct cpuidle_d
> > > >>   * set, and it returns with polling set.  If it ever stops polling, it
> > > >>   * must clear the polling bit.
> > > >>   */
> > > >> -static void cpuidle_idle_call(void)
> > > >> +static void cpuidle_idle_call(bool stop_tick)
> > > >>  {
> > > >>    struct cpuidle_device *dev = cpuidle_get_device();
> > > >>    struct cpuidle_driver *drv = cpuidle_get_cpu_driver(dev);
> > > >> @@ -186,7 +194,7 @@ static void cpuidle_idle_call(void)
> > > >>    }
> > > >>
> > > >>    if (cpuidle_not_available(drv, dev)) {
> > > >> -          tick_nohz_idle_stop_tick();
> > > >> +          idle_call_stop_or_retain_tick(stop_tick);
> > > >>
> > > >>            default_idle_call();
> > > >>            goto exit_idle;
> > > >> @@ -222,17 +230,14 @@ static void cpuidle_idle_call(void)
> > > >>            next_state = cpuidle_find_deepest_state(drv, dev, max_latency_ns);
> > > >>            call_cpuidle(drv, dev, next_state);
> > > >>    } else if (drv->state_count > 1) {
> > > >> -          bool stop_tick = true;
> > > >> +          stop_tick = true;
> > > >
> > > > Silly question, but wouldn't this benefit the normal path too to delay for one
> > > > tick? This will only matter for the cases where the governor doesn't explicitly
> > > > set stop_tick to either true or false - which I am not sure what they are :)
> > > >
> > > Right now the governors will always set stop_tick explicitly (and overriding
> > > that might confuse the governor-internal state).
> >
> > So we can drop this hunk then
> 
> Not really.
> 
> The governors expect that stop_tick is true by default and they clear
> it if needed/desired.  They may be confused if it is false to start
> with (theoretically, a governor may select an idle state with target
> residency beyond the tick period length then which won't make sense).

I see, thanks for the explanation. It could be me, but if you think a comment
is worthwhile to document this expectation, would be nice to have.

LGTM anyway.

Cheers