mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] delayacct: Account blkio start on the correct task
@ 2026-10-14  8:17 Zhan Xusheng
  2026-10-14 15:20 ` Christian Loehle
  0 siblings, 1 reply; 9+ messages in thread
From: Zhan Xusheng @ 2026-10-14  8:17 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar
  Cc: Balbir Singh, Juri Lelli, Vincent Guittot, Dietmar Eggemann,
	Valentin Schneider, Josh Snyder, linux-kernel, Zhan Xusheng

__block_task(rq, p) tests p->in_iowait and bumps rq->nr_iowait, but the
third statement in that block takes neither argument:

	if (p->in_iowait) {
		atomic_inc(&rq->nr_iowait);
		delayacct_blkio_start();
	}

delayacct_blkio_start() stamps current->delays->blkio_start.

That was fine when commit e8901061ca0c ("sched: Split DEQUEUE_SLEEP from
deactivate_task()") introduced __block_task(), because its only caller
was block_task() out of __schedule(), where prev == current.  Commit
152e11f6df29 ("sched/fair: Implement delayed dequeue") added a second
call, at the point the delayed entity is finally dequeued:

	/*
	 * Fix-up what block_task() skipped.
	 *
	 * Must be last, @p might not be valid after this.
	 */
	__block_task(rq, p);

The paths that reach it do not run as @p:

  - pick_next_entity() -> __dequeue_task(DEQUEUE_DELAYED), inside the
    __schedule() of whatever task is prev on that CPU
  - wait_task_inactive(), in the task doing the waiting
  - switching_from_fair(), via sched_setscheduler() or rt_mutex_setprio(),
    which may or may not be @p itself

So the stamp lands on an unrelated task, and @p keeps whatever
blkio_start it had.  When the I/O completes, delayacct_blkio_end(p)
computes local_clock() - p->delays->blkio_start and adds that to @p's
blkio_delay and blkio_count, and may overwrite blkio_delay_max and
blkio_delay_max_ts.  The error is however long it has been since @p's
blkio_start was last written, so it is unbounded.  ->delays is
kmem_cache_zalloc()'d, so if it was never written the delta is the time
since boot.

Measured on an 8-vCPU guest, 2.5 GB file on f2fs over virtio, one task
alternating a 1 MiB O_DIRECT read with two seconds of pure CPU work,
pinned to CPU 0 against three CPU hogs so that the reader gets
delay-dequeued.  Reading delayacct_blkio_ticks from /proc/self/stat
around the second read:

  DELAY_DEQUEUE    18 runs, 4 wrong: 206, 20316, 20729, 21968 ticks
                   reported for reads that took 25-50 ms
  NO_DELAY_DEQUEUE 12 runs, 0 wrong, all 1-3 ticks

The 206 case is the two seconds of CPU work swept in; the 20000+ cases
are blkio_start still at zero.  In a separate run under the same
conditions, a kprobe on __delayacct_blkio_start() caught it being called
with current == "sh" and current == "kworker/0:2", neither of which
issues any I/O.

Consumers are delayacct_blkio_ticks in /proc/pid/stat, taskstats
(getdelays, iotop, delaytop) and the per-task blkio_delay_max peak.  The
error also goes the other way: a task woken while still delayed goes
through ttwu_runnable() and has that sleep dropped entirely.

Pass the task to delayacct_blkio_start(), the same way commit
c96f5471ce7d ("delayacct: Account blkio completion on the correct task")
did for the end side when commit e33a9bba85a8 ("sched/core: move IO
scheduling accounting from io_schedule_timeout() into scheduler") moved
that call into ttwu().

Note this only makes blkio_start agree with the existing deferral: both
the stamp and rq->nr_iowait are still taken at __block_task() time, so
the interval a task spends delayed is counted by neither.  Whether that
interval should count as iowait is a separate question.

Fixes: 152e11f6df29 ("sched/fair: Implement delayed dequeue")
Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>
---
 include/linux/delayacct.h | 10 +++++-----
 kernel/delayacct.c        |  4 ++--
 kernel/sched/sched.h      |  2 +-
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/include/linux/delayacct.h b/include/linux/delayacct.h
index ecb06f16d22c5..e595d51699377 100644
--- a/include/linux/delayacct.h
+++ b/include/linux/delayacct.h
@@ -92,7 +92,7 @@ extern void delayacct_init(void);
 
 extern void __delayacct_tsk_init(struct task_struct *);
 extern void __delayacct_tsk_exit(struct task_struct *);
-extern void __delayacct_blkio_start(void);
+extern void __delayacct_blkio_start(struct task_struct *p);
 extern void __delayacct_blkio_end(struct task_struct *);
 extern int delayacct_add_tsk(struct taskstats *, struct task_struct *);
 extern __u64 __delayacct_blkio_ticks(struct task_struct *);
@@ -126,13 +126,13 @@ static inline void delayacct_tsk_free(struct task_struct *tsk)
 	tsk->delays = NULL;
 }
 
-static inline void delayacct_blkio_start(void)
+static inline void delayacct_blkio_start(struct task_struct *p)
 {
 	if (!static_branch_unlikely(&delayacct_key))
 		return;
 
-	if (current->delays)
-		__delayacct_blkio_start();
+	if (p->delays)
+		__delayacct_blkio_start(p);
 }
 
 static inline void delayacct_blkio_end(struct task_struct *p)
@@ -257,7 +257,7 @@ static inline void delayacct_tsk_init(struct task_struct *tsk)
 {}
 static inline void delayacct_tsk_free(struct task_struct *tsk)
 {}
-static inline void delayacct_blkio_start(void)
+static inline void delayacct_blkio_start(struct task_struct *p)
 {}
 static inline void delayacct_blkio_end(struct task_struct *p)
 {}
diff --git a/kernel/delayacct.c b/kernel/delayacct.c
index 479e860aff705..d6158ecbe0be5 100644
--- a/kernel/delayacct.c
+++ b/kernel/delayacct.c
@@ -126,9 +126,9 @@ static void delayacct_end(raw_spinlock_t *lock, u64 *start, u64 *total, u32 *cou
 	}
 }
 
-void __delayacct_blkio_start(void)
+void __delayacct_blkio_start(struct task_struct *p)
 {
-	current->delays->blkio_start = local_clock();
+	p->delays->blkio_start = local_clock();
 }
 
 /*
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 7d2ec527b8a25..5bba703160e58 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -3106,7 +3106,7 @@ static inline void __block_task(struct rq *rq, struct task_struct *p)
 
 	if (p->in_iowait) {
 		atomic_inc(&rq->nr_iowait);
-		delayacct_blkio_start();
+		delayacct_blkio_start(p);
 	}
 
 	ASSERT_EXCLUSIVE_WRITER(p->on_rq);

base-commit: 4a3b51aab6e25244d97936aa65e6d5425adf98e1
-- 
2.43.0


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

* Re: [PATCH] delayacct: Account blkio start on the correct task
  2026-10-14  8:17 [PATCH] delayacct: Account blkio start on the correct task Zhan Xusheng
@ 2026-10-14 15:20 ` Christian Loehle
  2026-10-14 16:12   ` [PATCH] sched: Account blkio delay independently of delayed dequeue Christian Loehle
  0 siblings, 1 reply; 9+ messages in thread
From: Christian Loehle @ 2026-10-14 15:20 UTC (permalink / raw)
  To: Zhan Xusheng, Peter Zijlstra, Ingo Molnar
  Cc: Balbir Singh, Juri Lelli, Vincent Guittot, Dietmar Eggemann,
	Valentin Schneider, Josh Snyder, linux-kernel, Zhan Xusheng

On 10/14/26 09:17, Zhan Xusheng wrote:
> __block_task(rq, p) tests p->in_iowait and bumps rq->nr_iowait, but the
> third statement in that block takes neither argument:
> 
> 	if (p->in_iowait) {
> 		atomic_inc(&rq->nr_iowait);
> 		delayacct_blkio_start();
> 	}
> 
> delayacct_blkio_start() stamps current->delays->blkio_start.
> 
> That was fine when commit e8901061ca0c ("sched: Split DEQUEUE_SLEEP from
> deactivate_task()") introduced __block_task(), because its only caller
> was block_task() out of __schedule(), where prev == current.  Commit
> 152e11f6df29 ("sched/fair: Implement delayed dequeue") added a second
> call, at the point the delayed entity is finally dequeued:
> 
> 	/*
> 	 * Fix-up what block_task() skipped.
> 	 *
> 	 * Must be last, @p might not be valid after this.
> 	 */
> 	__block_task(rq, p);
> 
> The paths that reach it do not run as @p:
> 
>   - pick_next_entity() -> __dequeue_task(DEQUEUE_DELAYED), inside the
>     __schedule() of whatever task is prev on that CPU
>   - wait_task_inactive(), in the task doing the waiting
>   - switching_from_fair(), via sched_setscheduler() or rt_mutex_setprio(),
>     which may or may not be @p itself
> 
> So the stamp lands on an unrelated task, and @p keeps whatever
> blkio_start it had.  When the I/O completes, delayacct_blkio_end(p)
> computes local_clock() - p->delays->blkio_start and adds that to @p's
> blkio_delay and blkio_count, and may overwrite blkio_delay_max and
> blkio_delay_max_ts.  The error is however long it has been since @p's
> blkio_start was last written, so it is unbounded.  ->delays is
> kmem_cache_zalloc()'d, so if it was never written the delta is the time
> since boot.
> 
> Measured on an 8-vCPU guest, 2.5 GB file on f2fs over virtio, one task
> alternating a 1 MiB O_DIRECT read with two seconds of pure CPU work,
> pinned to CPU 0 against three CPU hogs so that the reader gets
> delay-dequeued.  Reading delayacct_blkio_ticks from /proc/self/stat
> around the second read:
> 
>   DELAY_DEQUEUE    18 runs, 4 wrong: 206, 20316, 20729, 21968 ticks
>                    reported for reads that took 25-50 ms
>   NO_DELAY_DEQUEUE 12 runs, 0 wrong, all 1-3 ticks
> 
> The 206 case is the two seconds of CPU work swept in; the 20000+ cases
> are blkio_start still at zero.  In a separate run under the same
> conditions, a kprobe on __delayacct_blkio_start() caught it being called
> with current == "sh" and current == "kworker/0:2", neither of which
> issues any I/O.
> 
> Consumers are delayacct_blkio_ticks in /proc/pid/stat, taskstats
> (getdelays, iotop, delaytop) and the per-task blkio_delay_max peak.  The
> error also goes the other way: a task woken while still delayed goes
> through ttwu_runnable() and has that sleep dropped entirely.
> 
> Pass the task to delayacct_blkio_start(), the same way commit
> c96f5471ce7d ("delayacct: Account blkio completion on the correct task")
> did for the end side when commit e33a9bba85a8 ("sched/core: move IO
> scheduling accounting from io_schedule_timeout() into scheduler") moved
> that call into ttwu().
> 
> Note this only makes blkio_start agree with the existing deferral: both
> the stamp and rq->nr_iowait are still taken at __block_task() time, so
> the interval a task spends delayed is counted by neither.  Whether that
> interval should count as iowait is a separate question.
> 
> Fixes: 152e11f6df29 ("sched/fair: Implement delayed dequeue")
> Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>

2 iowait patches in one day? Wow
For the task-pointer fix:
Reviewed-by: Christian Loehle <christian.loehle@arm.com>

BUT FWIW I don't think this is correct in the first place, why would delayacct_blkio
only start after delayed dequeue? This means a task waking up within
delayed dequeue is invisible to the accounting (as you note).
Let me try to move this to the actual block/wakeup...


> ---
>  include/linux/delayacct.h | 10 +++++-----
>  kernel/delayacct.c        |  4 ++--
>  kernel/sched/sched.h      |  2 +-
>  3 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/include/linux/delayacct.h b/include/linux/delayacct.h
> index ecb06f16d22c5..e595d51699377 100644
> --- a/include/linux/delayacct.h
> +++ b/include/linux/delayacct.h
> @@ -92,7 +92,7 @@ extern void delayacct_init(void);
>  
>  extern void __delayacct_tsk_init(struct task_struct *);
>  extern void __delayacct_tsk_exit(struct task_struct *);
> -extern void __delayacct_blkio_start(void);
> +extern void __delayacct_blkio_start(struct task_struct *p);
>  extern void __delayacct_blkio_end(struct task_struct *);
>  extern int delayacct_add_tsk(struct taskstats *, struct task_struct *);
>  extern __u64 __delayacct_blkio_ticks(struct task_struct *);
> @@ -126,13 +126,13 @@ static inline void delayacct_tsk_free(struct task_struct *tsk)
>  	tsk->delays = NULL;
>  }
>  
> -static inline void delayacct_blkio_start(void)
> +static inline void delayacct_blkio_start(struct task_struct *p)
>  {
>  	if (!static_branch_unlikely(&delayacct_key))
>  		return;
>  
> -	if (current->delays)
> -		__delayacct_blkio_start();
> +	if (p->delays)
> +		__delayacct_blkio_start(p);
>  }
>  
>  static inline void delayacct_blkio_end(struct task_struct *p)
> @@ -257,7 +257,7 @@ static inline void delayacct_tsk_init(struct task_struct *tsk)
>  {}
>  static inline void delayacct_tsk_free(struct task_struct *tsk)
>  {}
> -static inline void delayacct_blkio_start(void)
> +static inline void delayacct_blkio_start(struct task_struct *p)
>  {}
>  static inline void delayacct_blkio_end(struct task_struct *p)
>  {}
> diff --git a/kernel/delayacct.c b/kernel/delayacct.c
> index 479e860aff705..d6158ecbe0be5 100644
> --- a/kernel/delayacct.c
> +++ b/kernel/delayacct.c
> @@ -126,9 +126,9 @@ static void delayacct_end(raw_spinlock_t *lock, u64 *start, u64 *total, u32 *cou
>  	}
>  }
>  
> -void __delayacct_blkio_start(void)
> +void __delayacct_blkio_start(struct task_struct *p)
>  {
> -	current->delays->blkio_start = local_clock();
> +	p->delays->blkio_start = local_clock();
>  }
>  
>  /*
> diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
> index 7d2ec527b8a25..5bba703160e58 100644
> --- a/kernel/sched/sched.h
> +++ b/kernel/sched/sched.h
> @@ -3106,7 +3106,7 @@ static inline void __block_task(struct rq *rq, struct task_struct *p)
>  
>  	if (p->in_iowait) {
>  		atomic_inc(&rq->nr_iowait);
> -		delayacct_blkio_start();
> +		delayacct_blkio_start(p);
>  	}
>  
>  	ASSERT_EXCLUSIVE_WRITER(p->on_rq);
> 
> base-commit: 4a3b51aab6e25244d97936aa65e6d5425adf98e1


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

* [PATCH] sched: Account blkio delay independently of delayed dequeue
  2026-10-14 15:20 ` Christian Loehle
