From: Kayra Cizmeci <kayracizmeci@gmail.com>
To: K Prateek Nayak <kprateek.nayak@amd.com>
Cc: albin_yang@163.com, albinwyang@tencent.com, bsegall@google.com,
chen.yu@linux.dev, dietmar.eggemann@arm.com,
juri.lelli@redhat.com, linux-kernel@vger.kernel.org,
mgorman@suse.de, mingo@redhat.com, peterz@infradead.org,
rostedt@goodmis.org, vincent.guittot@linaro.org,
vschneid@redhat.com
Subject: Re: [PATCH] sched/stats: Fix run_delay over-count for proxy-migrated blocked donors
Date: Tue, 27 Oct 2026 09:11:51 +0300 [thread overview]
Message-ID: <auA7AQ_S9iwPHr4d@scheduler.schedules> (raw)
In-Reply-To: <36f12b2f-9fc8-46e9-8312-536c918722f1@amd.com>
On Tue, Oct 27, 2026 at 09:41:51AM +0530, K Prateek Nayak wrote:
> Hello Wei,
>
> On 10/26/2026 7:08 PM, albin_yang@163.com wrote:
> > 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
>
> Thank you for testing the changes!
>
> > 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.
>
> Ah! That is an oversight. I didn't realise arrive clears the last_queued
> and yes, doing it unconditionally will account the full uptime.
>
> >
> > 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.
>
> Ack! I forgot task migrations. So based on a better look, all we have to
> do i start accounting when the task becomes runnable again (p->is_blocked
> is cleared) and don't account any time when the task is "on_rq" while
> being blocked (p->is_blocked && p->on_rq).
>
> >
> > 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.
>
> So all things considered, I think the following should work unless I'm
> terribly mistaken yet again :-) I've left my reasoning as comments:
>
> diff --git a/kernel/sched/stats.h b/kernel/sched/stats.h
> index ebe0a7765f98..c4c624deb25b 100644
> --- a/kernel/sched/stats.h
> +++ b/kernel/sched/stats.h
> @@ -239,6 +239,15 @@ static inline void psi_account_irqtime(struct rq *rq, struct task_struct *curr,
> static inline void sched_info_dequeue(struct rq *rq, struct task_struct *t)
> {
> unsigned long long delta = 0;
> + /*
> + * If a task is blocked (blocked donor, delayed dequeue task),
> + * do not account the queued time as runnable. A task that is
> + * blocking would have already cleared the "last_queued"
> + * indicator from sched_info_arrive() when it got a chance to
> + * run on the CPU.
> + */
> + if (t->is_blocked)
> + t->sched_info.last_queued = 0;
>
> if (!t->sched_info.last_queued)
> return;
> @@ -290,7 +299,23 @@ 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->is_blocked" is only cleared after the enqueue is done and
> + * the task is woken up. If the task is blocked, update the
> + * "last_queued" timestamp to that of last enqueue - if the task
> + * is still blocked while it leaves the runqueue, this timestamp
> + * is ignored and is cleared at sched_info_dequeue().
> + *
> + * To transition a task back to blocked after wakeup, it has to
> + * run on the CPU and naturally reach a safe preemption point.
> + * sched_info_arrive() will do the correct wait time accounting
> + * based on the last "last_queued".
> + *
> + * Any movement of task after being marked not blocked will be
> + * correctly captured as queued time at enqueue + dequeue
> + * boundaries when "t->is_blocked" cleared.
> + */
> + if (!t->sched_info.last_queued || t->is_blocked)
> t->sched_info.last_queued = rq_clock(rq);
> }
>
> ---
>
> That "t->is_blocked" check in sched_info_enqueue() may not be
> necessary either but I've left it in there for now since I did my
> quick testing with it.
Hi Prateek,
IMO too doing this with is_blocked would be cleaner.
But I think this not works on a case where a donor is in a different CPU
from its owner, like:
A holds the mutex M in CPU0, B is the waiter and its on CPU1.
proxy_migrate_task() gets B to CPU0.
After that, A suddenly wants to goto CPU1. And it goes.
Now B goes back to its home CPU next to A.
Now in here, timer starts.
And suddenly A drops the mutex. Now B is not is_blocked.
Because B is back to its home CPU, there is no dequeue.
Even if there was B is not is_blocked so timer wouldn't be reset.
Normally we need to add the time that task waited for itself.
But in this case we add the time task waited for its mutex + for itself.
Or am I missssssing something?
>
> --
> Thanks and Regards,
> Prateek
>
Thanks,
Kayra
next prev parent reply other threads:[~2026-10-27 6:11 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-10-25 13:07 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
2026-10-27 4:11 ` K Prateek Nayak
2026-10-27 6:11 ` Kayra Cizmeci [this message]
2026-10-27 6:24 ` K Prateek Nayak
2026-10-27 6:52 ` Kayra Cizmeci
2026-10-27 9:45 ` K Prateek Nayak
2026-10-27 9:53 ` Kayra Cizmeci
2026-10-27 9:58 ` K Prateek Nayak
2026-10-27 12:09 ` Peter Zijlstra
2026-10-27 12:13 ` Kayra Cizmeci
2026-10-28 3:40 ` K Prateek Nayak
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=auA7AQ_S9iwPHr4d@scheduler.schedules \
--to=kayracizmeci@gmail.com \
--cc=albin_yang@163.com \
--cc=albinwyang@tencent.com \
--cc=bsegall@google.com \
--cc=chen.yu@linux.dev \
--cc=dietmar.eggemann@arm.com \
--cc=juri.lelli@redhat.com \
--cc=kprateek.nayak@amd.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mgorman@suse.de \
--cc=mingo@redhat.com \
--cc=peterz@infradead.org \
--cc=rostedt@goodmis.org \
--cc=vincent.guittot@linaro.org \
--cc=vschneid@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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®