[PATCH 1/1 -v3] sched/fair: Sort out 'blocked_load*' namespace noise

[Ingo Molnar]

* Vincent Guittot <vincent.guittot@xxxxxxxxxx> wrote:

> On Tue, 2 Dec 2025 at 10:35, Ingo Molnar <mingo@xxxxxxxxxx> wrote:
> >
> >
> > * Vincent Guittot <vincent.guittot@xxxxxxxxxx> wrote:
> >
> > > On Tue, 2 Dec 2025 at 09:13, Ingo Molnar <mingo@xxxxxxxxxx> wrote:
> > > >
> > > > There's three separate, independent pieces of logic in the
> > > > scheduler that are named 'has_blocked':
> > > >
> > > >    1) nohz.has_blocked,
> > > >    2) rq->has_blocked_load - both of these relate to NOHZ balancing,
> > > >
> > > >    3) and cfs_rq_has_blocked(), which operates on SMP load-balancing
> > > >       averages.
> > > >
> > > > While reviewing this code I noticed a couple of inconsistencies:
> > > >
> > > >   - nohz.has_blocked sometimes gets handled via a local variable
> > > >     that is named 'has_blocked_load' - but it's the runqueue
> > > >     that has the has_blocked_load field, not the nohz structure ...
> > > >
> > > >   - The cfs_rq_has_blocked() function does SMP load-balancing and
> > > >     has no relation to NOHZ has_blocked logic.
> > > >
> > > >   - The update_blocked_load_status() function, which sets the
> > > >     rq->has_blocked_load field, has a parameter named 'has_blocked',
> > > >     but that's the field name of the nohz structure.
> > > >
> > > > To sort all of this out, standardize on 3 distinct patterns:
> > > >
> > > >   (1) nohz.has_blocked related functions and variables use the
> > > >       'has_blocked' nomenclature,
> > > >
> > > >   (2) rq->has_blocked_load related functions and variables use
> > > >       'has_blocked_load',
> > > >
> > > >   (3) and cfs_rq_has_blocked() uses 'has_blocked_load_avg'.
> > >
> > > They are all implementing the same feature: update the blocked pelt
> > > signal of idle rqs.
> >
> > Yeah, should have said 3 separate layers of logic that
> > each deal with the same thing, when writing the
> > changelog I missed how update_blocked_load_status()
> > feeds into rq->has_blocked_load via !done PELT signal
> > we get back from the load-balancers and didn't look
> > further. :-/
> >
> > > If we want some renaming, we should use the same naming for all to
> > > show that it's all about the same thing
> > >
> > > nohz.has_blocked_load()
> > > cfs_rq_has_blocked_load()
> > > rq->has_blocked_load()
> >
> > I'd still argue that greppability of the 3 layers might
> > have a small code readability value:
> >
> >   git grep 'has_blocked\>'                kernel/sched/
> >   git grep 'has_blocked_load\>'           kernel/sched/
> >   git grep 'has_blocked_load_avg\>'       kernel/sched/
> >
> > ... and I've fixed up the changelogs to say:
> >
> >         There's three separate layers of logic in the scheduler that
> >         deal with 'has_blocked' handling of the NOHZ code:
> >
> >           (1) nohz.has_blocked,
> >           (2) rq->has_blocked_load, deal with NOHZ idle balancing,
> >           (3) and cfs_rq_has_blocked(), which is part of the layer
> >               that is passing the SMP load-balancing signal to the
> >               NOHZ layers.
> >
> >         To make it easier to separate them, split these 3 shared-mixed
> >         uses of 'has_blocked' name patterns into 3 distinct and greppable
> >         patterns:
> >
> >           (1) nohz.has_blocked related functions and variables use
> >               'has_blocked',
> >
> >           (2) rq->has_blocked_load related functions and variables use
> >               'has_blocked_load',
> >
> >           (3) and cfs_rq_has_blocked() uses 'has_blocked_load_avg'.
> >
> > ... but if you still object to that notion, we can also
> > do your suggestion - see the patch below. Both variants
> > are fine to me, no strong preferences, as long as the
> > names remove the existing random noise. :-)
> >
> > In fact, on a second thought, I slightly prefer your
> > suggestion, as 'has_blocked_load' has a proper noun.
> 
> I would prefer using 'has_blocked_load' as I find it easier to get
> that it's the same info saved in different places
> 
> Thanks

Okay, agreed - the reworked -v3 version is attached. 
I've optimistically added your Reviewed-by tag as well. :-)

Thanks,

	Ingo

- Change from -v2: also rename to cfs_rq_has_blocked_load().

===================>