Re: [cocci] [PATCHv2] sched: Replace nr_iowait_cpu() with a bool accessor

[Kayra Cizmeci <kayracizmeci@gmail.com>]

On Thu, Oct 15, 2026 at 09:33:17AM +0100, Christian Loehle wrote:
> The menu governor was the last consumer of the per-CPU I/O-wait count
> that needed its magnitude, apart from nr_iowait() summing it globally.
> Commit 38f83090f515 ("cpuidle: menu: Remove iowait influence") removed
> that use. All remaining per-CPU consumers only test for zero/nonzero.
> 
> Replace nr_iowait_cpu() with bool cpu_has_iowait() and update the generic
> and s390 idle accounting callers. Have nr_iowait() read the counters
> directly so that the global total and existing accounting are preserved.
> Replace the stale menu comment with a description of the predicate.

So, this is a cleanup patch? OK.

> 
> The conversion was done with coccinelle:
> 
> @ predicate depends on patch @
> expression cpu;
> @@
> 
> - nr_iowait_cpu(cpu) > 0
> + cpu_has_iowait(cpu)
> 
> @ callers depends on patch @
> expression cpu;
> @@
> 
> - nr_iowait_cpu(cpu)
> + cpu_has_iowait(cpu)

I had to figure out what coccinelle was first.
Well, I still don't know fully what it is but I think
it's an automatic replacement tool.

I also checked Prateek's findings, I couldn't find more than he finds.
With his changes, all of the places would be covered I think.