@ 2026-10-14 16:12   ` Christian Loehle
  2026-10-14 16:55     ` Kayra Cizmeci
  2026-10-15  2:59     ` Zhan Xusheng
  0 siblings, 2 replies; 9+ messages in thread
From: Christian Loehle @ 2026-10-14 16:12 UTC (permalink / raw)
  To: Zhan Xusheng, Peter Zijlstra, Ingo Molnar
  Cc: Balbir Singh, Juri Lelli, Vincent Guittot, Dietmar Eggemann,
	Valentin Schneider, Josh Snyder, linux-kernel, Zhan Xusheng

Starting blkio delay accounting in __block_task() ties the interval to
removal from the runqueue. With EEVDF delayed dequeue, this misses the
initial part of an I/O sleep, or the entire sleep if the task wakes
before it is dequeued. Final dequeue can also run in another task's
context, causing delayacct_blkio_start() to timestamp the wrong task.

Start accounting in try_to_block_task(), when the current task commits
to blocking, before delayed dequeue or proxy execution can retain it on
the runqueue. End accounting when clearing is_blocked, including queued
wakeups and the proxy execution paths that let the current task continue.

Reported-by: Zhan Xusheng <zhanxusheng@xiaomi.com>
Closes: https://lore.kernel.org/r/20261014081731.1068794-1-zhanxusheng@xiaomi.com
Fixes: 152e11f6df29 ("sched/fair: Implement delayed dequeue")
Signed-off-by: Christian Loehle <christian.loehle@arm.com>
---
 kernel/sched/core.c  | 27 +++++++++++++++++----------
 kernel/sched/sched.h |  4 +---
 2 files changed, 18 insertions(+), 13 deletions(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 1fe40de6ebe3..824c8278d2fc 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -636,7 +636,7 @@ int task_llc(const struct task_struct *p)
  *
  * p->is_blocked <- { 0, 1 }:
  *
- *   is set by try_to_block_task() and cleared by ttwu_do_wakeup() and tracks
+ *   is set by try_to_block_task() and cleared by unblock_task() and tracks
  *   if the task is blocked. Traditionally this would mirror p->on_rq, however
  *   due things like DELAY_DEQUEUE and PROXY_EXEC, this can diverge.
  *
@@ -3737,12 +3737,20 @@ ttwu_stat(struct task_struct *p, int cpu, int wake_flags)
 		__schedstat_inc(p->stats.nr_wakeups_sync);
 }
 
+static inline void unblock_task(struct task_struct *p)
+{
+	if (p->is_blocked && p->in_iowait)
+		delayacct_blkio_end(p);
+
+	p->is_blocked = 0;
+}
+
 /*
  * Mark the task runnable.
  */
 static inline void ttwu_do_wakeup(struct task_struct *p)
 {
-	p->is_blocked = 0;
+	unblock_task(p);
 	WRITE_ONCE(p->__state, TASK_RUNNING);
 	trace_sched_wakeup(p);
 }
@@ -3839,10 +3847,8 @@ ttwu_do_activate(struct rq *rq, struct task_struct *p, int wake_flags,
 		en_flags |= ENQUEUE_RQ_SELECTED;
 	if (wake_flags & WF_MIGRATED)
 		en_flags |= ENQUEUE_MIGRATED;
-	else if (p->in_iowait) {
-		delayacct_blkio_end(p);
+	else if (p->in_iowait)
 		atomic_dec(&task_rq(p)->nr_iowait);
-	}
 
 	activate_task(rq, p, en_flags);
 	wakeup_preempt(rq, p, wake_flags);
@@ -4415,10 +4421,8 @@ int try_to_wake_up(struct task_struct *p, unsigned int state, int wake_flags)
 
 		cpu = select_task_rq(p, p->wake_cpu, &wake_flags);
 		if (task_cpu(p) != cpu) {
-			if (p->in_iowait) {
-				delayacct_blkio_end(p);
+			if (p->in_iowait)
 				atomic_dec(&task_rq(p)->nr_iowait);
-			}
 
 			wake_flags |= WF_MIGRATED;
 			psi_ttwu_dequeue(p);
@@ -6762,6 +6766,9 @@ static bool try_to_block_task(struct rq *rq, struct task_struct *p,
 
 	p->is_blocked = 1;
 
+	if (p->in_iowait)
+		delayacct_blkio_start();
+
 	/*
 	 * We check should_block after signal_pending because we
 	 * will want to wake the task in that case. But if
@@ -6934,7 +6941,7 @@ find_proxy_task(struct rq *rq, struct task_struct *donor, struct rq_flags *rf)
 		if (!mutex) {
 			clear_task_blocked_on(p, mutex);
 			if (task_current(rq, p)) {
-				p->is_blocked = 0;
+				unblock_task(p);
 				return p;
 			}
 			goto deactivate;
@@ -6970,7 +6977,7 @@ find_proxy_task(struct rq *rq, struct task_struct *donor, struct rq_flags *rf)
 			 */
 			__clear_task_blocked_on(p, NULL);
 			if (task_current(rq, p)) {
-				p->is_blocked = 0;
+				unblock_task(p);
 				return p;
 			}
 			goto deactivate;
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 4c25fbe84fb5..638c7709654a 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -3072,10 +3072,8 @@ static inline void __block_task(struct rq *rq, struct task_struct *p)
 	if (p->sched_contributes_to_load)
 		rq->nr_uninterruptible++;
 
-	if (p->in_iowait) {
+	if (p->in_iowait)
 		atomic_inc(&rq->nr_iowait);
-		delayacct_blkio_start();
-	}
 
 	ASSERT_EXCLUSIVE_WRITER(p->on_rq);
 
-- 
2.34.1

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

* Re: [PATCH] sched: Account blkio delay independently of delayed dequeue
  2026-10-14 16:12   ` [PATCH] sched: Account blkio delay independently of delayed dequeue Christian Loehle
