mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v1] sched/cputime: Preserve iowait accounting across waking interrupts in dyntick-idle
@ 2026-10-14  1:52 Wei Gao
  2026-10-14 12:49 ` Peter Zijlstra
  2026-10-14 14:14 ` Christian Loehle
  0 siblings, 2 replies; 4+ messages in thread
From: Wei Gao @ 2026-10-14  1:52 UTC (permalink / raw)
  To: Frederic Weisbecker, Peter Zijlstra, Ingo Molnar, Juri Lelli,
	Vincent Guittot
  Cc: Thomas Gleixner, Dietmar Eggemann, Steven Rostedt, Ben Segall,
	Mel Gorman, Valentin Schneider, K Prateek Nayak, Jens Axboe,
	io-uring, linux-kernel, Wei Gao

In liburing's test suite, the test/iowait.t test case intermittently fails
with:
    iowait diff too small: 48
    test 1 failed
on tickless systems where IRQ time accounting (CONFIG_IRQ_TIME_ACCOUNTING)
is not enabled.

Commit 7198e3927a14 ("sched/cputime: Handle idle irqtime gracefully")
optimized dyntick-idle cputime accounting by avoiding pausing idle
accounting on IRQ entry when IRQ time accounting is disabled.

However, this causes iowait time to be erroneously accounted as
CPUTIME_IDLE if the task waiting for I/O is woken up inside the interrupt
handler (such as the 1-second timeout hrtimer in liburing's test/iowait.t):

1. In test/iowait.t, a read request is submitted on an empty pipe and
   io_uring_wait_cqe_timeout() is called with a 1-second timeout. Because
   current->in_iowait is set, rq->nr_iowait on the CPU is incremented
   to 1. When the CPU enters tickless idle, the periodic scheduler tick
   is stopped.
2. When the 1-second timeout expires, the hrtimer interrupt fires.
   Because irqtime is disabled, kcpustat_irq_enter() does not pause idle
   accounting.
3. Inside the interrupt handler, io_cqring_timer_wakeup() calls
   wake_up_process(). In ttwu_do_activate(), waking up the task immediately
   decrements rq->nr_iowait from 1 to 0 before the CPU has exited idle.
4. When the CPU returns from the interrupt and calls tick_nohz_idle_exit()
   -> kcpustat_idle_stop() to settle the elapsed tickless idle duration,
   it checks:
       iowait = nr_iowait_cpu(smp_processor_id()) > 0;
   Because rq->nr_iowait was already decremented by ttwu_do_activate(),
   this evaluates to false. As a result, the entire 1-second idle duration
   spent waiting for I/O is credited to CPUTIME_IDLE instead of
   CPUTIME_IOWAIT, causing diff < 50 and failing the test.

Fix this by checking nr_iowait_cpu() in kcpustat_irq_enter(). If the CPU is
currently in iowait, pause and settle the elapsed time to CPUTIME_IOWAIT
before the interrupt handler runs. On kcpustat_irq_exit(), resume
accounting if it was paused.

Fixes: 7198e3927a14 ("sched/cputime: Handle idle irqtime gracefully")
Signed-off-by: Wei Gao <wegao@suse.com>
---
 kernel/sched/cputime.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/kernel/sched/cputime.c b/kernel/sched/cputime.c
index 06bddaa738e5..e8b56433c61c 100644
--- a/kernel/sched/cputime.c
+++ b/kernel/sched/cputime.c
@@ -496,7 +496,8 @@ void kcpustat_irq_enter(u64 now)
 	struct kernel_cpustat *kc = kcpustat_this_cpu;
 
 	if (!vtime_generic_enabled_this_cpu() &&
-	    (irqtime_enabled() || vtime_accounting_enabled_this_cpu()))
+	    (irqtime_enabled() || vtime_accounting_enabled_this_cpu() ||
+	     nr_iowait_cpu(smp_processor_id()) > 0))
 		kcpustat_idle_stop(kc, now);
 }
 
@@ -510,9 +511,13 @@ void kcpustat_irq_exit(u64 now)
 	 * need to pause nohz accounting. Resume nohz accounting as long
 	 * as the irqtime config is enabled to handle case where irqtime
 	 * accounting got runtime disabled in the middle of an IRQ.
