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; 8+ 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] 8+ 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-15  2:28   ` Wei Gao
  2026-10-14 14:14 ` Christian Loehle
  1 sibling, 2 replies; 8+ 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] 8+ 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
  2026-10-15  7:35     ` Peter Zijlstra
  2026-10-15  2:28   ` Wei Gao
  1 sibling, 1 reply; 8+ 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] 8+ 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
  2026-10-15  2:40   ` Wei Gao
  1 sibling, 1 reply; 8+ 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] 8+ 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
@ 2026-10-15  2:28   ` Wei Gao
  1 sibling, 0 replies; 8+ messages in thread
From: Wei Gao @ 2026-10-15  2:28 UTC (permalink / raw)
  To: Peter Zijlstra
  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 02:49:34PM +0200, 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.
> 
Thanks for such quick feedback. I see your point since per-CPU iowait accounting on SMP 
is fundamentally flawed. It doesn't make sense to create any more patches for 
current iowait accounting.

For iowait.t test i guess need revalue it or simply skipped it.

> > 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] 8+ messages in thread

* Re: [PATCH v1] sched/cputime: Preserve iowait accounting across waking interrupts in dyntick-idle
  2026-10-14 14:14 ` Christian Loehle
@ 2026-10-15  2:40   ` Wei Gao
  0 siblings, 0 replies; 8+ messages in thread
From: Wei Gao @ 2026-10-15  2:40 UTC (permalink / raw)
  To: Christian Loehle
  Cc: Frederic Weisbecker, Peter Zijlstra, 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 03:14:41PM +0100, Christian Loehle wrote:
> 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?
Margins should help, but this case has more chance to failing after
recent kernel change. It does help track kernel behaviro.
> 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] 8+ messages in thread

* Re: [PATCH v1] sched/cputime: Preserve iowait accounting across waking interrupts in dyntick-idle
  2026-10-14 13:17   ` Christian Loehle
@ 2026-10-15  7:35     ` Peter Zijlstra
  2026-10-15  8:24       ` Christian Loehle
  0 siblings, 1 reply; 8+ messages in thread
From: Peter Zijlstra @ 2026-10-15  7:35 UTC (permalink / raw)
  To: Christian Loehle
  Cc: Wei Gao, 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 02:17:52PM +0100, Christian Loehle wrote:

> > 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.

\o/

> FWIW the remaining aggregated nr_iowait users seem to be accounting/reporting.
> (cpufreq still uses it, but it comes directly from p->in_iowait).

I see: get_cpu_iowait_time_us() using nr_cpuwait_cpu(), which in turn is
used for both thermal and cpufreq.

> 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.

Right.

> 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...

Given the exposure in /proc I'm not sure we can *ever* fully get rid of
the per-cpu usage of iowait, but yeah, reducing in-kernel consumers of
this number might be somewhat feasible.

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

* Re: [PATCH v1] sched/cputime: Preserve iowait accounting across waking interrupts in dyntick-idle
  2026-10-15  7:35     ` Peter Zijlstra
@ 2026-10-15  8:24       ` Christian Loehle
  0 siblings, 0 replies; 8+ messages in thread
From: Christian Loehle @ 2026-10-15  8:24 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Wei Gao, 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/15/26 08:35, Peter Zijlstra wrote:
> On Wed, Oct 14, 2026 at 02:17:52PM +0100, Christian Loehle wrote:
> 
>>> 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.
> 
> \o/
> 
>> FWIW the remaining aggregated nr_iowait users seem to be accounting/reporting.
>> (cpufreq still uses it, but it comes directly from p->in_iowait).
> 
> I see: get_cpu_iowait_time_us() using nr_cpuwait_cpu(), which in turn is
> used for both thermal and cpufreq.

thermal uses it with io_is_busy=0, so that's okay, ondemand has it sysfs-exposed
with a default of 0 except for 
	return (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL &&
		boot_cpu_data.x86_vfm >= INTEL_PENTIUM_PRO);

:/

If that were the only users I'm sure there would be ways around it though.

> 
>> 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.
> 
> Right.
> 
>> 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...
> 
> Given the exposure in /proc I'm not sure we can *ever* fully get rid of
> the per-cpu usage of iowait, but yeah, reducing in-kernel consumers of
> this number might be somewhat feasible.


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

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

Thread overview: 8+ 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-15  7:35     ` Peter Zijlstra
2026-10-15  8:24       ` Christian Loehle
2026-10-15  2:28   ` Wei Gao
2026-10-14 14:14 ` Christian Loehle
2026-10-15  2:40   ` Wei Gao

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®