lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Sun, 24 Apr 2022 11:59:49 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Borislav Petkov <bp@...e.de>, kuyo chang <kuyo.chang@...iatek.com>,
        "Peter Zijlstra (Intel)" <peterz@...radead.org>,
        Vincent Guittot <vincent.guittot@...aro.org>,
        Dietmar Eggemann <dietmar.eggemann@....com>
Cc:     x86-ml <x86@...nel.org>, lkml <linux-kernel@...r.kernel.org>
Subject: Re: [GIT PULL] sched/urgent for 5.18-rc4

On Sun, Apr 24, 2022 at 2:55 AM Borislav Petkov <bp@...e.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.

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

Powered by blists - more mailing lists