Re: [PATCH] sched/fair: Stabilize idle SMT core selection with asym-capacity

[Andrea Righi <arighi@nvidia.com>]

Hi Prateek,

On Fri, Jul 03, 2026 at 11:21:57AM +0530, K Prateek Nayak wrote:
> Hello Andrea,
> 
> On 6/30/2026 8:57 PM, Andrea Righi wrote:
> > select_idle_capacity() scans all logical CPUs also when it is looking
> > for a fully idle SMT core. Two concurrent wakeups can therefore observe
> > the same core as idle, encounter different siblings first, and place one
> > task on each sibling while another core remains unused.
> > 
> > Make every logical CPU of a selected idle core resolve to the same
> > stable CPU representative within the scan's existing affinity and
> > scheduling-domain mask. If the first task is enqueued before the next
> > scan examines the core, that scan rejects the now-busy core. If both
> > scans observe the core as idle, they select the same runqueue even if
> > the first enqueue becomes visible before the second scan finishes,
> > exposing the imbalance to the load balancer.
> > 
> > The symmetric CPU idle selection path is subject to the same race, but
> > normally returns as soon as select_idle_core() finds a fully idle core,
> > reducing the conflict window. The per-CPU capacity scan can retain an
> > idle-core candidate while evaluating other CPUs, giving concurrent
> > wakeups more opportunity to select different siblings of the same SMT
> > core. Therefore, limit the normalization to the asym-capacity path,
> > where this behavior has a measurable impact.
> > 
> > On NVIDIA Vera Rubin (arm64, 176 CPUs/88 cores per NUMA node), a
> > CPU-intensive NVPL SGEMM workload restricted to 88 threads (one per
> > core) showed a consistent 23% increase in mean throughput across
> > multiple runs.
> 
> Interesting! This reads like active balance across cores is not aggressive
> enough for this workload and, as a result, stacking somehow helps.
> 
> I would have expected balance within the core would trigger first and that
> would just lead to the same scenario as both sibling sibling busy but I
> guess there is a higher order effect of stacking.

I think the key here is that temporary runqueue stacking is preferable to
consuming both SMT siblings when fully-idle SMT cores are available, more than
having benfits from the stacking itself.

> 
> perf sched stats reports for this workload before and after
> applying your patch may help to see what changes for the load
> balancer to start doing better.

Ack, I'll collect some perf stats and share.

> 
> Could you check if something like this helps:
> 
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index fc6cd55f9d22..f50f12316dd3 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -13221,7 +13221,8 @@ imbalanced_active_balance(struct lb_env *env)
>  	 * threads on a system with spare capacity
>  	 */
>  	if ((env->migration_type == migrate_task) &&
> -	    (sd->nr_balance_failed > sd->cache_nice_tries+2))
> +	    ((sd->groups->flags & SD_SHARE_CPUCAPACITY) ||
> +	      sd->nr_balance_failed > sd->cache_nice_tries+2))

I did a quick test and I don't see any significant difference with this applied.
Let's see if the perf stats tell us more.

>  		return 1;
>  
>  	return 0;
> ---
> 
> I'm assuming we have group_has_spare for the destination CPU and the
> busy core appears as group_fully_busy or group_has_spare.
> calculate_imbalance() will take the sibling_imbalance() path since we
> are balancing amongst cores (SD_PREFER_SIBLING domain) and we get
> "migrate_task" with imbalance of 1.
> 
> Then we single down on a rq with a single task on it but that requires
> active balance and need_active_balance() is too slow as a result of
> imbalanced_active_balance() bailout on cache_nice_tries which requires
> at least 3 failures and on a 176 CPUs system, it can take upwards of
> 176 ticks per retry and with 250Hz tick, that time goes into seconds
> which might be too late.
> 
> I remember Julia had similar problem where balancing was taking too
> long and setting very aggressive "min_interval" and "max_interval" for
> load balancing helped her. Maybe you can try that too:
> 
>     # Needed to toggle /sys/kernel/debug/sched/domains/* visible
>     echo Y > /sys/kernel/debug/sched/verbose
>     for i in /sys/kernel/debug/sched/domains/cpu*/domain[1-5]/*_interval; do echo 10 > $i; done
>     echo N > /sys/kernel/debug/sched/verbose
> 
> This will ensure there is one balance every 10 ticks on domains above
> SMT. You can try make it more aggressive to see if that helps too.

Tried this as well (both with the patched and unpatched kernels), also no
measurable difference.

> 
> > 
> > For comparison, DCPerf MediaWiki running at system saturation (with all
> > SMT siblings busy) showed neither a benefit nor a regression: throughput
> > and Nginx request latency remained within measurement error.
> > 
> > Likewise, schbench under partially idle conditions showed no material
> > change in wakeup latency, request latency, or throughput (within 0.1%).
> > Tail wakeup latency was more consistent across runs with this change
> > applied.
> > 
> > Signed-off-by: Andrea Righi <arighi@nvidia.com>
> > ---
> >  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 d78467ec6ee13..f846fbe7379f4 100644
> > --- a/kernel/sched/fair.c
> > +++ b/kernel/sched/fair.c
> > @@ -8647,6 +8647,16 @@ enum asym_fits_state {
> >  	ASYM_IDLE_CORE_BIAS = -3,
> >  };
> >  
> > +/*
> > + * Return a stable CPU representative of @cpu's SMT core within @cpus.
> > + */
> > +static int select_idle_core_cpu(int cpu, const struct cpumask *cpus)
> > +{
> > +	int sibling = cpumask_first_and(cpu_smt_mask(cpu), cpus);
> > +
> > +	return sibling < nr_cpu_ids ? sibling : cpu;
> > +}
> > +
> >  /*
> >   * Scan the asym_capacity domain for idle CPUs; pick the first idle one on which
> >   * the task fits. If no CPU is big enough, but there are idle ones, try to
> > @@ -8661,6 +8671,7 @@ select_idle_capacity(struct task_struct *p, struct sched_domain *sd, int target)
> >  	 * collapses to the plain capacity scan.
> >  	 */
> >  	bool has_idle_core = sched_smt_active() && test_idle_cores(target);
> > +	bool best_idle_core = false;
> >  	unsigned long task_util, util_min, util_max, best_cap = 0;
> >  	int fits, best_fits = ASYM_IDLE_THREAD_MISFIT;
> >  	int cpu, best_cpu = -1;
> > @@ -8686,7 +8697,8 @@ select_idle_capacity(struct task_struct *p, struct sched_domain *sd, int target)
> >  	}
> >  
> >  	for_each_cpu_wrap(cpu, cpus, target) {
> > -		bool preferred_core = !has_idle_core || is_core_idle(cpu);
> > +		bool idle_core = !sched_smt_active() || is_core_idle(cpu);
> > +		bool preferred_core = !has_idle_core || idle_core;
> 
> Do you want to take overhead of is_core_idle() for !has_idle_core too?
> Wouldn't a simple:
> 
>     /* True iff has_idle_core was true and is_core_idle() returned true. */
>     bool idle_core = !has_idle_core ^ preferred_core;
> 
> after computing preferred_core do just fine?

Ah yes, or maybe something this, which looks a bit more readable:

  bool preferred_core = !has_idle_core || is_core_idle(cpu);
  bool idle_core = has_idle_core && preferred_core;

Thanks,
-Andrea