Re: [PATCH 2/2] sched/fair: Honor asymmetric SMT priority in idle selection

[Andrea Righi <arighi@nvidia.com>]

Hi Dietmar,

On Thu, Sep 03, 2026 at 12:59:59PM +0200, Dietmar Eggemann wrote:
> On 31.08.26 20:10, Andrea Righi wrote:
> > SD_ASYM_PACKING orders CPUs that share an SMT core, but idle CPU
> > selection does not consult that order. A task can therefore wake on an
> > arbitrary sibling and remain there until load balancing corrects the
> > placement. On SMT implementations where changing the active sibling
> > repartitions core resources, that initial choice can cause a large and
> > persistent performance loss.
> 
> I assume this sentence refers to Olympus/Vera and Power7?

Yes. Olympus/Vera is the platform motivating this series, but this is affecting
POWER7 as well, since it's also using SD_ASYM_PACKING at the SMT level. I'll
change the description to make that scope explicit.

> 
> [...]
> 
> > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> > index 8dff37059faf7..3c49aa63742cb 100644
> > --- a/kernel/sched/fair.c
> > +++ b/kernel/sched/fair.c
> > @@ -8587,6 +8587,65 @@ static inline bool test_idle_cores(int cpu)
> >  	return false;
> >  }
> >  
> > +/*
> > + * Return true when @cpu has a higher asymmetric-packing priority than @other in their SMT
> > + * scheduling domain.
> > + */
> > +static bool sched_smt_asym_prefer(int cpu, int other)
> > +{
> > +	struct sched_domain *sd;
> > +
> > +	for_each_domain(cpu, sd) {
> > +		/*
> > +		 * Only honor priorities declared at shared-capacity SMT levels.
> > +		 * SD_ASYM_PACKING at higher levels may describe core ordering.
> > +		 */
> > +		if (!(sd->flags & SD_SHARE_CPUCAPACITY))
> > +			break;
> > +
> > +		if ((sd->flags & SD_ASYM_PACKING) && cpumask_test_cpu(other, sched_domain_span(sd)))
> 
> Looks like 'other' is always part of the mask?

It's always in the same hardware SMT sibling mask, but it's not necessarily in
the same scheduling-domain span, isolcpus could split siblings across scheduling
domain, select_idle_smt() has the same sched_domain_span() check for the same
resason IIUC.

> 
> > +			return sched_asym_prefer(cpu, other);
> > +	}
> > +
> > +	return false;
> > +}
> 
> SMT will always the lowest SD, so for_each_domain() is not necessary:

Agreed, walking the domain hierarchy is unnecessary. I'll use the lowest domain
directly.

> 
> static bool sched_smt_asym_prefer(int cpu, int other)
> {
>         struct sched_domain *sd = rcu_dereference_all(cpu_rq(cpu)->sd);
> 
>         if (sd && ((sd->flags & (SD_SHARE_CPUCAPACITY |
> SD_ASYM_PACKING)) == (SD_SHARE_CPUCAPACITY | SD_ASYM_PACKING)))
>                 return sched_asym_prefer(cpu, other);
> 
>         return false;
> }
> 
> [...]
> 
> > @@ -8668,7 +8727,7 @@ static int select_idle_smt(struct task_struct *p, struct sched_domain *sd, int t
> >  		if (!cpumask_test_cpu(cpu, sched_domain_span(sd)))
> >  			continue;
> >  		if (choose_idle_cpu(cpu, p))
> > -			return cpu;
> > +			return select_idle_smt_priority(p, cpu);
> 
> This one is weird for SMT2. AFAICS, select_idle_smt() is called when
> there are no idle cores. So if you find an idle CPU this is what you
> will return anyway.

Correcdt for SMT2: once the core is partially idle, there's only one idle
sibling and the priority lookup returns that same CPU.

The hook is useful for wider asym SMT though. POWER7 uses SMT4 with
SD_ASYM_PACKING at the SMT level, so a partially idle core can still have
multiple idle threads. In that case select_idle_smt() would otherwise return the
first idle thread rather than the highest-priority one.

> 
> I guess your tests on Olympus/Vera do wakeups via select_idle_capacity()
> so you haven't touched this one.

Correct, Vera also has SD_ASYM_CPUCAPACITY, so the scan path used by these tests
is select_idle_capacity().

Thanks for taking a look at this!
-Andrea