Re: [PATCH 4/4] sched/fair: Prefer fully-idle SMT core for NOHZ idle load balancer

[Andrea Righi]

Hi Vincent,

On Fri, Mar 27, 2026 at 09:45:56AM +0100, Vincent Guittot wrote:
> On Thu, 26 Mar 2026 at 16:12, Andrea Righi <arighi@xxxxxxxxxx> wrote:
> >
> > When choosing which idle housekeeping CPU runs the idle load balancer,
> > prefer one on a fully idle core if SMT is active, so balance can migrate
> > work onto a CPU that still offers full effective capacity. Fall back to
> > any idle candidate if none qualify.
> 
> This one isn't straightforward for me. The ilb cpu will check all
> other idle CPUs 1st and finish with itself so unless the next CPU in
> the idle_cpus_mask is a sibling, this should not make a difference
> 
> Did you see any perf diff ?

I actually see a benefit, in particular, with the first patch applied I see
a ~1.76x speedup, if I add this on top I get ~1.9x speedup vs baseline,
which seems pretty consistent across runs (definitely not in error range).

The intention with this change was to minimize SMT noise running the ILB
code on a fully-idle core when possible, but I also didn't expect to see
such big difference.

I'll investigate more to better understand what's happening.

> 
> 
> >
> > Cc: Vincent Guittot <vincent.guittot@xxxxxxxxxx>
> > Cc: Dietmar Eggemann <dietmar.eggemann@xxxxxxx>
> > Cc: Christian Loehle <christian.loehle@xxxxxxx>
> > Cc: Koba Ko <kobak@xxxxxxxxxx>
> > Reported-by: Felix Abecassis <fabecassis@xxxxxxxxxx>
> > Signed-off-by: Andrea Righi <arighi@xxxxxxxxxx>
> > ---
> >  kernel/sched/fair.c | 19 +++++++++++++++++--
> >  1 file changed, 17 insertions(+), 2 deletions(-)
> >
> > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> > index 593a89f688679..a1ee21f7b32f6 100644
> > --- a/kernel/sched/fair.c
> > +++ b/kernel/sched/fair.c
> > @@ -12733,11 +12733,15 @@ static inline int on_null_domain(struct rq *rq)
> >   * - When one of the busy CPUs notices that there may be an idle rebalancing
> >   *   needed, they will kick the idle load balancer, which then does idle
> >   *   load balancing for all the idle CPUs.
> > + *
> > + * - When SMT is active, prefer a CPU on a fully idle core as the ILB
> > + *   target, so that when it runs balance it becomes the destination CPU
> > + *   and can accept migrated tasks with full effective capacity.
> >   */
> >  static inline int find_new_ilb(void)
> >  {
> >         const struct cpumask *hk_mask;
> > -       int ilb_cpu;
> > +       int ilb_cpu, fallback = -1;
> >
> >         hk_mask = housekeeping_cpumask(HK_TYPE_KERNEL_NOISE);
> >
> > @@ -12746,11 +12750,22 @@ static inline int find_new_ilb(void)
> >                 if (ilb_cpu == smp_processor_id())
> >                         continue;
> >
> > +#ifdef CONFIG_SCHED_SMT
> 
> you can probably get rid of the CONFIG and put this special case below
> sched_smt_active()

Ah good point, will change this.

> 
> 
> > +               if (!idle_cpu(ilb_cpu))
> > +                       continue;
> > +
> > +               if (fallback < 0)
> > +                       fallback = ilb_cpu;
> > +
> > +               if (!sched_smt_active() || is_core_idle(ilb_cpu))
> > +                       return ilb_cpu;
> > +#else
> >                 if (idle_cpu(ilb_cpu))
> >                         return ilb_cpu;
> > +#endif
> >         }
> >
> > -       return -1;
> > +       return fallback;
> >  }
> >
> >  /*
> > --
> > 2.53.0
> >

Thanks,
-Andrea