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

[Andrea Righi]

Hi Srikar,

On Tue, Sep 08, 2026 at 11:07:20AM +0530, Srikar Dronamraju wrote:
> * Andrea Righi <arighi@xxxxxxxxxx> [2026-09-04 11:18:05]:
> 
> Hi Andrea,
> 
> > POWER7 and NVIDIA Olympus use SD_ASYM_PACKING at the shared-capacity
> > SMT level to order hardware threads. Idle CPU selection does not consult
> > that order, so a task can wake on an arbitrary sibling and remain there
> > until load balancing corrects the placement. On these systems, that
> > initial choice can prevent the core from entering its preferred
> > lower-thread resource mode and cause a large and persistent performance
> > loss.
> > 
> 
> > When idle selection finds an available CPU in an SMT core, choose the
> > highest-priority available sibling. On SMT2 Olympus this only changes
> > selection on fully idle cores. A partially idle core has only one
> > available CPU. On wider SMT systems such as POWER7, it also fills
> > available siblings in priority order while the core is partially busy.
> > 
> 
> Don't we need changes in the slow path too?
> Something like this?
> https://lore.kernel.org/all/20251204175405.1511340-2-srikar@xxxxxxxxxxxxx/T/#u

Ah yes, good catch! I'll include the WF_FORK / WF_EXEC slow-path as well.

> 
> > Apply the preference to idle-core and idle-CPU scans,
> > asymmetric-capacity scans, and the target, previous, and recently-used
> > CPU fast paths. Inspect the lowest scheduling domain directly, but
> > require both CPUs to share its span because isolcpus can split hardware
> > siblings across scheduling domains.
> > 
> > Keep physical-core capacity selection independent from SMT sibling
> > ordering. SD_ASYM_CPUCAPACITY first selects among cores with different
> > maximum capacities, then SD_ASYM_PACKING selects the preferred available
> > sibling inside the chosen core, whose siblings continue to share equal
> > capacity.
> > 
> > Signed-off-by: Andrea Righi <arighi@xxxxxxxxxx>
> > ---
> >  kernel/sched/fair.c     | 79 ++++++++++++++++++++++++++++++++++++-----
> >  kernel/sched/sched.h    |  6 ++++
> >  kernel/sched/topology.c | 36 +++++++++++++++++++
> >  3 files changed, 112 insertions(+), 9 deletions(-)
> > 
> > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> > index b8bd308c2d5b1..ff9a7b1fcbe7f 100644
> > --- a/kernel/sched/fair.c
> > +++ b/kernel/sched/fair.c
> > @@ -8587,6 +8587,63 @@ static inline bool test_idle_cores(int cpu)
> >  	return false;
> >  }
> >  
> > +/*
> > + * Return true when @cpu has a higher asymmetric-packing priority than
> > + * @other in their shared SMT scheduling domain.
> > + */
> > +static bool sched_smt_asym_prefer(int cpu, int other)
> > +{
> > +	struct sched_domain *sd = rcu_dereference_all(cpu_rq(cpu)->sd);
> > +
> > +	if (!sd)
> > +		return false;
> > +
> > +	if (!(sd->flags & SD_SHARE_CPUCAPACITY) ||
> > +	    !(sd->flags & SD_ASYM_PACKING))
> > +		return false;
> > +
> > +	if (!cpumask_test_cpu(other, sched_domain_span(sd)))
> > +		return false;
> > +
> > +	return sched_asym_prefer(cpu, other);
> > +}
> > +
> > +/*
> > + * Return the highest-priority available CPU in @cpu's SMT core that is also in @cpus.
> > + */
> > +static int __select_idle_smt_cpu(struct task_struct *p, int cpu, const struct cpumask *cpus)
> > +{
> > +	int best = cpu;
> > +	int sibling;
> > +
> > +	for_each_cpu_and(sibling, cpu_smt_mask(cpu), cpus) {
> > +		if (sibling == best || !choose_idle_cpu(sibling, p))
> > +			continue;
> > +
> > +		if (sched_smt_asym_prefer(sibling, best))
> > +			best = sibling;
> > +	}
> > +
> > +	return best;
> > +}
> > +
> > +static inline int
> > +select_idle_smt_cpu(struct task_struct *p, int cpu, const struct cpumask *cpus)
> > +{
> > +	if (!sched_smt_asym_active())
> > +		return cpu;
> > +
> > +	return __select_idle_smt_cpu(p, cpu, cpus);
> 
> Nit: I see __select_idle_smt_cpu called only here.
> Cant we fold __select_idle_smt_cpu() here itself.

Agreed, will fold in v4.

> 
> > +}
> > +
> > +/*
> > + * Redirect an available SMT CPU to a higher-priority available sibling allowed by task affinity.
> > + */
> > +static inline int select_idle_smt_priority(struct task_struct *p, int cpu)
> > +{
> > +	return select_idle_smt_cpu(p, cpu, p->cpus_ptr);
> > +}
> > +
> 
> nit: Can we also replace select_idle_smt_priority with select_idle_smt_cpu()
> itself.

Ack, we can use select_idle_smt_cpu() directly.

> 
> Otherwise looks good to me
> 
> Reviewed-by: Srikar Dronamraju <srikar@xxxxxxxxxxxxx>

Thanks for taking a look at this!
-Andrea

> 
> -- 
> Thanks and Regards
> Srikar Dronamraju