From: Vincent Guittot <firstname.lastname@example.org> To: Linus Torvalds <email@example.com> Cc: Borislav Petkov <firstname.lastname@example.org>, kuyo chang <email@example.com>, "Peter Zijlstra (Intel)" <firstname.lastname@example.org>, Dietmar Eggemann <email@example.com>, x86-ml <firstname.lastname@example.org>, lkml <email@example.com> 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 <firstname.lastname@example.org> wrote: > > On Sun, Apr 24, 2022 at 2:55 AM Borislav Petkov <email@example.com> 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
next prev parent 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 \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --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.