+	 *
+	 * Also resume accounting if it was paused on IRQ enter due to
+	 * iowait.
 	 */
 	if (!vtime_generic_enabled_this_cpu() &&
-	    (IS_ENABLED(CONFIG_IRQ_TIME_ACCOUNTING) || vtime_accounting_enabled_this_cpu()))
+	    (IS_ENABLED(CONFIG_IRQ_TIME_ACCOUNTING) || vtime_accounting_enabled_this_cpu() ||
+	     !kc->idle_elapse))
 		kcpustat_idle_start(kc, now);
 }
 
-- 
2.55.0


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

* Re: [PATCH v1] sched/cputime: Preserve iowait accounting across waking interrupts in dyntick-idle
  2026-10-14  1:52 [PATCH v1] sched/cputime: Preserve iowait accounting across waking interrupts in dyntick-idle Wei Gao
@ 2026-10-14 12:49 ` Peter Zijlstra
  2026-10-14 13:17   ` Christian Loehle
  2026-10-14 14:14 ` Christian Loehle
  1 sibling, 1 reply; 4+ messages in thread
From: Peter Zijlstra @ 2026-10-14 12:49 UTC (permalink / raw)
  To: Wei Gao
  Cc: Frederic Weisbecker, Ingo Molnar, Juri Lelli, Vincent Guittot,
	Thomas Gleixner, Dietmar Eggemann, Steven Rostedt, Ben Segall,
	Mel Gorman, Valentin Schneider, K Prateek Nayak, Jens Axboe,
	io-uring, linux-kernel

On Wed, Oct 14, 2026 at 01:52:58AM +0000, Wei Gao wrote:
> In liburing's test suite, the test/iowait.t test case intermittently fails
> with:
>     iowait diff too small: 48
>     test 1 failed
> on tickless systems where IRQ time accounting (CONFIG_IRQ_TIME_ACCOUNTING)
> is not enabled.
> 
> Commit 7198e3927a14 ("sched/cputime: Handle idle irqtime gracefully")
> optimized dyntick-idle cputime accounting by avoiding pausing idle
> accounting on IRQ entry when IRQ time accounting is disabled.
> 
> However, this causes iowait time to be erroneously accounted as
> CPUTIME_IDLE if the task waiting for I/O is woken up inside the interrupt
> handler (such as the 1-second timeout hrtimer in liburing's test/iowait.t):
> 
> 1. In test/iowait.t, a read request is submitted on an empty pipe and
>    io_uring_wait_cqe_timeout() is called with a 1-second timeout. Because
>    current->in_iowait is set, rq->nr_iowait on the CPU is incremented
>    to 1. When the CPU enters tickless idle, the periodic scheduler tick
>    is stopped.
> 2. When the 1-second timeout expires, the hrtimer interrupt fires.
>    Because irqtime is disabled, kcpustat_irq_enter() does not pause idle
>    accounting.
> 3. Inside the interrupt handler, io_cqring_timer_wakeup() calls
>    wake_up_process(). In ttwu_do_activate(), waking up the task immediately
>    decrements rq->nr_iowait from 1 to 0 before the CPU has exited idle.
> 4. When the CPU returns from the interrupt and calls tick_nohz_idle_exit()
>    -> kcpustat_idle_stop() to settle the elapsed tickless idle duration,
>    it checks:
>        iowait = nr_iowait_cpu(smp_processor_id()) > 0;
>    Because rq->nr_iowait was already decremented by ttwu_do_activate(),
>    this evaluates to false. As a result, the entire 1-second idle duration
>    spent waiting for I/O is credited to CPUTIME_IDLE instead of
>    CPUTIME_IOWAIT, causing diff < 50 and failing the test.
> 
> Fix this by checking nr_iowait_cpu() in kcpustat_irq_enter(). If the CPU is
> currently in iowait, pause and settle the elapsed time to CPUTIME_IOWAIT
> before the interrupt handler runs. On kcpustat_irq_exit(), resume
> accounting if it was paused.

Using nr_iowait_cpu() is fundamentally never a fix. It is rolling a dice
and liking random numbers. See the comment that goes with that function.

> Fixes: 7198e3927a14 ("sched/cputime: Handle idle irqtime gracefully")
> Signed-off-by: Wei Gao <wegao@suse.com>
> ---
>  kernel/sched/cputime.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/sched/cputime.c b/kernel/sched/cputime.c
> index 06bddaa738e5..e8b56433c61c 100644
> --- a/kernel/sched/cputime.c
> +++ b/kernel/sched/cputime.c
> @@ -496,7 +496,8 @@ void kcpustat_irq_enter(u64 now)
>  	struct kernel_cpustat *kc = kcpustat_this_cpu;
>  
>  	if (!vtime_generic_enabled_this_cpu() &&
> -	    (irqtime_enabled() || vtime_accounting_enabled_this_cpu()))
> +	    (irqtime_enabled() || vtime_accounting_enabled_this_cpu() ||
> +	     nr_iowait_cpu(smp_processor_id()) > 0))
>  		kcpustat_idle_stop(kc, now);
>  }
>  
> @@ -510,9 +511,13 @@ void kcpustat_irq_exit(u64 now)
>  	 * need to pause nohz accounting. Resume nohz accounting as long
>  	 * as the irqtime config is enabled to handle case where irqtime
>  	 * accounting got runtime disabled in the middle of an IRQ.
> +	 *
> +	 * Also resume accounting if it was paused on IRQ enter due to
> +	 * iowait.
>  	 */
>  	if (!vtime_generic_enabled_this_cpu() &&
> -	    (IS_ENABLED(CONFIG_IRQ_TIME_ACCOUNTING) || vtime_accounting_enabled_this_cpu()))
> +	    (IS_ENABLED(CONFIG_IRQ_TIME_ACCOUNTING) || vtime_accounting_enabled_this_cpu() ||
> +	     !kc->idle_elapse))
>  		kcpustat_idle_start(kc, now);
>  }
>  
> -- 
> 2.55.0
> 

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

* Re: [PATCH v1] sched/cputime: Preserve iowait accounting across waking interrupts in dyntick-idle
  2026-10-14 12:49 ` Peter Zijlstra
