All of
 help / color / mirror / Atom feed
From: Vincent Guittot <>
To: Linus Torvalds <>
Cc: Borislav Petkov <>,
	kuyo chang <>,
	"Peter Zijlstra (Intel)" <>,
	Dietmar Eggemann <>,
	x86-ml <>, lkml <>
Subject: Re: [GIT PULL] sched/urgent for 5.18-rc4
Date: Sun, 24 Apr 2022 21:31:42 +0200	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

On Sun, 24 Apr 2022 at 21:00, Linus Torvalds
<> wrote:
> On Sun, Apr 24, 2022 at 2:55 AM Borislav Petkov <> wrote:
> >
> > - Fix a corner case when calculating sched runqueue variables
> This worries me.
> It now does:
> +       if (se_weight(se) < se->avg.load_sum)
> +               se->avg.load_sum = div_u64(se->avg.load_sum, se_weight(se));
> and at no point does it check if se_weight(se) is zero.
> It *used* to check for that divide-by-zero issue, so from what I can
> tell, a zero se_weight() is actually possible.
> Now, it's entirely possible that no, se_weight() can never go down to
> zero. But it's not obvious,. and the commit message doesn't mention
> this change at all.
> So I pulled, but then after looking at it I unpulled again in the
> hopes that somebody will clarify the issue for me.
> And scale_load_down() (in se_weight()) does try to make the result be
> at least 2 on 64-bit, but only if the original wasn't zero. Very
> confusing.
> So can somebody please tell me why se_weight() cannot be 0, and why we
> _used_ to check for zero? Because that commit sure as heck doesn't
> explain it.

For task, weight can't be null
For task group, weight is initialized to nice 0 in init_tg_cfs_entry()
and then it's clamp in calc_group_shares in order to not be null
Then since 26cf52229efc ("sched: Avoid scale real weight down to
zero"), scale_load_down can't return null value.

In fact, the condition if (se_weight(se)) was not needed any more and
should have been removed with commit 26cf52229efc

> And - as usual with the -tip tree - the "Link:" thing is almost
> entirely pointless. It doesn't actually point to any discussion of the
> problems, it only points to the patch submission.
> I realize that is convenient for automation, but it's really not
> generally a very useful link. It would be much more useful to link to
> whatever problem report that actually *causes* the submission, not to
> the submission itself. We already see the end result in the commit,
> it's the "how did we get here" that is the most interesting part.
> And no, I don't see any explanation for "why se_weight() cannot be
> zero" in that submission thread either.
> Somebody please hit me over the head with a clue bat.
>                  Linus

  reply	other threads:[~2022-04-24 19:31 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-24  9:55 Borislav Petkov
2022-04-24 18:59 ` Linus Torvalds
2022-04-24 19:31   ` Vincent Guittot [this message]
2022-04-24 20:35 ` pr-tracker-bot

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:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \ \ \ \ \ \
    --subject='Re: [GIT PULL] sched/urgent for 5.18-rc4' \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.