From: K Prateek Nayak <kprateek.nayak@amd.com>
To: <albin_yang@163.com>
Cc: <albinwyang@tencent.com>, <bsegall@google.com>,
<chen.yu@linux.dev>, <dietmar.eggemann@arm.com>,
<juri.lelli@redhat.com>, <kayracizmeci@gmail.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:41:51 +0530 [thread overview]
Message-ID: <36f12b2f-9fc8-46e9-8312-536c918722f1@amd.com> (raw)
In-Reply-To: <20261026133831.4111227-1-albin_yang@163.com>
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.
--
Thanks and Regards,
Prateek
next prev parent reply other threads:[~2026-10-27 4:12 UTC|newest]
Thread overview: 15+ 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 [this message]
2026-10-27 6:11 ` Kayra Cizmeci
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
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=36f12b2f-9fc8-46e9-8312-536c918722f1@amd.com \
--to=kprateek.nayak@amd.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=kayracizmeci@gmail.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®