mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: Hui Su <sh_def@163.com>
Cc: mingo@redhat.com, juri.lelli@redhat.com,
	vincent.guittot@linaro.org, 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
Subject: Re: [PATCH] sched/fair: Avoid overflow in place_entity()
Date: Thu, 15 Oct 2026 11:40:08 +0200	[thread overview]
Message-ID: <20261015094008.GW4120091@noisy.programming.kicks-ass.net> (raw)
In-Reply-To: <20260928124754.1692077-1-sh_def@163.com>

On Mon, Sep 28, 2026 at 09:47:54PM +0900, Hui Su wrote:
> place_entity() inflates an entity's virtual lag with
> 
>     lag * (load + weight) / load
> 
> Commit 4823725d9d1d ("sched/fair: Increase weight bits for avg_vruntime")
> replaced the scaled-down weights used in this calculation with
> avg_vruntime_weight(), removing the arithmetic headroom provided by
> scale_load_down().
> 
> The multiplication can overflow s64 before the division even when the
> final quotient is representable. A task's hierarchical effective weight
> can be as small as 2 after avg_vruntime_weight() scaling under group
> scheduling, while entity_lag() bounds |vlag| by
> 
>     calc_delta_fair(cfs_rq_max_slice(cfs_rq) + TICK_NSEC, se)
> 
> For example, with HZ=1000, NICE_0_LOAD=1048576, and the minimum scaled
> hierarchical weight scale_load(2)=2048, this bound is 891289600. With 114
> maximum-weight entities, the runqueue load is 10361604096. The
> intermediate product is:
> 
>     891289600 * (10361604096 + 2)
>         = 9235189971864780800
>         > S64_MAX
> 
> The signed overflow corrupts the entity's placement.
> 
> Rewrite
> 
>     lag * (load + weight) / load
> 
> as
> 
>     lag + lag * weight / load
> 
> The two expressions are equivalent, but the latter avoids multiplying the
> virtual lag by the total runqueue weight. entity_lag() bounds vlag against
> the entity's effective weight, so the remaining lag * weight product stays
> bounded. rescale_entity() preserves this scale across h_load changes and
> already relies on the same product when rescaling vlag.
> 
> Keep the original zero-load warning semantics by multiplying lag by weight
> after replacing the divisor with one.
> 
> Fixes: 4823725d9d1d ("sched/fair: Increase weight bits for avg_vruntime")
> Signed-off-by: Hui Su <sh_def@163.com>
> ---

Indeed so; however I've rewritten things like below. Hope that works for
you.

---
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 7455a83a6a99..6ce7233cd339 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -6266,12 +6266,20 @@ place_entity(struct cfs_rq *cfs_rq, struct sched_entity *se, int flags)
 		load = cfs_rq->sum_weight;
 		if (curr && curr->on_rq)
 			load += avg_vruntime_weight(cfs_rq, curr->h_load.weight);
-
-		weight = avg_vruntime_weight(cfs_rq, se->h_load.weight);
-		lag *= load + weight;
 		if (WARN_ON_ONCE(!load))
 			load = 1;
-		lag = div64_long(lag, load);
+
+		weight = avg_vruntime_weight(cfs_rq, se->h_load.weight);
+
+		/*
+		 * Rewrite, because the product (W + w_i)*vl'_i might
+		 * overflow s64:
+		 *
+		 *   vl_i = (W + w_i)*vl'_i / W
+		 *        = (W*vl'_i + w_i*vl'_i) / W
+		 *        = vl'_i + w_i*vl'_i/W
+		 */
+		lag += div64_long(lag * weight, load);
 
 		/*
 		 * A heavy entity (relative to the tree) will pull the

  reply	other threads:[~2026-10-15  9:40 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-28 12:47 Hui Su
2026-10-15  9:40 ` Peter Zijlstra [this message]
2026-10-15 10:10   ` Hui Su
2026-10-16  7:55 ` [tip: sched/urgent] " tip-bot2 for Hui Su

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=20261015094008.GW4120091@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=sh_def@163.com \
    --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®