@ 2026-10-14 16:55     ` Kayra Cizmeci
  2026-10-15  2:59     ` Zhan Xusheng
  1 sibling, 0 replies; 9+ messages in thread
From: Kayra Cizmeci @ 2026-10-14 16:55 UTC (permalink / raw)
  To: christian.loehle
  Cc: bsingharora, dietmar.eggemann, joshs, juri.lelli, linux-kernel,
	mingo, peterz, vincent.guittot, vschneid, zhanxusheng1024,
	zhanxusheng, Kayra Cizmeci

Hi Christian,

> Starting blkio delay accounting in __block_task() ties the interval to
> removal from the runqueue. With EEVDF delayed dequeue, this misses the
> initial part of an I/O sleep, or the entire sleep if the task wakes
> before it is dequeued. Final dequeue can also run in another task's
> context, causing delayacct_blkio_start() to timestamp the wrong task.

> Start accounting in try_to_block_task(), when the current task commits
> to blocking, before delayed dequeue or proxy execution can retain it on
> the runqueue. End accounting when clearing is_blocked, including queued
> wakeups and the proxy execution paths that let the current task continue.

> Reported-by: Zhan Xusheng <zhanxusheng@xiaomi.com>
> Closes: https://lore.kernel.org/r/20261014081731.1068794-1-zhanxusheng@xiaomi.com
> Fixes: 152e11f6df29 ("sched/fair: Implement delayed dequeue")
> Signed-off-by: Christian Loehle <christian.loehle@arm.com>

