mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] sched/stats: Fix run_delay over-count for proxy-migrated blocked donors
@ 2026-10-25 13:07 albin_yang
  2026-10-25 19:28 ` Kayra Cizmeci
  2026-10-26  5:46 ` K Prateek Nayak
  0 siblings, 2 replies; 6+ messages in thread
From: albin_yang @ 2026-10-25 13:07 UTC (permalink / raw)
  To: peterz, mingo, juri.lelli, vincent.guittot
  Cc: dietmar.eggemann, rostedt, bsegall, mgorman, vschneid,
	kprateek.nayak, chen.yu, kayracizmeci, linux-kernel, albin_yang,
	albinwyang

From: Wei Yang <albinwyang@tencent.com>

When a task is blocked on a kernel mutex (a proxy-exec donor) and its mutex
owner runs on another CPU, find_proxy_task() migrates the still-blocked donor
to the owner's CPU via proxy_migrate_task(). This happens when the donor is
picked as next while not current, and the owner is on_rq and not delayed. The
migration re-attaches the donor with attach_task() -> activate_task(rq, p,
ENQUEUE_NOCLOCK), i.e. without ENQUEUE_RESTORE, so sched_info_enqueue() arms
last_queued even though the donor is not runnable at all -- a fake enqueue.

The real wakeup does not correct it: the donor is still on_rq and is not
sched_delayed, so ttwu_runnable() never calls enqueue_task(), and last_queued
keeps the migration timestamp. sched_info_arrive() then charges the whole
interval from the migration until the donor actually runs, i.e. the time it
spent blocked on the mutex is counted as time spent waiting for a CPU.

Fix by not re-arming last_queued while the task is still blocked on a mutex.
A delayed dequeue sleeper has no blocked_on, so this is an additional
condition rather than a replacement for the se.sched_delayed check, and the
DELAY_DEQUEUE fix is unaffected.

Measured on a 2 CPU VM: the same ~170 ms block is charged ~167 ms of
run_delay when the mutex owner runs on another CPU, and 0.01 ms with this
patch.

Fixes: b049b81bdff6 ("sched: Handle blocked-waiter migration (and return migration)")
Signed-off-by: Wei Yang <albinwyang@tencent.com>
---

Reproducer notes and full numbers (5 iterations each, run_delay is field 2 of
/proc/<tid>/schedstat). A userspace futex mutex never sets blocked_on, so this
needs a small test module exposing a kernel mutex:

  config                          block      run_delay delta   migrated
  -----------------------------   -------    ---------------   --------
  cross-cpu, unfixed              169.9 ms   167.1 ms          5/5
  cross-cpu, DELAY_DEQUEUE fix    169.9 ms   167.1 ms          5/5
  cross-cpu, with this patch      169.8 ms     0.01 ms         5/5
  same-cpu (control)              167.3 ms     0.00 ms         0/5

Sanity check: with the donor runnable and contending for the CPU, run_delay
still grows by ~39 ms over an 80 ms window (5/5), so the gate suppresses only
the fake enqueue, not real waits.

Applies on top of:
  [PATCH v4] sched/stats: Fix run_delay over-count for migrated sched_delayed tasks
  https://lore.kernel.org/all/20261024042753.2276779-1-albin_yang@163.com/

Kept separate from the DELAY_DEQUEUE fix on purpose: the delayed dequeue
over-count exists from v6.12, while this one needs the proxy exec
blocked-waiter migration, which only landed in v7.1. Coupling them would
either fail to build (v6.12.y..v6.16.y, no proxy exec at all) or add a
dependency on code that cannot trigger (v6.17.y..v7.0.y, proxy exec present
but no blocked-waiter migration). The two are also independent: the
DELAY_DEQUEUE fix alone leaves this case at 167.1 ms.

 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 dc626f99ffd9..ef52fbe86bba 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 && !t->se.sched_delayed)
+	if (!t->sched_info.last_queued && !t->se.sched_delayed && !task_is_blocked(t))
 		t->sched_info.last_queued = rq_clock(rq);
 }
 
-- 
2.43.7


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

* Re: [PATCH] sched/stats: Fix run_delay over-count for proxy-migrated blocked donors
  2026-10-25 13:07 [PATCH] sched/stats: Fix run_delay over-count for proxy-migrated blocked donors albin_yang
