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

[Shrikanth Hegde <sshegde@linux.ibm.com>]

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.

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

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