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

[Qais Yousef <qyousef@layalina.io>]

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 :)

>  
>  		/*
>  		 * Ask the cpuidle framework to choose a convenient idle state.
>  		 */
>  		next_state = cpuidle_select(drv, dev, &stop_tick);
>  
> -		if (stop_tick || tick_nohz_tick_stopped())
> -			tick_nohz_idle_stop_tick();
> -		else
> -			tick_nohz_idle_retain_tick();
> +		idle_call_stop_or_retain_tick(stop_tick);
>  
>  		entered_state = call_cpuidle(drv, dev, next_state);
>  		/*
> @@ -240,7 +245,7 @@ static void cpuidle_idle_call(void)
>  		 */
>  		cpuidle_reflect(dev, entered_state);
>  	} else {
> -		tick_nohz_idle_retain_tick();
> +		idle_call_stop_or_retain_tick(stop_tick);
>  
>  		/*
>  		 * If there is only a single idle state (or none), there is
> @@ -268,6 +273,7 @@ exit_idle:
>  static void do_idle(void)
>  {
>  	int cpu = smp_processor_id();
> +	bool got_tick = false;
>  
>  	/*
>  	 * Check if we need to update blocked load
> @@ -338,8 +344,9 @@ static void do_idle(void)
>  			tick_nohz_idle_restart_tick();
>  			cpu_idle_poll();
>  		} else {
> -			cpuidle_idle_call();
> +			cpuidle_idle_call(got_tick);
>  		}
> +		got_tick = tick_nohz_idle_got_tick();
>  		arch_cpu_idle_exit();
>  	}
>  
> 
> 
>