mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] sched/stats: Fix run_delay over-count for migrated sched_delayed tasks
@ 2026-09-09 13:33 albin_yang
  2026-09-19  3:59 ` albin_yang
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: albin_yang @ 2026-09-09 13:33 UTC (permalink / raw)
  To: peterz, mingo, juri.lelli, vincent.guittot
  Cc: dietmar.eggemann, rostedt, bsegall, mgorman, vschneid,
	kprateek.nayak, linux-kernel, albinwyang

From: Wei Yang <albinwyang@tencent.com>

With DELAY_DEQUEUE, a blocked task stays on the runqueue with
se.sched_delayed set and its sched_info.last_queued is cleared, so the sleep
is not counted into run_delay.

When such a delayed (sleeping) task is migrated across CPUs via the plain
migration paths (move_queued_task / move_queued_task_locked, the latter used
by __migrate_swap_task), activate_task(dst, 0) calls enqueue_task() without
ENQUEUE_RESTORE, re-arming last_queued to the migration timestamp while the
task is still sleeping. The later real wakeup (ENQUEUE_DELAYED) tries to
re-arm last_queued at wakeup time but is suppressed because last_queued is
already non-zero, so sched_info_arrive() folds the whole sleep duration
between migration and wakeup into run_delay.

Fix by not re-arming last_queued for a sched_delayed task in
sched_info_enqueue(). The wakeup path clears sched_delayed before reaching
sched_info_enqueue(), so it still re-arms at the real wakeup time. Plain
runnable tasks and load-balance migrations are unaffected (sched_delayed
tasks are excluded from active load balancing).

Fixes: 152e11f6df29 ("sched/fair: Implement delayed dequeue")
Signed-off-by: Wei Yang <albinwyang@tencent.com>
---
 kernel/sched/stats.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/sched/stats.h b/kernel/sched/stats.h
index ebe0a7765f98..dc626f99ffd9 100644
--- a/kernel/sched/stats.h
+++ b/kernel/sched/stats.h
@@ -290,7 +290,7 @@ static void sched_info_arrive(struct rq *rq, struct task_struct *t)
  */
 static inline void sched_info_enqueue(struct rq *rq, struct task_struct *t)
 {
-	if (!t->sched_info.last_queued)
+	if (!t->sched_info.last_queued && !t->se.sched_delayed)
 		t->sched_info.last_queued = rq_clock(rq);
 }
 
-- 
2.43.7


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

* Re: [PATCH] sched/stats: Fix run_delay over-count for migrated sched_delayed tasks
  2026-09-09 13:33 [PATCH] sched/stats: Fix run_delay over-count for migrated sched_delayed tasks albin_yang
@ 2026-09-19  3:59 ` albin_yang
  2026-09-19  7:58 ` Chen Yu
  2026-09-19 10:24 ` Kayra Cizmeci
  2 siblings, 0 replies; 5+ messages in thread
From: albin_yang @ 2026-09-19  3:59 UTC (permalink / raw)
  To: peterz, mingo, juri.lelli, vincent.guittot
  Cc: dietmar.eggemann, rostedt, bsegall, mgorman, vschneid,
	kprateek.nayak, linux-kernel

Hi,

To make the bug more concrete, I wrote a userspace reproducer
(available on request) that exercises exactly the path described in the
commit.

Setup: the system needs enough CPU pressure for the worker to become
!entity_eligible and take the delayed-dequeue path. In my runs I first
start background load, e.g.:

    stress-ng --cpu $(( $(nproc) - 1 )) &

The reproducer itself also pins a busy competitor thread to CPU0 for the
same reason.

What the reproducer does:

  - A worker thread burns ~80 ms of CPU on CPU0 then usleep()s for
    150 ms. With the background load keeping min_vruntime ahead, when
    the worker sleeps it is !entity_eligible and is kept on the rq with
    se.sched_delayed set (delayed dequeue) - still "sleeping" but not
    really runnable.
  - A separate thread migrates the worker to CPU1 with
    sched_setaffinity(worker_tid, cpu1) ~2 ms after it goes to sleep.
    This takes the plain migration path (activate_task(dst, 0)), which
    re-arms last_queued at the migration time while the task is still
    sleeping.
  - run_delay is read from /proc/<tid>/schedstat right before the
    migration and again after the worker actually wakes up and is
    scheduled.

Observed result (20 iterations; run_delay delta in ms):

    iter   run_delay   expected_err   verdict
      1      148.03      148.12        BUG
      4      148.88      148.93        BUG
      6      148.05      148.12        BUG
      9      148.88      148.93        BUG
     11      148.06      148.12        BUG
     14      148.89      148.93        BUG
     16      148.03      148.08        BUG
     17      148.89      148.94        BUG
   (others)     0.00      148.00        ok

