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

[Andrea Righi <arighi@nvidia.com>]

Hi Vincent,

On Wed, Sep 09, 2026 at 05:42:43PM +0200, Vincent Guittot wrote:
> On Wed, 9 Sept 2026 at 17:18, Andrea Righi <arighi@nvidia.com> wrote:
> >
> > Hi Vincent,
> >
> > On Wed, Sep 09, 2026 at 04:42:44PM +0200, Vincent Guittot wrote:
> > > On Tue, 8 Sept 2026 at 10:24, Andrea Righi <arighi@nvidia.com> wrote:
> > > >
> > > > 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.
> > > >
> > > > Apply the preference to idle-core and idle-CPU scans,
> > > > asymmetric-capacity scans, target, previous, recently-used CPU fast
> > > > paths and the slow path. 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.
> > > >
> > > > Reviewed-by: Srikar Dronamraju <srikar@linux.ibm.com>
> > > > Signed-off-by: Andrea Righi <arighi@nvidia.com>
> > > > ---
> > > >  kernel/sched/fair.c     | 85 ++++++++++++++++++++++++++++++++---------
> > > >  kernel/sched/sched.h    |  6 +++
> > > >  kernel/sched/topology.c | 36 +++++++++++++++++
> > > >  3 files changed, 110 insertions(+), 17 deletions(-)
> > > >
> > > > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> > > > index b8bd308c2d5b1..37837c36288a0 100644
> > > > --- a/kernel/sched/fair.c
> > > > +++ b/kernel/sched/fair.c
> > > > @@ -8587,6 +8587,35 @@ static inline bool test_idle_cores(int cpu)
> > > >         return false;
> > > >  }
> > > >
> > > > +/*
> > > > + * Redirect a CPU to a higher-priority available sibling in its SMT domain,
> > > > + * subject to task affinity.
> > > > + */
> > > > +static inline int select_idle_smt_cpu(struct task_struct *p, int cpu)
> > > > +{
> > > > +       struct sched_domain *sd;
> > > > +       int best = cpu;
> > > > +       int sibling;
> > > > +
> > > > +       if (!sched_smt_asym_active())
> > >
> > > I wonder if it's worth creating a new static key. All other pieces
> > > related to asym packing use sched_smt_active() to opt out the related
> > > code
> >
> > The intent was to keep the additional sd dereference and flag checks out of the
> > wakeup path for the more common symmetric SMT systems; sched_smt_active()
> > remains enabled on those systems, the new key lets them return immediately.
> >
> > Without it, the additional cost should be small when everything is cache-hot
> > (roughly a couple of dependent loads, flag tests and branches), but this is a
> > hot path and a cache miss could make it more noticeable. I haven't measured
> > whether the saving is significant, though. If the extra key and its topology
> > accounting are not considered worth the potential saving, we can remove it and
> > use sched_smt_active() instead.
> 
> If we start having a static key per sub part of a feature like the
> asym packing, that can quickly become unmanageable. In this case we
> should better have a a static key for whole asym_packing feature
> instead

Makes sense, I'll remove the static key and use sched_smt_active().

Thanks,
-Andrea