@ 2026-10-14 13:17   ` Christian Loehle
  0 siblings, 0 replies; 4+ messages in thread
From: Christian Loehle @ 2026-10-14 13:17 UTC (permalink / raw)
  To: Peter Zijlstra, Wei Gao
  Cc: Frederic Weisbecker, Ingo Molnar, Juri Lelli, Vincent Guittot,
	Thomas Gleixner, Dietmar Eggemann, Steven Rostedt, Ben Segall,
	Mel Gorman, Valentin Schneider, K Prateek Nayak, Jens Axboe,
	io-uring, linux-kernel

On 10/14/26 13:49, Peter Zijlstra wrote:
> On Wed, Oct 14, 2026 at 01:52:58AM +0000, Wei Gao wrote:
>> In liburing's test suite, the test/iowait.t test case intermittently fails
>> with:
>>     iowait diff too small: 48
>>     test 1 failed
>> on tickless systems where IRQ time accounting (CONFIG_IRQ_TIME_ACCOUNTING)
>> is not enabled.
>>
>> Commit 7198e3927a14 ("sched/cputime: Handle idle irqtime gracefully")
>> optimized dyntick-idle cputime accounting by avoiding pausing idle
>> accounting on IRQ entry when IRQ time accounting is disabled.
>>
>> However, this causes iowait time to be erroneously accounted as
>> CPUTIME_IDLE if the task waiting for I/O is woken up inside the interrupt
>> handler (such as the 1-second timeout hrtimer in liburing's test/iowait.t):
>>
>> 1. In test/iowait.t, a read request is submitted on an empty pipe and
>>    io_uring_wait_cqe_timeout() is called with a 1-second timeout. Because
>>    current->in_iowait is set, rq->nr_iowait on the CPU is incremented
>>    to 1. When the CPU enters tickless idle, the periodic scheduler tick
>>    is stopped.
>> 2. When the 1-second timeout expires, the hrtimer interrupt fires.
>>    Because irqtime is disabled, kcpustat_irq_enter() does not pause idle
>>    accounting.
>> 3. Inside the interrupt handler, io_cqring_timer_wakeup() calls
>>    wake_up_process(). In ttwu_do_activate(), waking up the task immediately
>>    decrements rq->nr_iowait from 1 to 0 before the CPU has exited idle.
>> 4. When the CPU returns from the interrupt and calls tick_nohz_idle_exit()
>>    -> kcpustat_idle_stop() to settle the elapsed tickless idle duration,
>>    it checks:
>>        iowait = nr_iowait_cpu(smp_processor_id()) > 0;
>>    Because rq->nr_iowait was already decremented by ttwu_do_activate(),
>>    this evaluates to false. As a result, the entire 1-second idle duration
>>    spent waiting for I/O is credited to CPUTIME_IDLE instead of
>>    CPUTIME_IOWAIT, causing diff < 50 and failing the test.
>>
>> Fix this by checking nr_iowait_cpu() in kcpustat_irq_enter(). If the CPU is
>> currently in iowait, pause and settle the elapsed time to CPUTIME_IOWAIT
>> before the interrupt handler runs. On kcpustat_irq_exit(), resume
>> accounting if it was paused.
> 
> Using nr_iowait_cpu() is fundamentally never a fix. It is rolling a dice
> and liking random numbers. See the comment that goes with that function.
> 
Speaking of, that comment is stale, menu doesn't use that heuristic anymore.
FWIW the remaining aggregated nr_iowait users seem to be accounting/reporting.
(cpufreq still uses it, but it comes directly from p->in_iowait).
Given they are exported (/proc/stat procs_blocked and something in s390) I don't
see how we can remove this entirely but at least nr_iowait_cpu() as opposed
to cpu_has_iowait() could be refactored. The issues are obviously the same still.
I'll send a patch.

Peter, if you see a path to ever retire iowait (or at least the CPU iowait for now),
I'm more than happy to sink some time into it...

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

* Re: [PATCH v1] sched/cputime: Preserve iowait accounting across waking interrupts in dyntick-idle
  2026-10-14  1:52 [PATCH v1] sched/cputime: Preserve iowait accounting across waking interrupts in dyntick-idle Wei Gao
  2026-10-14 12:49 ` Peter Zijlstra
