Linux s390 Architecture development
 help / color / mirror / Atom feed
* [PATCHv2] sched: Replace nr_iowait_cpu() with a bool accessor
@ 2026-10-15  8:33 Christian Loehle
  2026-10-15  8:39 ` sashiko-bot
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Christian Loehle @ 2026-10-15  8:33 UTC (permalink / raw)
  To: Ingo Molnar, Peter Zijlstra, Juri Lelli, Vincent Guittot
  Cc: Dietmar Eggemann, Steven Rostedt, Ben Segall, Mel Gorman,
	Valentin Schneider, K Prateek Nayak, Heiko Carstens,
	Vasily Gorbik, Alexander Gordeev, Christian Borntraeger,
	Sven Schnelle, linux-s390, linux-kernel, cocci, Christian Loehle

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.

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)

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);
 
@@ -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);
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCHv2] sched: Replace nr_iowait_cpu() with a bool accessor
  2026-10-15  8:33 [PATCHv2] sched: Replace nr_iowait_cpu() with a bool accessor Christian Loehle
@ 2026-10-15  8:39 ` sashiko-bot
  2026-10-16  5:13 ` K Prateek Nayak
  2026-10-16  6:01 ` [cocci] " Kayra Cizmeci
  2 siblings, 0 replies; 4+ messages in thread
From: sashiko-bot @ 2026-10-15  8:39 UTC (permalink / raw)
  To: Christian Loehle
  Cc: linux-s390, Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
	Christian Borntraeger

> 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.
> 
> 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)
> 
> Signed-off-by: Christian Loehle <christian.loehle@arm.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261015083317.3733733-1-christian.loehle@arm.com?part=1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCHv2] sched: Replace nr_iowait_cpu() with a bool accessor
  2026-10-15  8:33 [PATCHv2] sched: Replace nr_iowait_cpu() with a bool accessor Christian Loehle
  2026-10-15  8:39 ` sashiko-bot
@ 2026-10-16  5:13 ` K Prateek Nayak
  2026-10-16  6:01 ` [cocci] " Kayra Cizmeci
  2 siblings, 0 replies; 4+ messages in thread
From: K Prateek Nayak @ 2026-10-16  5:13 UTC (permalink / raw)
  To: Christian Loehle, Ingo Molnar, Peter Zijlstra, Juri Lelli,
	Vincent Guittot
  Cc: Dietmar Eggemann, Steven Rostedt, Ben Segall, Mel Gorman,
	Valentin Schneider, Heiko Carstens, Vasily Gorbik,
	Alexander Gordeev, Christian Borntraeger, Sven Schnelle,
	linux-s390, linux-kernel, cocci

Hello Chris,

On 10/15/2026 2:03 PM, 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.
> 
> 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 think few more of the open-coded checks for cpu_has_iowait()
can also be converted to use this wrapper which makes some of
them a tad bit more cleaner IMO:

diff --git a/kernel/sched/cputime.c b/kernel/sched/cputime.c
index 1f3a9c1dc853..e9c9fbfb0269 100644
--- a/kernel/sched/cputime.c
+++ b/kernel/sched/cputime.c
@@ -226,9 +226,8 @@ void account_steal_time(u64 cputime)
 void account_idle_time(u64 cputime)
 {
 	u64 *cpustat = kcpustat_this_cpu->cpustat;
-	struct rq *rq = this_rq();
 
-	if (atomic_read(&rq->nr_iowait) > 0)
+	if (cpu_has_iowait(smp_processor_id()))
 		cpustat[CPUTIME_IOWAIT] += cputime;
 	else
 		cpustat[CPUTIME_IDLE] += cputime;
@@ -1109,7 +1108,6 @@ static int kcpustat_field_vtime(u64 *cpustat,
 				int cpu, u64 *val)
 {
 	struct vtime *vtime = &tsk->vtime;
-	struct rq *rq = cpu_rq(cpu);
 	unsigned int seq;
 
 	do {
@@ -1152,11 +1150,11 @@ static int kcpustat_field_vtime(u64 *cpustat,
 				*val += vtime->gtime + vtime_delta(vtime);
 			break;
 		case CPUTIME_IDLE:
-			if (state == VTIME_IDLE && !atomic_read(&rq->nr_iowait))
+			if (state == VTIME_IDLE && !cpu_has_iowait(cpu))
 				*val += vtime_delta(vtime);
 			break;
 		case CPUTIME_IOWAIT:
-			if (state == VTIME_IDLE && atomic_read(&rq->nr_iowait) > 0)
+			if (state == VTIME_IDLE && cpu_has_iowait(cpu))
 				*val += vtime_delta(vtime);
 			break;
 		default:
@@ -1251,7 +1249,7 @@ static int kcpustat_cpu_fetch_vtime(struct kernel_cpustat *dst,
 			}
 			break;
 		case VTIME_IDLE:
-			if (atomic_read(&cpu_rq(cpu)->nr_iowait) > 0)
+			if (cpu_has_iowait(cpu))
 				cpustat[CPUTIME_IOWAIT] += delta;
 			else
 				cpustat[CPUTIME_IDLE] += delta;
---

Thoughts?

> 
> Signed-off-by: Christian Loehle <christian.loehle@arm.com>

Feel free to include:

Reviewed-by: K Prateek Nayak <kprateek.nayak@amd.com>
Tested-by: K Prateek Nayak <kprateek.nayak@amd.com>

-- 
Thanks and Regards,
Prateek


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [cocci] [PATCHv2] sched: Replace nr_iowait_cpu() with a bool accessor
  2026-10-15  8:33 [PATCHv2] sched: Replace nr_iowait_cpu() with a bool accessor Christian Loehle
  2026-10-15  8:39 ` sashiko-bot
  2026-10-16  5:13 ` K Prateek Nayak
@ 2026-10-16  6:01 ` Kayra Cizmeci
  2 siblings, 0 replies; 4+ messages in thread
From: Kayra Cizmeci @ 2026-10-16  6:01 UTC (permalink / raw)
  To: Christian Loehle
  Cc: Ingo Molnar, Peter Zijlstra, Juri Lelli, Vincent Guittot,
	Dietmar Eggemann, Steven Rostedt, Ben Segall, Mel Gorman,
	Valentin Schneider, K Prateek Nayak, Heiko Carstens,
	Vasily Gorbik, Alexander Gordeev, Christian Borntraeger,
	Sven Schnelle, linux-s390, linux-kernel, cocci

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
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-10-16  6:01 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-15  8:33 [PATCHv2] sched: Replace nr_iowait_cpu() with a bool accessor Christian Loehle
2026-10-15  8:39 ` sashiko-bot
2026-10-16  5:13 ` K Prateek Nayak
2026-10-16  6:01 ` [cocci] " Kayra Cizmeci

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox