All of lore.kernel.org
 help / color / mirror / Atom feed
From: Vincent Guittot <vincent.guittot@linaro.org>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Borislav Petkov <bp@suse.de>,
	kuyo chang <kuyo.chang@mediatek.com>,
	"Peter Zijlstra (Intel)" <peterz@infradead.org>,
	Dietmar Eggemann <dietmar.eggemann@arm.com>,
	x86-ml <x86@kernel.org>, lkml <linux-kernel@vger.kernel.org>
Subject: Re: [GIT PULL] sched/urgent for 5.18-rc4
Date: Sun, 24 Apr 2022 21:31:42 +0200	[thread overview]
Message-ID: <CAKfTPtD2QEyZ6ADd5WrwETMOX0XOwJGnVddt7VHgfURdqgOS-Q@mail.gmail.com> (raw)
In-Reply-To: <CAHk-=wjccVKAcK7JmpPpOrqR3fXrfza6dCbCLr9BmTyTasJ2GA@mail.gmail.com>

On Sun, 24 Apr 2022 at 21:00, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> On Sun, Apr 24, 2022 at 2:55 AM Borislav Petkov <bp@suse.de> 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:
  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=CAKfTPtD2QEyZ6ADd5WrwETMOX0XOwJGnVddt7VHgfURdqgOS-Q@mail.gmail.com \
    --to=vincent.guittot@linaro.org \
    --cc=bp@suse.de \
    --cc=dietmar.eggemann@arm.com \
    --cc=kuyo.chang@mediatek.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=peterz@infradead.org \
    --cc=torvalds@linux-foundation.org \
    --cc=x86@kernel.org \
    --subject='Re: [GIT PULL] sched/urgent for 5.18-rc4' \
    /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

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.