Every migration that lands during the sched_delayed sleep charges
~148 ms of phantom wait into run_delay - i.e. the *entire* sleep
duration between migration and wakeup. With the patch, run_delay
delta stays ~0 in all iterations.

So this is a real, user-visible run_delay over-count, not just a
theoretical one. The fix is the single-line change in
sched_info_enqueue().

Would appreciate a review / any feedback. Happy to share the full
reproducer if that helps.

Original patch:
https://lore.kernel.org/all/20260909133345.1572954-1-albin_yang@163.com/

Thanks,
Wei Yang


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

* Re: [PATCH] sched/stats: Fix run_delay over-count for migrated sched_delayed tasks
  2026-09-09 13:33 [PATCH] sched/stats: Fix run_delay over-count for migrated sched_delayed tasks albin_yang
  2026-09-19  3:59 ` albin_yang
@ 2026-09-19  7:58 ` Chen Yu
  2026-09-19 10:24 ` Kayra Cizmeci
  2 siblings, 0 replies; 5+ messages in thread
From: Chen Yu @ 2026-09-19  7:58 UTC (permalink / raw)
  To: albin_yang
  Cc: peterz, mingo, juri.lelli, vincent.guittot, dietmar.eggemann,
	rostedt, bsegall, mgorman, vschneid, kprateek.nayak,
	linux-kernel, albinwyang

On Wed, Sep 09, 2026 at 09:33:45PM +0800, albin_yang@163.com wrote:
> Date: Wed,  9 Sep 2026 21:33:45 +0800
> From: albin_yang@163.com
> To: peterz@infradead.org, mingo@redhat.com, juri.lelli@redhat.com,
>  vincent.guittot@linaro.org
> Cc: dietmar.eggemann@arm.com, rostedt@goodmis.org, bsegall@google.com,
>  mgorman@suse.de, vschneid@redhat.com, kprateek.nayak@amd.com,
>  linux-kernel@vger.kernel.org, albinwyang@tencent.com
> Subject: [PATCH] sched/stats: Fix run_delay over-count for migrated
>  sched_delayed tasks
> X-Mailer: git-send-email 2.43.7
> 
> From: Wei Yang <albinwyang@tencent.com>
> 
> With DELAY_DEQUEUE, a blocked task stays on the runqueue with
> se.sched_delayed set and its sched_info.last_queued is cleared, so the sleep
> is not counted into run_delay.

Agree.
The sched_info.last_queued was cleared when that blocked task
switched in on the CPU, its last_queued was cleared in sched_info_arrive(),
because the task did not have to wait/delay anymore, it seems to be me that
the "delayed status" has nothing to do with "run_delay". The issue is what is
the realy timestamp of the task starts to wait.

> 
> When such a delayed (sleeping) task is migrated across CPUs via the plain
> migration paths (move_queued_task / move_queued_task_locked, the latter used
> by __migrate_swap_task), 

Right, for all the migration caused by passive/active load balance(attach_task,
with migrate_load type, see can_migrate_task()), NUMA balancing, CPU affinity changes
via sched_setaffinity(move_queued_task), etc, we should not update the "delayed"
task's last_queued field when enqueuing it on a new CPU - afterall it is not ready
for running.

> activate_task(dst, 0) calls enqueue_task() without
> ENQUEUE_RESTORE, re-arming last_queued to the migration timestamp while the
> task is still sleeping. The later real wakeup (ENQUEUE_DELAYED) tries to
> re-arm last_queued at wakeup time but is suppressed because last_queued is
> already non-zero, so sched_info_arrive() folds the whole sleep duration
> between migration and wakeup into run_delay.
> 
> Fix by not re-arming last_queued for a sched_delayed task in
> sched_info_enqueue(). The wakeup path clears sched_delayed before reaching
> sched_info_enqueue(), so it still re-arms at the real wakeup time. Plain
> runnable tasks and load-balance migrations are unaffected (sched_delayed
> tasks are excluded from active load balancing).
> 
> Fixes: 152e11f6df29 ("sched/fair: Implement delayed dequeue")
> Signed-off-by: Wei Yang <albinwyang@tencent.com>

Per my understanding, it looks good to me,

Reviewed-by: Chen Yu <yu.c.chen@intel.com>

thanks,
Chenyu

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

* Re: [PATCH] sched/stats: Fix run_delay over-count for migrated sched_delayed tasks
  2026-09-09 13:33 [PATCH] sched/stats: Fix run_delay over-count for migrated sched_delayed tasks albin_yang
  2026-09-19  3:59 ` albin_yang
  2026-09-19  7:58 ` Chen Yu
@ 2026-09-19 10:24 ` Kayra Cizmeci
  2026-09-20  2:52   ` albin_yang
  2 siblings, 1 reply; 5+ messages in thread
From: Kayra Cizmeci @ 2026-09-19 10:24 UTC (permalink / raw)
  To: albin_yang
  Cc: albinwyang, bsegall, dietmar.eggemann, juri.lelli,
	kprateek.nayak, linux-kernel, mgorman, mingo, peterz, rostedt,
	vincent.guittot, vschneid

> With DELAY_DEQUEUE, a blocked task stays on the runqueue with
> se.sched_delayed set and its sched_info.last_queued is cleared, so the sleep
> is not counted into run_delay.

> When such a delayed (sleeping) task is migrated across CPUs via the plain
> migration paths (move_queued_task / move_queued_task_locked, the latter used
> by __migrate_swap_task), activate_task(dst, 0) calls enqueue_task() without
> ENQUEUE_RESTORE, re-arming last_queued to the migration timestamp while the
> task is still sleeping. The later real wakeup (ENQUEUE_DELAYED) tries to
> re-arm last_queued at wakeup time but is suppressed because last_queued is
> already non-zero, so sched_info_arrive() folds the whole sleep duration
> between migration and wakeup into run_delay.

> Fix by not re-arming last_queued for a sched_delayed task in
> sched_info_enqueue(). The wakeup path clears sched_delayed before reaching
> sched_info_enqueue(), so it still re-arms at the real wakeup time. Plain
> runnable tasks and load-balance migrations are unaffected (sched_delayed
> tasks are excluded from active load balancing).

No? can_migrate_task() skips delayed tasks unless they are both delayed and 
migration_type not equals migrate_load. 

And can_migrate_task() is called from detach_one_task() which
gets called from active_load_balance_cpu_stop(), with a custom
env where the migration type is not set. That means, the migration
type is migrate_load, and can_migrate_task() can return 1 for delayed
tasks on this path. 

> Fixes: 152e11f6df29 ("sched/fair: Implement delayed dequeue")
> Signed-off-by: Wei Yang <albinwyang@tencent.com>

Thanks,
Kayra :>

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

* Re: [PATCH] sched/stats: Fix run_delay over-count for migrated sched_delayed tasks
  2026-09-19 10:24 ` Kayra Cizmeci
