Re: [PATCH 2/2] sched/topology: Add asymmetric SMT packing override

[Andrea Righi <arighi@nvidia.com>]

Hi Vincent,

On Fri, Sep 18, 2026 at 06:03:53PM +0200, Vincent Guittot wrote:
...
> > > diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> > > index 33cd30996e47e..36c3b2e563441 100644
> > > --- a/Documentation/admin-guide/kernel-parameters.txt
> > > +++ b/Documentation/admin-guide/kernel-parameters.txt
> > > @@ -6799,6 +6799,17 @@ Kernel parameters
> > >                       solution to mutex-based priority inversion.
> > >                       Format: <bool>
> > >
> > > +     sched_smt_asym_packing= [KNL,SMP]
> > > +                     Override asymmetric packing at the SMT scheduling domain.
> > > +                     Format: { auto | on | off }
> > > +                     auto: Preserve the architecture default. This is the
> > > +                     default when the option is omitted.
> 
> AFAICT "auto" equals nothing added in the command line so why is it needed ?

Right, auto is redundant. Omitting the parameter already preserves the topology
provided by the architecture or firmware.

> 
> > > +                     on: Force asymmetric packing at the SMT scheduling domain.
> > > +                     Idle CPU selection prefers siblings with a higher
> > > +                     architecture-defined priority. Siblings with equal
> > > +                     priorities remain unordered.
> > > +                     off: Ignore SMT sibling priorities.
> 
> Is this for debugging purposes?

Yes, that was the intent, but I don't have a concrete use case that justifies
exposing it.

> 
> only the sched_smt_asym_packing=on is really useful to force
> asym_packing when firmware doesn't provide the info

Agreed. I'll simplify the interface to an explicit force option for systems
whose firmware can't describe the SMT preference.

Thanks,
-Andrea

> 
> > > +
> >
> > Wasn't this option/parameter to come from arch specific file?
> >
> > Isn't it going to be confusing for archs which don't benefit from asym packing at SMT,
> > but now there is kernel parameter to say on.
> >
> > >       sched_verbose   [KNL,EARLY] Enables verbose scheduler debug messages.
> > >
> > >       schedstats=     [KNL,X86] Enable or disable scheduled statistics.
> > > diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
> > > index 0248227d983a7..cdfcecf673fd7 100644
> > > --- a/kernel/sched/topology.c
> > > +++ b/kernel/sched/topology.c
> > > @@ -32,6 +32,46 @@ static int __init sched_debug_setup(char *str)
> > >   }
> > >   early_param("sched_verbose", sched_debug_setup);
> > >
> > > +#ifdef CONFIG_SCHED_SMT
> > > +enum sched_smt_asym_packing_mode {
> > > +     SCHED_SMT_ASYM_PACKING_AUTO,
> > > +     SCHED_SMT_ASYM_PACKING_ON,
> > > +     SCHED_SMT_ASYM_PACKING_OFF,
> > > +     SCHED_SMT_ASYM_PACKING_NR,
> > > +};
> > > +
> > > +static enum sched_smt_asym_packing_mode sched_smt_asym_packing __read_mostly =
> > > +     SCHED_SMT_ASYM_PACKING_AUTO;
> > > +
> > > +static const char * const sched_smt_asym_packing_modes[SCHED_SMT_ASYM_PACKING_NR] = {
> > > +     [SCHED_SMT_ASYM_PACKING_AUTO]   = "auto",
> > > +     [SCHED_SMT_ASYM_PACKING_ON]     = "on",
> > > +     [SCHED_SMT_ASYM_PACKING_OFF]    = "off",
> > > +};
> > > +
> > > +static int __init sched_smt_asym_packing_parse(const char *str)
> > > +{
> > > +     for (int mode = 0; mode < SCHED_SMT_ASYM_PACKING_NR; mode++) {
> > > +             if (!strcmp(str, sched_smt_asym_packing_modes[mode]))
> > > +                     return mode;
> > > +     }
> > > +
> > > +     return -EINVAL;
> > > +}
> > > +
> > > +static int __init setup_sched_smt_asym_packing(char *str)
> > > +{
> > > +     int mode = sched_smt_asym_packing_parse(str);
> > > +
> > > +     if (mode < 0)
> > > +             return 0;
> > > +
> > > +     sched_smt_asym_packing = mode;
> > > +     return 1;
> > > +}
> > > +__setup("sched_smt_asym_packing=", setup_sched_smt_asym_packing);
> > > +#endif
> > > +
> > >   static inline bool sched_debug(void)
> > >   {
> > >       return sched_debug_verbose;
> > > @@ -1950,6 +1990,15 @@ sd_init(struct sched_domain_topology_level *tl,
> > >       if (WARN_ONCE(sd_flags & ~TOPOLOGY_SD_FLAGS,
> > >                     "wrong sd_flags in topology description\n"))
> > >               sd_flags &= TOPOLOGY_SD_FLAGS;
> > > +#ifdef CONFIG_SCHED_SMT
> > > +     if (sd_flags & SD_SHARE_CPUCAPACITY) {
> > > +             if (sched_smt_asym_packing == SCHED_SMT_ASYM_PACKING_ON)
> > > +                     sd_flags |= SD_ASYM_PACKING;
> > > +             else if (sched_smt_asym_packing ==
> > > +                      SCHED_SMT_ASYM_PACKING_OFF)
> > > +                     sd_flags &= ~SD_ASYM_PACKING;
> > > +     }
> > > +#endif
> > >       sd_flags |= asym_cpu_capacity_classify(sd_span, cpu_map);
> > >
> > >       *sd = (struct sched_domain){
> >