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

[Vincent Guittot]

On Tue, 4 Aug 2026 at 11:49, K Prateek Nayak <kprateek.nayak@xxxxxxx> 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

>
> >
> > 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
>