@ 2026-09-20  2:52   ` albin_yang
  0 siblings, 0 replies; 5+ messages in thread
From: albin_yang @ 2026-09-20  2:52 UTC (permalink / raw)
  To: kayracizmeci
  Cc: albin_yang, albinwyang, bsegall, dietmar.eggemann, juri.lelli,
	kprateek.nayak, linux-kernel, mgorman, mingo, peterz, rostedt,
	vincent.guittot, vschneid

Hi Kayra,

On Sat, Sep 19, 2026 at 01:24:20PM +0300, Kayra Cizmeci wrote:
> No? can_migrate_task() skips delayed tasks unless they are both delayed and
> migration_type not equals migrate_load.
>
> And can_migrate_task() is called from detach_one_task() which
> gets called from active_load_balance_cpu_stop(), with a custom
> env where the migration type is not set. That means, the migration
> type is migrate_load, and can_migrate_task() can return 1 for delayed
> tasks on this path.

You are right, thanks for catching this.

migrate_load is 0, and active_load_balance_cpu_stop()'s lb_env does not
set .migration_type, so it stays 0 == migrate_load. Hence in
can_migrate_task():

	if ((p->se.sched_delayed) && (env->migration_type != migrate_load))
		return 0;

the condition is false on that path, and a sched_delayed task can indeed
be migrated.

It is also a bit broader than the active balance case: load_balance() ->
calculate_imbalance() sets env->migration_type = migrate_load for some
group types, so regular load balance can hit it too. Either way those
paths re-attach via attach_task() -> activate_task(rq, p, ENQUEUE_NOCLOCK),
again without ENQUEUE_RESTORE, so they hit the same bug.

So the "load-balance migrations are unaffected" part of my commit message
is wrong. The fix itself is unchanged and covers those paths as well; only
the description was wrong. I'll correct it in v2.

Thanks,
Wei Yang


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

end of thread, other threads:[~2026-09-20  2:53 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-09 13:33 [PATCH] sched/stats: Fix run_delay over-count for migrated sched_delayed tasks albin_yang
2026-09-19  3:59 ` albin_yang
2026-09-19  7:58 ` Chen Yu
2026-09-19 10:24 ` Kayra Cizmeci
2026-09-20  2:52   ` albin_yang

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®