@ 2026-10-14 14:14 ` Christian Loehle
  1 sibling, 0 replies; 4+ messages in thread
From: Christian Loehle @ 2026-10-14 14:14 UTC (permalink / raw)
  To: Wei Gao, Frederic Weisbecker, Peter Zijlstra, Ingo Molnar,
	Juri Lelli, Vincent Guittot
  Cc: Thomas Gleixner, Dietmar Eggemann, Steven Rostedt, Ben Segall,
	Mel Gorman, Valentin Schneider, K Prateek Nayak, Jens Axboe,
	io-uring, linux-kernel

On 10/14/26 02:52, Wei Gao wrote:
> In liburing's test suite, the test/iowait.t test case intermittently fails
> with:
>     iowait diff too small: 48
>     test 1 failed
> on tickless systems where IRQ time accounting (CONFIG_IRQ_TIME_ACCOUNTING)
> is not enabled.
> 
> Commit 7198e3927a14 ("sched/cputime: Handle idle irqtime gracefully")
> optimized dyntick-idle cputime accounting by avoiding pausing idle
> accounting on IRQ entry when IRQ time accounting is disabled.
> 
> However, this causes iowait time to be erroneously accounted as
> CPUTIME_IDLE if the task waiting for I/O is woken up inside the interrupt
> handler (such as the 1-second timeout hrtimer in liburing's test/iowait.t):
> 
> 1. In test/iowait.t, a read request is submitted on an empty pipe and
>    io_uring_wait_cqe_timeout() is called with a 1-second timeout. Because
>    current->in_iowait is set, rq->nr_iowait on the CPU is incremented
>    to 1. When the CPU enters tickless idle, the periodic scheduler tick
>    is stopped.
> 2. When the 1-second timeout expires, the hrtimer interrupt fires.
>    Because irqtime is disabled, kcpustat_irq_enter() does not pause idle
>    accounting.
> 3. Inside the interrupt handler, io_cqring_timer_wakeup() calls
>    wake_up_process(). In ttwu_do_activate(), waking up the task immediately
>    decrements rq->nr_iowait from 1 to 0 before the CPU has exited idle.
> 4. When the CPU returns from the interrupt and calls tick_nohz_idle_exit()
>    -> kcpustat_idle_stop() to settle the elapsed tickless idle duration,
>    it checks:
>        iowait = nr_iowait_cpu(smp_processor_id()) > 0;
>    Because rq->nr_iowait was already decremented by ttwu_do_activate(),
>    this evaluates to false. As a result, the entire 1-second idle duration
>    spent waiting for I/O is credited to CPUTIME_IDLE instead of
>    CPUTIME_IOWAIT, causing diff < 50 and failing the test.

So reading the test, the reason this doesn't fail constantly (on a non-isolated
machine) is the massive margins?
It pins the io_uring_wait_cqe_timeout() to CPU0 with either in_iowait=1 or
in_iowait=0.
It then asserts after 1s that CPU0 got: 
a) for in_iowait=0 at most +10 iowait units (<10% CPU time)
b) for in_iowait=1 at least +50 iowait units (>50% CPU time)
So a) would fail on too many unrelated iowaits, b) would fail on unrelated
runnable tasks.

I don't think validating the per-task in_iowait setting via the CPU aggregate
is really feasible FWIW.

> 
> Fix this by checking nr_iowait_cpu() in kcpustat_irq_enter(). If the CPU is
> currently in iowait, pause and settle the elapsed time to CPUTIME_IOWAIT
> before the interrupt handler runs. On kcpustat_irq_exit(), resume
> accounting if it was paused.
> 
> Fixes: 7198e3927a14 ("sched/cputime: Handle idle irqtime gracefully")
> Signed-off-by: Wei Gao <wegao@suse.com>
> ---
>  kernel/sched/cputime.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/sched/cputime.c b/kernel/sched/cputime.c
> index 06bddaa738e5..e8b56433c61c 100644
> --- a/kernel/sched/cputime.c
> +++ b/kernel/sched/cputime.c
> @@ -496,7 +496,8 @@ void kcpustat_irq_enter(u64 now)
>  	struct kernel_cpustat *kc = kcpustat_this_cpu;
>  
>  	if (!vtime_generic_enabled_this_cpu() &&
> -	    (irqtime_enabled() || vtime_accounting_enabled_this_cpu()))
> +	    (irqtime_enabled() || vtime_accounting_enabled_this_cpu() ||
> +	     nr_iowait_cpu(smp_processor_id()) > 0))
>  		kcpustat_idle_stop(kc, now);
>  }
>  
> @@ -510,9 +511,13 @@ void kcpustat_irq_exit(u64 now)
>  	 * need to pause nohz accounting. Resume nohz accounting as long
>  	 * as the irqtime config is enabled to handle case where irqtime
>  	 * accounting got runtime disabled in the middle of an IRQ.
> +	 *
> +	 * Also resume accounting if it was paused on IRQ enter due to
> +	 * iowait.
>  	 */
>  	if (!vtime_generic_enabled_this_cpu() &&
> -	    (IS_ENABLED(CONFIG_IRQ_TIME_ACCOUNTING) || vtime_accounting_enabled_this_cpu()))
> +	    (IS_ENABLED(CONFIG_IRQ_TIME_ACCOUNTING) || vtime_accounting_enabled_this_cpu() ||
> +	     !kc->idle_elapse))
>  		kcpustat_idle_start(kc, now);
>  }
>  


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

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

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-14  1:52 [PATCH v1] sched/cputime: Preserve iowait accounting across waking interrupts in dyntick-idle Wei Gao
2026-10-14 12:49 ` Peter Zijlstra
2026-10-14 13:17   ` Christian Loehle
2026-10-14 14:14 ` Christian Loehle

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®