Aha. Again one of these Proxy Exec. &  Delayed Dequeue dou causing issue type of issue.

> @@ -636,7 +636,7 @@ int task_llc(const struct task_struct *p)
>   *
>   * p->is_blocked <- { 0, 1 }:
>   *
> - *   is set by try_to_block_task() and cleared by ttwu_do_wakeup() and tracks
> + *   is set by try_to_block_task() and cleared by unblock_task() and tracks
>   *   if the task is blocked. Traditionally this would mirror p->on_rq, however
>   *   due things like DELAY_DEQUEUE and PROXY_EXEC, this can diverge.
>   *

Nice...

> @@ -3737,12 +3737,20 @@ ttwu_stat(struct task_struct *p, int cpu, int wake_flags)
>  		__schedstat_inc(p->stats.nr_wakeups_sync);
>  }
>  
> +static inline void unblock_task(struct task_struct *p)
> +{
> +	if (p->is_blocked && p->in_iowait)
> +		delayacct_blkio_end(p);
> +
> +	p->is_blocked = 0;
> +}
> +

Hm. So if I'm understanding this correctly were trying to track the changes on is_blocked here, correct?
try_to_block_task() finally found his lost twin!

> static inline void ttwu_do_wakeup(struct task_struct *p)
>  {
> -	p->is_blocked = 0;
> +	unblock_task(p);
>  	WRITE_ONCE(p->__state, TASK_RUNNING);
>  	trace_sched_wakeup(p);
>  }
> @@ -3839,10 +3847,8 @@ ttwu_do_activate(struct rq *rq, struct task_struct *p, int wake_flags,
>  		en_flags |= ENQUEUE_RQ_SELECTED;
>  	if (wake_flags & WF_MIGRATED)
>  		en_flags |= ENQUEUE_MIGRATED;
> -	else if (p->in_iowait) {
> -		delayacct_blkio_end(p);
> +	else if (p->in_iowait)
>  		atomic_dec(&task_rq(p)->nr_iowait);
> -	}
>  
>  	activate_task(rq, p, en_flags);
>  	wakeup_preempt(rq, p, wake_flags);
> @@ -4415,10 +4421,8 @@ int try_to_wake_up(struct task_struct *p, unsigned int state, int wake_flags)
>  
>  		cpu = select_task_rq(p, p->wake_cpu, &wake_flags);
>  		if (task_cpu(p) != cpu) {
> -			if (p->in_iowait) {
> -				delayacct_blkio_end(p);
> +			if (p->in_iowait)
>  				atomic_dec(&task_rq(p)->nr_iowait);
> -			}
>  
>  			wake_flags |= WF_MIGRATED;
>  			psi_ttwu_dequeue(p);
> @@ -6762,6 +6766,9 @@ static bool try_to_block_task(struct rq *rq, struct task_struct *p,
>  
>  	p->is_blocked = 1;
>  
> +	if (p->in_iowait)
> +		delayacct_blkio_start();
> +
>  	/*
>  	 * We check should_block after signal_pending because we
>  	 * will want to wake the task in that case. But if
> @@ -6934,7 +6941,7 @@ find_proxy_task(struct rq *rq, struct task_struct *donor, struct rq_flags *rf)
>  		if (!mutex) {
>  			clear_task_blocked_on(p, mutex);
>  			if (task_current(rq, p)) {
> -				p->is_blocked = 0;
> +				unblock_task(p);
>  				return p;
>  			}
>  			goto deactivate;
> @@ -6970,7 +6977,7 @@ find_proxy_task(struct rq *rq, struct task_struct *donor, struct rq_flags *rf)
>  			 */
>  			__clear_task_blocked_on(p, NULL);
>  			if (task_current(rq, p)) {
> -				p->is_blocked = 0;
> +				unblock_task(p);
>  				return p;
>  			}
>  			goto deactivate;
> diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
> index 4c25fbe84fb5..638c7709654a 100644
> --- a/kernel/sched/sched.h
> +++ b/kernel/sched/sched.h
> @@ -3072,10 +3072,8 @@ static inline void __block_task(struct rq *rq, struct task_struct *p)
>  	if (p->sched_contributes_to_load)
>  		rq->nr_uninterruptible++;
>  
> -	if (p->in_iowait) {
> +	if (p->in_iowait)
>  		atomic_inc(&rq->nr_iowait);
> -		delayacct_blkio_start();
> -	}
>  
>  	ASSERT_EXCLUSIVE_WRITER(p->on_rq);
>  

Hmmmm.... HMMMMMMMM....

I mean, looks clean to me. The is_blocked status is followed correctly.
The only place its made 0 is only unblock_task(). And blocking is only on try_to_block_task().

Hope I'm not missing something...

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
















I have no work todo tomorrow, so I can review as many patches as I want.
But I guess no one cares, so I put this down here. As a joke too,
I need to do more jokes. To entertain our sched people.

Thanks,
Kayra





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

* Re: [PATCH] sched: Account blkio delay independently of delayed dequeue
  2026-10-14 16:12   ` [PATCH] sched: Account blkio delay independently of delayed dequeue Christian Loehle
  2026-10-14 16:55     ` Kayra Cizmeci
@ 2026-10-15  2:59     ` Zhan Xusheng
  2026-10-15 10:40       ` Christian Loehle
  1 sibling, 1 reply; 9+ messages in thread
From: Zhan Xusheng @ 2026-10-15  2:59 UTC (permalink / raw)
  To: Christian Loehle
  Cc: Zhan Xusheng, Peter Zijlstra, Ingo Molnar, Balbir Singh,
	Juri Lelli, Vincent Guittot, Dietmar Eggemann,
	Valentin Schneider, Josh Snyder, Kayra Cizmeci, linux-kernel

On 10/14/26 17:12, Christian Loehle wrote:
> Start accounting in try_to_block_task(), when the current task commits
> to blocking, before delayed dequeue or proxy execution can retain it on
> the runqueue. End accounting when clearing is_blocked, including queued
> wakeups and the proxy execution paths that let the current task continue.

This is the right place, and it subsumes my patch. Please drop mine
rather than queue both.

try_to_block_task() has only one caller, __schedule(), which passes prev,
and prev is rq->curr there. So @p is current, the argument-less
delayacct_blkio_start() is correct once the call lives there, and the
signature change I proposed is unnecessary.

The two also conflict: my patch rewrites the sched.h line yours deletes,
so applying mine first makes your sched.h hunk fail, and the new call in
try_to_block_task() would not compile against the changed prototype.

Reviewed-by: Zhan Xusheng <zhanxusheng@xiaomi.com>

One question. rq->nr_iowait stays in __block_task(), so the two now
measure different intervals: blkio delay from the commit to block,
nr_iowait from the actual dequeue. A task woken while still delayed is
accounted a blkio delay but was never counted in nr_iowait, so
procs_blocked in /proc/stat and the iowait time derived from
nr_iowait_cpu() still lose that window. The counter stays balanced,
because such a wakeup returns through ttwu_runnable() and never reaches
the atomic_dec() in ttwu_do_activate(), so this is not a leak. Is leaving
nr_iowait behind deliberate, or a follow-up?

Thanks,
Zhan Xusheng

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

* Re: [PATCH] sched: Account blkio delay independently of delayed dequeue
  2026-10-15  2:59     ` Zhan Xusheng
@ 2026-10-15 10:40       ` Christian Loehle
  2026-10-15 11:37         ` [PATCHv2] sched: Account I/O waits " Christian Loehle
  0 siblings, 1 reply; 9+ messages in thread
From: Christian Loehle @ 2026-10-15 10:40 UTC (permalink / raw)
  To: Zhan Xusheng
  Cc: Zhan Xusheng, Peter Zijlstra, Ingo Molnar, Balbir Singh,
	Juri Lelli, Vincent Guittot, Dietmar Eggemann,
	Valentin Schneider, Josh Snyder, Kayra Cizmeci, linux-kernel

On 10/15/26 03:59, Zhan Xusheng wrote:
> On 10/14/26 17:12, Christian Loehle wrote:
>> Start accounting in try_to_block_task(), when the current task commits
>> to blocking, before delayed dequeue or proxy execution can retain it on
>> the runqueue. End accounting when clearing is_blocked, including queued
>> wakeups and the proxy execution paths that let the current task continue.
> 
> This is the right place, and it subsumes my patch. Please drop mine
> rather than queue both.
> 
> try_to_block_task() has only one caller, __schedule(), which passes prev,
> and prev is rq->curr there. So @p is current, the argument-less
> delayacct_blkio_start() is correct once the call lives there, and the
> signature change I proposed is unnecessary.
> 
> The two also conflict: my patch rewrites the sched.h line yours deletes,
> so applying mine first makes your sched.h hunk fail, and the new call in
> try_to_block_task() would not compile against the changed prototype.
> 
> Reviewed-by: Zhan Xusheng <zhanxusheng@xiaomi.com>
> 
> One question. rq->nr_iowait stays in __block_task(), so the two now
> measure different intervals: blkio delay from the commit to block,
> nr_iowait from the actual dequeue. A task woken while still delayed is
> accounted a blkio delay but was never counted in nr_iowait, so
> procs_blocked in /proc/stat and the iowait time derived from
> nr_iowait_cpu() still lose that window. The counter stays balanced,
> because such a wakeup returns through ttwu_runnable() and never reaches
> the atomic_dec() in ttwu_do_activate(), so this is not a leak. Is leaving
> nr_iowait behind deliberate, or a follow-up?

Your observation is correct.
First of all we're hoping that /proc/stat isn't observed at such granularity
to matter, but that's not really a good enough reason to leave it behind.
AFAICS we would need to start tracking iowait_cpu in task_struct then?
I'm fine with that and can send a follow-up, I guess Peter feel free to take
whatever you prefer.

(Given the update interval of /proc/stat and the expected delayed-dequeue time
 not 'fixing' it is also an option IMO. I don't see why we need to guarantee
that the blkio delay and nr_iowait need to track the exact same transitions.)

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

* [PATCHv2] sched: Account I/O waits independently of delayed dequeue
  2026-10-15 10:40       ` Christian Loehle
@ 2026-10-15 11:37         ` Christian Loehle
  2026-10-15 12:02           ` Zhan Xusheng
  2026-10-15 12:16           ` Kayra Cizmeci
  0 siblings, 2 replies; 9+ messages in thread
From: Christian Loehle @ 2026-10-15 11:37 UTC (permalink / raw)
  To: Zhan Xusheng, Peter Zijlstra
  Cc: Zhan Xusheng, Peter Zijlstra, Ingo Molnar, Balbir Singh,
	Juri Lelli, Vincent Guittot, Dietmar Eggemann,
	Valentin Schneider, Josh Snyder, Kayra Cizmeci, linux-kernel

Starting blkio delay accounting and incrementing rq->nr_iowait in
__block_task() ties both to removal from the runqueue. With EEVDF delayed
dequeue, this misses the initial part of an I/O sleep, or the entire
sleep if the task wakes before it is dequeued. Such tasks are also
missing from procs_blocked while their dequeue is delayed. Final dequeue
can run in another task's context, causing delayacct_blkio_start() to
timestamp the wrong task.

Start both forms of accounting in try_to_block_task(), when the current
task commits to blocking, before delayed dequeue or proxy execution can
retain it on the runqueue. End both when clearing is_blocked, including
queued wakeups and the proxy execution paths that let the current task
continue. Checking is_blocked excludes wakeups that precede blocking.

Save the CPU charged at the start of the wait. Delayed tasks can migrate
while still queued, and proxy execution can also move blocked tasks.
Decrement the saved CPU's counter on unblock, regardless of the task's
current placement, and remove the old dequeue-based accounting.

CPU iowait continues to classify idle time using the counter on the
charged CPU; it does not accumulate the full per-task blocked duration.

Reported-by: Zhan Xusheng <zhanxusheng@xiaomi.com>
Closes: https://lore.kernel.org/r/20261014081731.1068794-1-zhanxusheng@xiaomi.com
Fixes: 152e11f6df29 ("sched/fair: Implement delayed dequeue")
Signed-off-by: Christian Loehle <christian.loehle@arm.com>
---
 include/linux/sched.h |  2 ++
 kernel/sched/core.c   | 33 ++++++++++++++++++++-------------
 kernel/sched/sched.h  |  5 -----
 3 files changed, 22 insertions(+), 18 deletions(-)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index d35ae49a991f..d558816cc05a 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -880,6 +880,8 @@ struct task_struct {
 	 */
 	int				recent_used_cpu;
 	int				wake_cpu;
+	/* CPU charged while is_blocked && in_iowait. */
+	int				iowait_cpu;
 
 	int				prio;
 	int				static_prio;
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 1fe40de6ebe3..85eba26f00d6 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -636,7 +636,7 @@ int task_llc(const struct task_struct *p)
  *
  * p->is_blocked <- { 0, 1 }:
  *
- *   is set by try_to_block_task() and cleared by ttwu_do_wakeup() and tracks
+ *   is set by try_to_block_task() and cleared by unblock_task() and tracks
  *   if the task is blocked. Traditionally this would mirror p->on_rq, however
  *   due things like DELAY_DEQUEUE and PROXY_EXEC, this can diverge.
  *
@@ -3737,12 +3737,22 @@ ttwu_stat(struct task_struct *p, int cpu, int wake_flags)
 		__schedstat_inc(p->stats.nr_wakeups_sync);
 }
 
+static inline void unblock_task(struct task_struct *p)
+{
+	if (p->is_blocked && p->in_iowait) {
+		delayacct_blkio_end(p);
+		atomic_dec(&cpu_rq(p->iowait_cpu)->nr_iowait);
+	}
+
+	p->is_blocked = 0;
+}
+
 /*
  * Mark the task runnable.
  */
 static inline void ttwu_do_wakeup(struct task_struct *p)
 {
-	p->is_blocked = 0;
+	unblock_task(p);
 	WRITE_ONCE(p->__state, TASK_RUNNING);
 	trace_sched_wakeup(p);
 }
@@ -3839,10 +3849,6 @@ ttwu_do_activate(struct rq *rq, struct task_struct *p, int wake_flags,
 		en_flags |= ENQUEUE_RQ_SELECTED;
 	if (wake_flags & WF_MIGRATED)
 		en_flags |= ENQUEUE_MIGRATED;
-	else if (p->in_iowait) {
-		delayacct_blkio_end(p);
-		atomic_dec(&task_rq(p)->nr_iowait);
-	}
 
 	activate_task(rq, p, en_flags);
 	wakeup_preempt(rq, p, wake_flags);
@@ -4415,11 +4421,6 @@ int try_to_wake_up(struct task_struct *p, unsigned int state, int wake_flags)
 
 		cpu = select_task_rq(p, p->wake_cpu, &wake_flags);
 		if (task_cpu(p) != cpu) {
-			if (p->in_iowait) {
-				delayacct_blkio_end(p);
-				atomic_dec(&task_rq(p)->nr_iowait);
-			}
-
 			wake_flags |= WF_MIGRATED;
 			psi_ttwu_dequeue(p);
 			set_task_cpu(p, cpu);
@@ -6762,6 +6763,12 @@ static bool try_to_block_task(struct rq *rq, struct task_struct *p,
 
 	p->is_blocked = 1;
 
+	if (p->in_iowait) {
+		p->iowait_cpu = cpu_of(rq);
+		atomic_inc(&rq->nr_iowait);
+		delayacct_blkio_start();
+	}
+
 	/*
 	 * We check should_block after signal_pending because we
 	 * will want to wake the task in that case. But if
@@ -6934,7 +6941,7 @@ find_proxy_task(struct rq *rq, struct task_struct *donor, struct rq_flags *rf)
 		if (!mutex) {
 			clear_task_blocked_on(p, mutex);
 			if (task_current(rq, p)) {
-				p->is_blocked = 0;
+				unblock_task(p);
 				return p;
 			}
 			goto deactivate;
@@ -6970,7 +6977,7 @@ find_proxy_task(struct rq *rq, struct task_struct *donor, struct rq_flags *rf)
 			 */
 			__clear_task_blocked_on(p, NULL);
 			if (task_current(rq, p)) {
-				p->is_blocked = 0;
+				unblock_task(p);
 				return p;
 			}
 			goto deactivate;
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 4c25fbe84fb5..eaad943a27a0 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -3072,11 +3072,6 @@ static inline void __block_task(struct rq *rq, struct task_struct *p)
 	if (p->sched_contributes_to_load)
 		rq->nr_uninterruptible++;
 
-	if (p->in_iowait) {
-		atomic_inc(&rq->nr_iowait);
-		delayacct_blkio_start();
-	}
-
 	ASSERT_EXCLUSIVE_WRITER(p->on_rq);
 
 	/*

-- 
2.34.1

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

* Re: [PATCHv2] sched: Account I/O waits independently of delayed dequeue
  2026-10-15 11:37         ` [PATCHv2] sched: Account I/O waits " Christian Loehle
@ 2026-10-15 12:02           ` Zhan Xusheng
  2026-10-15 12:16           ` Kayra Cizmeci
  1 sibling, 0 replies; 9+ messages in thread
From: Zhan Xusheng @ 2026-10-15 12:02 UTC (permalink / raw)
  To: Christian Loehle
  Cc: Zhan Xusheng, Peter Zijlstra, Ingo Molnar, Balbir Singh,
	Juri Lelli, Vincent Guittot, Dietmar Eggemann,
	Valentin Schneider, Josh Snyder, Kayra Cizmeci, linux-kernel

From: Zhan Xusheng <zhanxusheng@xiaomi.com>

On 10/15/26 12:37, Christian Loehle wrote:
> Save the CPU charged at the start of the wait. Delayed tasks can migrate
> while still queued, and proxy execution can also move blocked tasks.
> Decrement the saved CPU's counter on unblock, regardless of the task's
> current placement, and remove the old dequeue-based accounting.

This addresses what I reported, and the nr_iowait pairing holds for the
paths I could find, including the should_block == false one where the
decrement comes from find_proxy_task().

Reviewed-by: Zhan Xusheng <zhanxusheng@xiaomi.com>

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

* Re: [PATCHv2] sched: Account I/O waits independently of delayed dequeue
  2026-10-15 11:37         ` [PATCHv2] sched: Account I/O waits " Christian Loehle
  2026-10-15 12:02           ` Zhan Xusheng
@ 2026-10-15 12:16           ` Kayra Cizmeci
  1 sibling, 0 replies; 9+ messages in thread
From: Kayra Cizmeci @ 2026-10-15 12:16 UTC (permalink / raw)
  To: Christian Loehle
  Cc: Zhan Xusheng, Peter Zijlstra, Zhan Xusheng, Ingo Molnar,
	Balbir Singh, Juri Lelli, Vincent Guittot, Dietmar Eggemann,
	Valentin Schneider, Josh Snyder, linux-kernel

On Thu, Oct 15, 2026 at 12:37:54PM +0100, Christian Loehle wrote:
Normally I wouldn't add these. But I changed my setup. It adds
these itself. Not a big fan of changes but this is way more easier...

> ---
>  include/linux/sched.h |  2 ++
>  kernel/sched/core.c   | 33 ++++++++++++++++++++-------------
>  kernel/sched/sched.h  |  5 -----
>  3 files changed, 22 insertions(+), 18 deletions(-)
> 
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index d35ae49a991f..d558816cc05a 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -880,6 +880,8 @@ struct task_struct {
>  	 */
>  	int				recent_used_cpu;
>  	int				wake_cpu;
> +	/* CPU charged while is_blocked && in_iowait. */
> +	int				iowait_cpu;
>  
>  	int				prio;
>  	int				static_prio;

I wondered if the charge could follow the task while it changes CPUs,
but I think that it would break down more easily than this.

Still, there could be a better approach but right now, I can't find
any. Maybe there's a section that we could reuse? But IDK.

> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 1fe40de6ebe3..85eba26f00d6 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -636,7 +636,7 @@ int task_llc(const struct task_struct *p)
>   *
>   * p->is_blocked <- { 0, 1 }:
>   *
> - *   is set by try_to_block_task() and cleared by ttwu_do_wakeup() and tracks
> + *   is set by try_to_block_task() and cleared by unblock_task() and tracks
>   *   if the task is blocked. Traditionally this would mirror p->on_rq, however
>   *   due things like DELAY_DEQUEUE and PROXY_EXEC, this can diverge.
>   *
> @@ -3737,12 +3737,22 @@ ttwu_stat(struct task_struct *p, int cpu, int wake_flags)
>  		__schedstat_inc(p->stats.nr_wakeups_sync);
>  }
>  
> +static inline void unblock_task(struct task_struct *p)
> +{
> +	if (p->is_blocked && p->in_iowait) {
> +		delayacct_blkio_end(p);
> +		atomic_dec(&cpu_rq(p->iowait_cpu)->nr_iowait);
> +	}
> +
> +	p->is_blocked = 0;
> +}
> +
>  /*
>   * Mark the task runnable.
>   */
>  static inline void ttwu_do_wakeup(struct task_struct *p)
>  {
> -	p->is_blocked = 0;
> +	unblock_task(p);
>  	WRITE_ONCE(p->__state, TASK_RUNNING);
>  	trace_sched_wakeup(p);
>  }
> @@ -3839,10 +3849,6 @@ ttwu_do_activate(struct rq *rq, struct task_struct *p, int wake_flags,
>  		en_flags |= ENQUEUE_RQ_SELECTED;
>  	if (wake_flags & WF_MIGRATED)
>  		en_flags |= ENQUEUE_MIGRATED;
> -	else if (p->in_iowait) {
> -		delayacct_blkio_end(p);
> -		atomic_dec(&task_rq(p)->nr_iowait);
> -	}
>  
>  	activate_task(rq, p, en_flags);
>  	wakeup_preempt(rq, p, wake_flags);
> @@ -4415,11 +4421,6 @@ int try_to_wake_up(struct task_struct *p, unsigned int state, int wake_flags)
>  
>  		cpu = select_task_rq(p, p->wake_cpu, &wake_flags);
>  		if (task_cpu(p) != cpu) {
> -			if (p->in_iowait) {
> -				delayacct_blkio_end(p);
> -				atomic_dec(&task_rq(p)->nr_iowait);
> -			}
> -
>  			wake_flags |= WF_MIGRATED;
>  			psi_ttwu_dequeue(p);
>  			set_task_cpu(p, cpu);
> @@ -6762,6 +6763,12 @@ static bool try_to_block_task(struct rq *rq, struct task_struct *p,
>  
>  	p->is_blocked = 1;
>  
> +	if (p->in_iowait) {
> +		p->iowait_cpu = cpu_of(rq);
> +		atomic_inc(&rq->nr_iowait);
> +		delayacct_blkio_start();
> +	}
> +

So now we increase this nr_iowait on try_to_block_task(), and decrease it on unblock_task().

Also, there's a wrong comment about this in here:

 /*
  * This task is about to go to sleep on IO. Increment rq->nr_iowait so 
  * that process accounting knows that this is a task in IO wait state.
  */
 long __sched io_schedule_timeout(long timeout)
 {
 	int token;
 	long ret;
 
 	token = io_schedule_prepare();
 	ret = schedule_timeout(timeout);
 	io_schedule_finish(token);
 
 	return ret;
 }

This doesn't even touch nr_iowait, so this is stale.
Maybe remove this comment or just correct this while in this?

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

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

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

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-14  8:17 [PATCH] delayacct: Account blkio start on the correct task Zhan Xusheng
2026-10-14 15:20 ` Christian Loehle
2026-10-14 16:12   ` [PATCH] sched: Account blkio delay independently of delayed dequeue Christian Loehle
2026-10-14 16:55     ` Kayra Cizmeci
2026-10-15  2:59     ` Zhan Xusheng
2026-10-15 10:40       ` Christian Loehle
2026-10-15 11:37         ` [PATCHv2] sched: Account I/O waits " Christian Loehle
2026-10-15 12:02           ` Zhan Xusheng
2026-10-15 12:16           ` 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®