@ 2026-10-25 19:28 ` Kayra Cizmeci
  2026-10-26 13:53   ` albin_yang
  2026-10-26  5:46 ` K Prateek Nayak
  1 sibling, 1 reply; 6+ messages in thread
From: Kayra Cizmeci @ 2026-10-25 19:28 UTC (permalink / raw)
  To: albin_yang
  Cc: peterz, mingo, juri.lelli, vincent.guittot, dietmar.eggemann,
	rostedt, bsegall, mgorman, vschneid, kprateek.nayak, chen.yu,
	linux-kernel, albinwyang, jstultz

On Sun, Oct 25, 2026 at 09:07:40PM +0800, albin_yang@163.com wrote:

Hi Wei,

> From: Wei Yang <albinwyang@tencent.com>
> 
> When a task is blocked on a kernel mutex (a proxy-exec donor) and its mutex
> owner runs on another CPU, find_proxy_task() migrates the still-blocked donor
> to the owner's CPU via proxy_migrate_task(). This happens when the donor is
> picked as next while not current, and the owner is on_rq and not delayed. The
> migration re-attaches the donor with attach_task() -> activate_task(rq, p,
> ENQUEUE_NOCLOCK), i.e. without ENQUEUE_RESTORE, so sched_info_enqueue() arms
> last_queued even though the donor is not runnable at all -- a fake enqueue.
> 
> The real wakeup does not correct it: the donor is still on_rq and is not
> sched_delayed, so ttwu_runnable() never calls enqueue_task(), and last_queued
> keeps the migration timestamp. sched_info_arrive() then charges the whole
> interval from the migration until the donor actually runs, i.e. the time it

Well it normally should be like that but, sched_info_arrive() only gets
called when we *hit* the CPU. The issue is already that we start the
counter when we are not waiting for a CPU. The counter gets
charged on dequeue.

ttwu_runnable() calls proxy_needs_return() in the proxy case, then that
calls dequeue_block_task() with TASK_WAKING, where we call
sched_info_dequeue(). Or Am I missing something?

> spent blocked on the mutex is counted as time spent waiting for a CPU.
> 
> Fix by not re-arming last_queued while the task is still blocked on a mutex.
> A delayed dequeue sleeper has no blocked_on, so this is an additional
> condition rather than a replacement for the se.sched_delayed check, and the
> DELAY_DEQUEUE fix is unaffected.
> 
> Measured on a 2 CPU VM: the same ~170 ms block is charged ~167 ms of
> run_delay when the mutex owner runs on another CPU, and 0.01 ms with this
> patch.
> 
> Fixes: b049b81bdff6 ("sched: Handle blocked-waiter migration (and return migration)")
> Signed-off-by: Wei Yang <albinwyang@tencent.com>
> ---
> 
> Reproducer notes and full numbers (5 iterations each, run_delay is field 2 of
> /proc/<tid>/schedstat). A userspace futex mutex never sets blocked_on, so this
> needs a small test module exposing a kernel mutex:
> 
>   config                          block      run_delay delta   migrated
>   -----------------------------   -------    ---------------   --------
>   cross-cpu, unfixed              169.9 ms   167.1 ms          5/5
>   cross-cpu, DELAY_DEQUEUE fix    169.9 ms   167.1 ms          5/5
>   cross-cpu, with this patch      169.8 ms     0.01 ms         5/5
>   same-cpu (control)              167.3 ms     0.00 ms         0/5
> 
> Sanity check: with the donor runnable and contending for the CPU, run_delay
> still grows by ~39 ms over an 80 ms window (5/5), so the gate suppresses only
> the fake enqueue, not real waits.
> 
> Applies on top of:
>   [PATCH v4] sched/stats: Fix run_delay over-count for migrated sched_delayed tasks
>   https://lore.kernel.org/all/20261024042753.2276779-1-albin_yang@163.com/

This should be a series instead of separate patches if this goes above.

Also AFAICT this could go wrong once John’s series is applied:

https://lore.kernel.org/lkml/20261023220431.1106611-8-jstultz@google.com/

Adding John to CC.

With this patch, now waiters get dragged from their beloved rq's and
gets in to owner's own list. In this case, on_rq can be false, and
ttwu_runnable() could early return without clearing blocked_on.

Waiters normally wake up with their owners, and wait the CPU for them
so normally we should not start the timer. But on a hierarchy like this,

Where A sleeps and holds the mutex M while B wants it, B with
mutex_lock_interruptible(), and when B gets a kill signal
B need to enqueue separately from A, so it starts to wait.
But blocked_on is set, so B's counter never starts.
This only happens when both pathes are applied.

There's a chance I'm getting something wrong tho. :-).

Thanks,

Kayra

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

* Re: [PATCH] sched/stats: Fix run_delay over-count for proxy-migrated blocked donors
  2026-10-25 13:07 [PATCH] sched/stats: Fix run_delay over-count for proxy-migrated blocked donors albin_yang
  2026-10-25 19:28 ` Kayra Cizmeci
