Re: [PATCH v3] sched/fair: Prefer fully idle cores for NOHZ balancing

Hi Vincent and Prateek,

On Tue, Aug 04, 2026 at 12:30:22PM +0200, Vincent Guittot wrote:
> On Tue, 4 Aug 2026 at 11:49, K Prateek Nayak <[email protected]> wrote:
> >
> > Hello Vincent,
> >
> > On 8/4/2026 2:12 PM, Vincent Guittot wrote:
> > >> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> > >> index 37001c63452e5..574b6b3ee922a 100644
> > >> --- a/kernel/sched/fair.c
> > >> +++ b/kernel/sched/fair.c
> > >> @@ -13965,28 +13965,66 @@ static inline int on_null_domain(struct rq *rq)
> > >>  static inline int find_new_ilb(void)
> > >>  {
> > >>         int this_cpu = smp_processor_id();
> > >> -       const struct cpumask *hk_mask;
> > >> -       int ilb_cpu;
> > >> +       struct cpumask *ilb_cpus;
> > >> +       int ilb_cpu, fallback = -1;
> > >> +
> > >> +       lockdep_assert_irqs_disabled();
> > >>
> > >> -       hk_mask = housekeeping_cpumask(HK_TYPE_KERNEL_NOISE);
> > >> +       /*
> > >> +        * Reuse the per-CPU select_rq_mask, which is protected from concurrent
> > >> +        * use on this CPU by having interrupts disabled.
> > >> +        */
> > >> +       ilb_cpus = this_cpu_cpumask_var_ptr(select_rq_mask);
> > >> +       cpumask_and(ilb_cpus, nohz.idle_cpus_mask,
> > >> +                   housekeeping_cpumask(HK_TYPE_KERNEL_NOISE));
> > >>
> > >> -       for_each_cpu_and(ilb_cpu, nohz.idle_cpus_mask, hk_mask) {
> > >> +       for_each_cpu(ilb_cpu, ilb_cpus) {
> > >>                 if (ilb_cpu == this_cpu)
> > >
> > > this_cpu is not idle so you can apply the same as below
> >
> > Dumb question: nohz_balancer_kick() already does a
> > nohz_balance_exit_idle(rq) before trying to find the ilb_cpu via
> > find_new_ilb() so is it even possible for this_cpu to be set on the
> > nohz.idle_cpus_mask here?
> 
> Yes that's a good point, the test is useless
> 
> >
> > Even on weakly ordered systems, reads and writes to the same location
> > (nohz.idle_cpus_mask) cannot get reordered right?
> 
> Yes

Right, checking this_cpu is redundant, I'll remove it and send a v4.

Thanks!
-Andrea

> 
> >
> > >
> > > Other than that looks good to me
> > >
> > >>                         continue;
> > >>
> > >> -               if (idle_cpu(ilb_cpu))
> > >> -                       return ilb_cpu;
> > >> +               if (!idle_cpu(ilb_cpu)) {
> > >
> > > +               if (ilb_cpu == this_cpu || !idle_cpu(ilb_cpu)) {
> >
> > Otherwise, this makes sense.
> >
> > --
> > Thanks and Regards,
> > Prateek
> >