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

[Andrea Righi <arighi@nvidia.com>]

Hi Shrikanth,

On Fri, Sep 18, 2026 at 07:44:25PM +0530, Shrikanth Hegde wrote:
> Hi Andrea,
> 
> On 9/17/26 7:35 PM, Andrea Righi wrote:
> > Architectures can use SD_ASYM_PACKING to describe preferred CPU ordering
> > at the SMT scheduling domain. Some systems benefit from the same policy,
> > but their firmware cannot currently describe the preference. Inferring
> > it from the CPU model would embed a platform-specific policy in the
> > kernel.
> > 
> > Add the sched_smt_asym_packing boot option to override SD_ASYM_PACKING
> > at the SMT level. Accept auto, on and off. Auto preserves the
> > architecture-provided topology and is also the default when the option
> > is absent. On and off force the flag without changing asymmetric packing
> > at higher topology levels.
> > 
> > Apply the override centrally to domains with SD_SHARE_CPUCAPACITY so it
> > also covers architectures with custom SMT topology callbacks, including
> > powerpc. When forced on, priority remains defined by
> > arch_asym_cpu_priority(). The weak default prefers lower-numbered
> > logical CPUs, while architecture overrides remain authoritative.
> > Siblings with equal priorities remain unordered.
> > 
> 
> Wasn't this a temporary solution you wanted until the firmware changes are
> sorted out? or something changed in between?
> 
> If yes, could you please capture that in changelog or in comment.

The firmware-described ACPI property remains the preferred long-term solution.
However, defining and validating the interface across firmware, Linux, and other
operating systems, as well as completing the ACPI standardization process, will
take some time.

The command-line option is intended as an interim workaround and as a fallback
for deployed systems whose firmware cannot be updated. It allows users to enable
the optimization independently of their firmware version. I agree that this
should be explained more clearly in the changelog.

> 
> > Signed-off-by: Andrea Righi <arighi@nvidia.com>
> > ---
> >   .../admin-guide/kernel-parameters.txt         | 11 +++++
> >   kernel/sched/topology.c                       | 49 +++++++++++++++++++
> >   2 files changed, 60 insertions(+)
> > 
> > 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.
> > +			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.
> > +
> 
> Wasn't this option/parameter to come from arch specific file?

Will didn't like very much the idea of detecting this through an Olympus CPU
quirk and requested a firmware-described property instead. Therefore, we moved
to a boot-time override that avoids any Arm- or CPU-specific detection for now.

> 
> 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.

That's a fair concern. Following Vincent's suggestion, I can drop the auto and
off values and retain only an explicit force option. Without the option, the
existing architecture-/firmware-provided topology is left unchanged.

Thanks,
-Andrea

> 
> >   	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){
>