@ 2026-10-26  5:46 ` K Prateek Nayak
  2026-10-26 13:38   ` albin_yang
  1 sibling, 1 reply; 6+ messages in thread
From: K Prateek Nayak @ 2026-10-26  5:46 UTC (permalink / raw)
  To: albin_yang, peterz, mingo, juri.lelli, vincent.guittot
  Cc: dietmar.eggemann, rostedt, bsegall, mgorman, vschneid, chen.yu,
	kayracizmeci, linux-kernel, albinwyang

On 10/25/2026 6:37 PM, albin_yang@163.com wrote:
> diff --git a/kernel/sched/stats.h b/kernel/sched/stats.h
> index dc626f99ffd9..ef52fbe86bba 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 && !t->se.sched_delayed)
> +	if (!t->sched_info.last_queued && !t->se.sched_delayed && !task_is_blocked(t))

I think the sched info bits should be adapted to use the p->is_blocked
indicator instead of adding these endless exceptions.

Could you see if something like below works:

  (Only build tested; Prepared on top of sched-core-2026-10-18)

diff --git a/kernel/sched/stats.h b/kernel/sched/stats.h
index ebe0a7765f98..bd8c5cdfd12a 100644
--- a/kernel/sched/stats.h
+++ b/kernel/sched/stats.h
@@ -233,16 +233,13 @@ static inline void psi_account_irqtime(struct rq *rq, struct task_struct *curr,
 /*
  * We are interested in knowing how long it was from the *first* time a
  * task was queued to the time that it finally hit a CPU, we call this routine
- * from dequeue_task() to account for possible rq->clock skew across CPUs. The
- * delta taken on each CPU would annul the skew.
+ * from dequeue_task() / __schedule() to account for possible rq->clock skew
+ * across CPUs. The delta taken on each CPU would annul the skew.
  */
-static inline void sched_info_dequeue(struct rq *rq, struct task_struct *t)
+static inline void __sched_info_dequeue(struct rq *rq, struct task_struct *t)
 {
 	unsigned long long delta = 0;
 
-	if (!t->sched_info.last_queued)
-		return;
-
 	delta = rq_clock(rq) - t->sched_info.last_queued;
 	t->sched_info.last_queued = 0;
 	t->sched_info.run_delay += delta;
@@ -255,6 +252,15 @@ static inline void sched_info_dequeue(struct rq *rq, struct task_struct *t)
 	rq_sched_info_dequeue(rq, delta);
 }
 
+static inline void sched_info_dequeue(struct rq *rq, struct task_struct *t)
+{
+	/* blocked tasks are handled at sched_info_depart() */
+	if (t->is_blocked)
+		return;
+
+	__sched_info_dequeue(rq, t);
+}
+
 /*
  * Called when a task finally hits the CPU.  We can now calculate how
  * long it was waiting to run.  We also note when it began so that we
@@ -290,7 +296,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->is_blocked)
 		t->sched_info.last_queued = rq_clock(rq);
 }
 
@@ -310,6 +316,9 @@ static inline void sched_info_depart(struct rq *rq, struct task_struct *t)
 
 	if (task_is_running(t))
 		sched_info_enqueue(rq, t);
+
+	if (t->is_blocked)
+		__sched_info_dequeue(rq, t);
 }
 
 /*
---

It is similar to how we handle PSI signals.

>  		t->sched_info.last_queued = rq_clock(rq);
>  }
>  

-- 
Thanks and Regards,
Prateek


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

* Re: [PATCH] sched/stats: Fix run_delay over-count for proxy-migrated blocked donors
  2026-10-26  5:46 ` K Prateek Nayak
@ 2026-10-26 13:38   ` albin_yang
  0 siblings, 0 replies; 6+ messages in thread
From: albin_yang @ 2026-10-26 13:38 UTC (permalink / raw)
  To: kprateek.nayak
  Cc: albin_yang, albinwyang, bsegall, chen.yu, dietmar.eggemann,
	juri.lelli, kayracizmeci, linux-kernel, mgorman, mingo, peterz,
	rostedt, vincent.guittot, vschneid

Hi Prateek,

Thanks - using is_blocked as the single indicator is certainly cleaner than
piling up exceptions.

I rebuilt your diff so it applies cleanly and gave it a run. I'm afraid it
does not work as-is in my testing - two problems:

1) sched_info_depart() now calls __sched_info_dequeue(), which no longer has
   the "if (!last_queued) return;" guard. A blocking task has last_queued == 0
   (sched_info_arrive() cleared it at switch-in), so delta becomes
   rq_clock(rq) - 0, i.e. the whole uptime:

     cross-cpu: blocked 169.80 ms, run_delay delta 169047.52 ms
     same-cpu:  blocked 168.70 ms, run_delay delta 185515.00 ms

   The control is affected too, so this is not proxy specific.

2) sched_info_enqueue() now arms only when t->is_blocked, leaving ordinary
   runnable tasks with no counter at all - my sanity check went from +39 ms to
   0.00 ms in every iteration, including the control. I do not think either
   polarity works: is_blocked is still set during the real wakeup enqueue
   (delayed sleeper: inside ttwu_runnable()'s is_blocked branch; donor: cleared
   only later by ttwu_do_wakeup()). So it can gate the charge side, but the
   arming side needs something already cleared at wakeup - task_is_blocked() or
   sched_delayed.

The delayed reproducer shows 0, but that is easy to misread: it measures only a
delta, and the bogus depart charge lands before the window. Its absolute
run_delay is polluted too. (The 0.00 ms sanity figure is from the proxy
reproducer, which carries that check.)

For reference, !sched_delayed && !task_is_blocked() gives 0.01 ms on the proxy
case with the sanity check intact (+39 ms), and 0/20 on the delayed reproducer.

Happy to retest whatever you come up with.

Thanks,
Wei Yang


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

* Re: [PATCH] sched/stats: Fix run_delay over-count for proxy-migrated blocked donors
  2026-10-25 19:28 ` Kayra Cizmeci
@ 2026-10-26 13:53   ` albin_yang
  2026-10-26 14:48     ` Kayra Cizmeci
  0 siblings, 1 reply; 6+ messages in thread
From: albin_yang @ 2026-10-26 13:53 UTC (permalink / raw)
  To: kayracizmeci
  Cc: albin_yang, albinwyang, bsegall, chen.yu, dietmar.eggemann,
	jstultz, juri.lelli, kprateek.nayak, linux-kernel, mgorman,
	mingo, peterz, rostedt, vincent.guittot, vschneid

Hi Kayra,

Thanks for your review.

> ... sched_info_dequeue() ... Or Am I missing something?

No, you're right: the fake enqueue starts the counter at migration time, and
the bill is settled later in sched_info_dequeue() on the proxy_needs_return()
path (dequeue_block_task() with TASK_WAKING). The ~167 ms I measured is that
delta. I had written sched_info_arrive() charging it directly, which is wrong -
I'll fix the description.

> This should be a series instead of separate patches if this goes above.

Agreed - I'll resend as [PATCH v5 1/2] (DELAY_DEQUEUE fix) + [PATCH v5 2/2]
(this one).

> ... when B gets a kill signal B need to enqueue separately from A, so it
> starts to wait. But blocked_on is set, so B's counter never starts.

A case I hadn't considered, and my sanity check does not cover it: I only
verified that a donor waking normally and then contending for the CPU still
accumulates run_delay. A waiter enqueuing separately from its owner does
genuinely start waiting for a CPU.

So let me ask first: in that path, is blocked_on really still set when B is
enqueued separately? If yes, !task_is_blocked() would under-count B, and the
options as I see them are:

  a) a predicate that is also cleared on that path,
  b) clear blocked_on there (proxy-exec territory - for you/John/Peter),
  c) hold this patch until John's series lands and re-evaluate then,
  d) fix it at the source instead: give proxy_migrate_task() DEQUEUE_SAVE /
     ENQUEUE_RESTORE so the migration never touches sched_info at all. That
     sidesteps the blocked_on question entirely, but it also affects psi, rt,
     dl and sched_ext, so I'd want your/John's input first.

Do you know whether blocked_on is still set there? If it is, I'd still prefer
to take this now, since the under-count only shows up once John's series is in,
and revisit (or adjust) then - unless you'd rather wait.

Adding John to CC.

Thanks,
Wei Yang


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

* Re: [PATCH] sched/stats: Fix run_delay over-count for proxy-migrated blocked donors
  2026-10-26 13:53   ` albin_yang
@ 2026-10-26 14:48     ` Kayra Cizmeci
  0 siblings, 0 replies; 6+ messages in thread
From: Kayra Cizmeci @ 2026-10-26 14:48 UTC (permalink / raw)
  To: albin_yang
  Cc: albinwyang, bsegall, chen.yu, dietmar.eggemann, jstultz,
	juri.lelli, kprateek.nayak, linux-kernel, mgorman, mingo, peterz,
	rostedt, vincent.guittot, vschneid

On Mon, Oct 26, 2026 at 09:53:51PM +0800, albin_yang@163.com wrote:
> Hi Kayra,
> 
> Thanks for your review.
> 
> > ... sched_info_dequeue() ... Or Am I missing something?
> 
> No, you're right: the fake enqueue starts the counter at migration time, and
> the bill is settled later in sched_info_dequeue() on the proxy_needs_return()
> path (dequeue_block_task() with TASK_WAKING). The ~167 ms I measured is that
> delta. I had written sched_info_arrive() charging it directly, which is wrong -
> I'll fix the description.
> 
> > This should be a series instead of separate patches if this goes above.
> 
> Agreed - I'll resend as [PATCH v5 1/2] (DELAY_DEQUEUE fix) + [PATCH v5 2/2]
> (this one).
> 
> > ... when B gets a kill signal B need to enqueue separately from A, so it
> > starts to wait. But blocked_on is set, so B's counter never starts.
> 
> A case I hadn't considered, and my sanity check does not cover it: I only
> verified that a donor waking normally and then contending for the CPU still
> accumulates run_delay. A waiter enqueuing separately from its owner does
> genuinely start waiting for a CPU.
> 
> So let me ask first: in that path, is blocked_on really still set when B is
> enqueued separately? If yes, !task_is_blocked() would under-count B, and the
> options as I see them are:

AFAICT from the code, yes. BTW, I thought about this and I think a
separate fix would be better clearing blocked_on in this case. Since
the task is really waiting for CPU time. Separate from its owner.

Or am I missing something and this accually does not works?

> 
>   a) a predicate that is also cleared on that path,
>   b) clear blocked_on there (proxy-exec territory - for you/John/Peter),
>   c) hold this patch until John's series lands and re-evaluate then,
>   d) fix it at the source instead: give proxy_migrate_task() DEQUEUE_SAVE /
>      ENQUEUE_RESTORE so the migration never touches sched_info at all. That
>      sidesteps the blocked_on question entirely, but it also affects psi, rt,
>      dl and sched_ext, so I'd want your/John's input first.
> 
> Do you know whether blocked_on is still set there? If it is, I'd still prefer
> to take this now, since the under-count only shows up once John's series is in,
> and revisit (or adjust) then - unless you'd rather wait.

It's up to Peter to take it or not so... IDK.

Thanks,

Kayra

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

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

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-25 13:07 [PATCH] sched/stats: Fix run_delay over-count for proxy-migrated blocked donors albin_yang
2026-10-25 19:28 ` Kayra Cizmeci
2026-10-26 13:53   ` albin_yang
2026-10-26 14:48     ` Kayra Cizmeci
2026-10-26  5:46 ` K Prateek Nayak
2026-10-26 13:38   ` 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®