> 
> Signed-off-by: Christian Loehle <christian.loehle@arm.com>
> ---
> v2: Move the cocci excerpt to the commit message (Peter)
> 
> 
>  arch/s390/kernel/idle.c    |  4 ++--
>  include/linux/sched/stat.h |  2 +-
>  kernel/sched/core.c        | 13 +++++--------
>  kernel/sched/cputime.c     | 10 +++++-----
>  4 files changed, 13 insertions(+), 16 deletions(-)
> 
> diff --git a/arch/s390/kernel/idle.c b/arch/s390/kernel/idle.c
> index dd12efb58201..ff9732ef494c 100644
> --- a/arch/s390/kernel/idle.c
> +++ b/arch/s390/kernel/idle.c
> @@ -82,13 +82,13 @@ static u64 arch_cpu_idle_time(int cpu, enum cpu_usage_stat idx, bool compute_del
>  
>  u64 arch_kcpustat_field_idle(int cpu)
>  {
> -	return arch_cpu_idle_time(cpu, CPUTIME_IDLE, !nr_iowait_cpu(cpu));
> +	return arch_cpu_idle_time(cpu, CPUTIME_IDLE, !cpu_has_iowait(cpu));
>  }
>  EXPORT_SYMBOL_GPL(arch_kcpustat_field_idle);
>  
>  u64 arch_kcpustat_field_iowait(int cpu)
>  {
> -	return arch_cpu_idle_time(cpu, CPUTIME_IOWAIT, nr_iowait_cpu(cpu));
> +	return arch_cpu_idle_time(cpu, CPUTIME_IOWAIT, cpu_has_iowait(cpu));
>  }
>  EXPORT_SYMBOL_GPL(arch_kcpustat_field_iowait);
>  
> diff --git a/include/linux/sched/stat.h b/include/linux/sched/stat.h
> index 0108a38bb64d..4810b14d0372 100644
> --- a/include/linux/sched/stat.h
> +++ b/include/linux/sched/stat.h
> @@ -20,7 +20,7 @@ extern int nr_processes(void);
>  extern unsigned int nr_running(void);
>  extern bool single_task_running(void);
>  extern unsigned int nr_iowait(void);
> -extern unsigned int nr_iowait_cpu(int cpu);
> +extern bool cpu_has_iowait(int cpu);
>  
>  static inline int sched_info_on(void)
>  {
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 1fe40de6ebe3..1bde5187dafd 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -5588,15 +5588,12 @@ unsigned long long nr_context_switches(void)
>  }
>
>  /*
> - * Consumers of these two interfaces, like for example the cpuidle menu
> - * governor, are using nonsensical data. Preferring shallow idle state selection
> - * for a CPU that has IO-wait which might not even end up running the task when
> - * it does become runnable.
> + * Return whether any tasks are accounted as I/O waiters on this CPU.
> + * They may wake up on a different CPU.
>   */
> -
> -unsigned int nr_iowait_cpu(int cpu)
> +bool cpu_has_iowait(int cpu)
>  {
> -	return atomic_read(&cpu_rq(cpu)->nr_iowait);
> +	return atomic_read(&cpu_rq(cpu)->nr_iowait) != 0;
>  }
>  
>  /*
> @@ -5634,7 +5631,7 @@ unsigned int nr_iowait(void)
>  	unsigned int i, sum = 0;
>  
>  	for_each_possible_cpu(i)
> -		sum += nr_iowait_cpu(i);
> +		sum += atomic_read(&cpu_rq(i)->nr_iowait);
>  
>  	return sum;
>  }
> diff --git a/kernel/sched/cputime.c b/kernel/sched/cputime.c
> index 06bddaa738e5..1f84a950abed 100644
> --- a/kernel/sched/cputime.c
> +++ b/kernel/sched/cputime.c
> @@ -431,7 +431,7 @@ static void kcpustat_idle_stop(struct kernel_cpustat *kc, u64 now)
>  	if (!kc->idle_elapse)
>  		return;
>  
> -	iowait = nr_iowait_cpu(smp_processor_id()) > 0;
> +	iowait = cpu_has_iowait(smp_processor_id());
>  	delta = now - kc->idle_entrytime;
>  	steal = steal_account_process_time(delta);

Yeah, these look way more cleaner to me.

>  
> @@ -544,14 +544,14 @@ static u64 kcpustat_field_dyntick(int cpu, enum cpu_usage_stat idx,
>  u64 kcpustat_field_idle(int cpu)
>  {
>  	return kcpustat_field_dyntick(cpu, CPUTIME_IDLE,
> -				      !nr_iowait_cpu(cpu), ktime_get());
> +				      !cpu_has_iowait(cpu), ktime_get());
>  }
>  EXPORT_SYMBOL_GPL(kcpustat_field_idle);
>  
>  u64 kcpustat_field_iowait(int cpu)
>  {
>  	return kcpustat_field_dyntick(cpu, CPUTIME_IOWAIT,
> -				      nr_iowait_cpu(cpu), ktime_get());
> +				      cpu_has_iowait(cpu), ktime_get());
>  }
>  EXPORT_SYMBOL_GPL(kcpustat_field_iowait);
>  #else
> @@ -601,7 +601,7 @@ static u64 get_cpu_sleep_time_us(int cpu, enum cpu_usage_stat idx,
>  u64 get_cpu_idle_time_us(int cpu, u64 *last_update_time)
>  {
>  	return get_cpu_sleep_time_us(cpu, CPUTIME_IDLE,
> -				     !nr_iowait_cpu(cpu), last_update_time);
> +				     !cpu_has_iowait(cpu), last_update_time);
>  }
>  EXPORT_SYMBOL_GPL(get_cpu_idle_time_us);
>  
> @@ -625,7 +625,7 @@ EXPORT_SYMBOL_GPL(get_cpu_idle_time_us);
>  u64 get_cpu_iowait_time_us(int cpu, u64 *last_update_time)
>  {
>  	return get_cpu_sleep_time_us(cpu, CPUTIME_IOWAIT,
> -				     nr_iowait_cpu(cpu), last_update_time);
> +				     cpu_has_iowait(cpu), last_update_time);
>  }
>  EXPORT_SYMBOL_GPL(get_cpu_iowait_time_us);
>  

BTW, this is tied to counter being correct, if its not...
Well we have problems. I think your other patch was covering that?
But I have a bad memory, so IDK.

Reviewed-by: Kayra Cizmeci <kayracizmeci@gmail.com>

Thanks!

Kayra



> -- 
> 2.34.1
>