Re: [PATCH v2 2/3] sched/fair: Change likelyhood of nohz.nr_cpus and do stats update if its due

[Shrikanth Hegde]

On 1/6/26 8:33 AM, K Prateek Nayak wrote:
> Hello Shrikanth,
>
> On 1/5/2026 4:09 PM, Shrikanth Hegde wrote:
> > > So cumulatively, including Patch 3, we do:
> > >
> > >       flags = 0;
> > >
> > >       if (READ_ONCE(nohz.has_blocked_load) && ...)
> > >           flags = NOHZ_STATS_KICK;
> > >
> > >       if (time_before(now, nohz.next_balance))
> > >           goto out; /* Checks nohz.idle_cpus_mask in find_new_ilb() ... (1) */
> > >
> > >       if (unlikely(cpumask_empty(nohz.idle_cpus_mask)))
> > >           goto out; /* Still goes to kick_ilb()                     ... (2) */
> > >
> > >       ...
> > >
> > > out:
> > >       if (READ_ONCE(nohz.needs_update))
> > >           flags |= NOHZ_NEXT_KICK;
> > >
> > >       /* assume either NOHZ_STATS_KICK or NOHZ_NEXT_KICK is set */
> > >       kick_ilb()
> > >       {
> > >            if (flags & NOHZ_BALANCE_KICK) /* Not possible */
> > >                ...
> > >
> > >            ilb_cpu = find_new_ilb(); /* Find CPU in nohz.idle_cpus_mask */
> > >
> > >
> > > If we arrive here from (2), we know "nohz.idle_cpus_mask" was empty a
> > > while back and we've not updated any global "nohz" state. If we don't
> > > find an ilb_cpu, we just do:
> > >
> > >           if (ilb_cpu < 0)
> > >               return;
> > >
> > > So why not simply return from (2)?
> >
> > I see, kick_ilb though called will not do a balance since ilb_cpu was not found.
> >
> > I don't want to have that return in between the two out's.
> >
> > How about we do below? When there are no idle CPUs left, both has_blocked_load
> > and needs_update should be reset. no?
> >
> > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> > index 805b53d9709e..fa0e6065bc9c 100644
> > --- a/kernel/sched/fair.c
> > +++ b/kernel/sched/fair.c
> > @@ -12377,6 +12377,15 @@ static inline int find_new_ilb(void)
> >                          return ilb_cpu;
> >          }
> >   
> > +       /* There is no idle CPU left.
> > +        * reset has_blocked_load and needs_update, such that unless
> > +        * some CPU enters idle state, it will not trigger kick_ilb
> > +        */

cpumask_empty check.

> > +       if (READ_ONCE(nohz.has_blocked_load))
> > +               WRITE_ONCE(nohz.has_blocked_load, 0);
> > +       if (READ_ONCE(nohz.needs_update))
> > +               WRITE_ONCE(nohz.needs_update, 0);
> > +
>
> I'm slightly skeptical - find_new_ilb() will also fail to find any CPU
> if idle_cpu() returns false momentarily.

Yes. I even thought to put cpumask_empty check here and then do these updates.
But since there might be a window where a remote CPU may be going idle,
might race with it and possibly lose a chance for idle balance.

Since it is extreme case of 100% busy, all these checks may not be necessary.
Will put a return instead :)

Thanks for pointing that out.

> Those CPUs can again go back to idle without updating
> "nohz.has_blocked_load" - tick_nohz_idle_stop_tick() will skip
> nohz_balance_enter_idle() if the CPU already had TS_FLAG_STOPPED set.
>
> For those cases, we'll need to retain the "nohz" state as is until a
> suitable ILB CPU can be found - this is the reason why we delay
> clearing the "nohz" state until _nohz_idle_balance() and reconstruct it
> once the CPU is done with idle balancing.
>
> There are also nuances like the smp_mb__after_atomic() in

That reminds me, need to upgrade this to smp_mb now, given atomic is gone.

> nohz_balance_enter_idle() which requires us to check the
> "nohz.idle_cpus_mask" after we are done clearing "nohz.needs_update"
> and "nohz.has_blocked_load".
>
> >          return -1;
> >   }