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

[Qais Yousef]

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