[PATCH] delayacct: Account blkio start on the correct task

[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@xxxxxxxxxx>
---
 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