From: Peter Zijlstra <peterz@infradead.org>
To: K Prateek Nayak <kprateek.nayak@amd.com>
Cc: Zhan Xusheng <zhanxusheng1024@gmail.com>,
Ingo Molnar <mingo@redhat.com>,
Vincent Guittot <vincent.guittot@linaro.org>,
Juri Lelli <juri.lelli@redhat.com>,
linux-kernel@vger.kernel.org,
Zhan Xusheng <zhanxusheng@xiaomi.com>,
Dietmar Eggemann <dietmar.eggemann@arm.com>,
Valentin Schneider <vschneid@redhat.com>,
Ben Segall <bsegall@google.com>,
Steven Rostedt <rostedt@goodmis.org>,
Mel Gorman <mgorman@suse.de>
Subject: Re: [PATCH RESEND] sched/fair: Fix overflow in vruntime_eligible()
Date: Tue, 28 Apr 2026 19:35:21 +0200 [thread overview]
Message-ID: <20260428173521.GK3126523@noisy.programming.kicks-ass.net> (raw)
In-Reply-To: <57e94d7e-b8fb-49bc-a914-8a7401e43af3@amd.com>
On Tue, Apr 28, 2026 at 09:47:11PM +0530, K Prateek Nayak wrote:
> (+ scheduler folks)
>
> Hello Zhan,
>
> On 4/28/2026 8:19 PM, Zhan Xusheng wrote:
> > After commit 556146ce5e94 ("sched/fair: Avoid overflow in
> > enqueue_entity()"), place_entity() can shift cfs_rq->zero_vruntime
> > towards a newly enqueued heavy entity. This can make (vruntime -
> > zero_vruntime) very large for other entities and cause key * load in
> > vruntime_eligible() to overflow s64, flipping the eligibility result.
>
> So the commit in question moves the zero_vruntime only when the
> load > sum_weight.
>
> You seem to have found a case where the entity_key() is already large
> enough that moving the zero_vruntime farther will make the eligibility
> check overflow which we were hoping will not be the case.
>
> Do you have a reproducer that fails pick_eevdf() after introduction of
> commit 556146ce5e94? Also, do you see any splats in the dmesg since we
> have a defensive WARN_ON() to catch an overflow.
Right, either a reproducer or a trace showing the values leading up to
this are the absolute minimum you need to provide. That is, you need a
definite description of how you got there, otherwise you cannot judge
the solution is sound.
Anyway... let me construct a worst case.
So if you have this cgroup crap on, then you can have an entity of
weight 2, and vlag should then be bounded by: (slice+TICK_NSEC) *
NICE_0_LOAD, which is around 44 bits as per the comment on entity_key().
The other end is 100*NICE_0_LOAD, so lets wake that, then you get:
{key, weight}[] := {
puny: { (slice + TICK_NSEC) * NICE_0_LOAD, 2 },
max: { 0, 100*NICE_0_LOAD },
}
The avg_vruntime() would end up being very close to 0 (which is
zero_vruntime), so no real help making that more accurate.
vruntime_eligible(puny) ends up with:
avg = 2 * puny.key (+ 0)
weight = 2 + 100 * NICE_0_LOAD
avg >= puny.key * weight
And that is: (slice + TICK_NSEC) * NICE_0_LOAD * NICE_0_LOAD * 100
and yes, I suppose that can exceed 64bit :-(
Can someone double check that? I have a silly head-ache and could easily
have made mistake. Not sure what the best solution is either. I think we
can show avg cannot overflow, and if this multiplication overflows, then
it must be larger, so perhaps the proposed patch is the best option.
Dunno, need clear head :/
next prev parent reply other threads:[~2026-04-28 17:35 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-04-15 14:57 [PATCH] sched/fair: Fix overflow in vruntime_eligible() Zhan Xusheng
2026-04-28 14:49 ` [PATCH RESEND] " Zhan Xusheng
2026-04-28 16:17 ` K Prateek Nayak
2026-04-28 17:35 ` Peter Zijlstra [this message]
2026-04-29 5:03 ` K Prateek Nayak
2026-04-29 7:31 ` Zhan Xusheng
2026-05-01 10:40 ` Peter Zijlstra
2026-05-01 10:48 ` Peter Zijlstra
2026-05-01 13:05 ` David Laight
2026-05-01 13:43 ` Peter Zijlstra
2026-05-04 11:22 ` Peter Zijlstra
2026-05-04 13:16 ` Heiko Carstens
2026-05-04 14:57 ` Peter Zijlstra
2026-05-04 17:40 ` Stefan Schulze Frielinghaus
2026-05-05 10:31 ` Peter Zijlstra
2026-05-05 10:50 ` [tip: sched/urgent] " tip-bot2 for Zhan Xusheng
2026-05-05 14:13 ` tip-bot2 for Zhan Xusheng
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=20260428173521.GK3126523@noisy.programming.kicks-ass.net \
--to=peterz@infradead.org \
--cc=bsegall@google.com \
--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=rostedt@goodmis.org \
--cc=vincent.guittot@linaro.org \
--cc=vschneid@redhat.com \
--cc=zhanxusheng1024@gmail.com \
--cc=zhanxusheng@xiaomi.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