Re: [GIT PULL] sched/urgent for 5.18-rc4

[Linus Torvalds <torvalds@linux-foundation.org>]

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.

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