* [PATCHv2] sched: Replace nr_iowait_cpu() with a bool accessor
@ 2026-10-15 8:33 Christian Loehle
2026-10-16 5:13 ` K Prateek Nayak
2026-10-16 6:01 ` [cocci] " Kayra Cizmeci
0 siblings, 2 replies; 3+ 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 [flat|nested] 3+ 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-16 5:13 ` K Prateek Nayak
2026-10-16 6:01 ` [cocci] " Kayra Cizmeci
1 sibling, 0 replies; 3+ 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 [flat|nested] 3+ 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-16 5:13 ` K Prateek Nayak
@ 2026-10-16 6:01 ` Kayra Cizmeci
1 sibling, 0 replies; 3+ 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] 3+ messages in thread
end of thread, other threads:[~2026-10-16 6:01 UTC | newest]
Thread overview: 3+ 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-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
all inboxes | Powered by JetHome®