Re: [PATCH 2/3] sched: Simplify ifdeffery around cpu_smt_mask

[Valentin Schneider]

On 06/05/26 16:30, Shrikanth Hegde wrote:
> Now, that cpu_smt_mask is defined as cpumask_of(cpu) for
> CONFIG_SCHED_SMT=n, it is possible to get rid of the ifdeffery.
>
> Effectively,
> - This makes sched_smt_present is defined always
>
> - cpumask_weight(cpumask_of(cpu)) == 1. So sched_smt_present_inc/dec
>   will never enable the sched_smt_present. Which is expected.
>
> - Paths that were compile-time eliminated become runtime guarded
>   using static keys.
>
> - Defines set_idle_cores, test_idle_cores etc which could likely benefit
>   the CONFIG_SCHED_SMT=n systems to use the same optimizations within the
>   LLC at wakeups.
>
> - This will expose sched_smt_present,stop_core_cpuslocked symbol for
>   CONFIG_SCHED_SMT=n. Likely not a concern.
>
> - There a bloat of code CONFIG_SCHED_SMT=n. (NR_CPUS=2048)
>   add/remove: 25/18 grow/shrink: 26/19 up/down: 6696/-3064 (3632)
>   Total: Before=30771823, After=30775455, chg +0.01%
>
> - No code bloat for CONFIG_SCHED_SMT=y, which is expected.
>

Some nitpicks below, otherwise this LGTM except the sched_ext bits which
I'm not familiar enough with.

> @@ -8703,9 +8699,7 @@ int sched_cpu_deactivate(unsigned int cpu)
>        */
>       sched_smt_present_dec(cpu);
>
> -#ifdef CONFIG_SCHED_SMT
>       sched_core_cpu_deactivate(cpu);
> -#endif

That ends up grabbing @core_lock, arguably this is during hotplug but still
seems a bit wasteful when, with CONFIG_SCHED_SMT=1, we know the mask weight
will never exceed 1. Probably worth adding a sched_smt_active() check
within the callee.


> @@ -632,7 +632,6 @@ int stop_machine(cpu_stop_fn_t fn, void *data, const struct cpumask *cpus)
>  }
>  EXPORT_SYMBOL_GPL(stop_machine);
>
> -#ifdef CONFIG_SCHED_SMT
>  int stop_core_cpuslocked(unsigned int cpu, cpu_stop_fn_t fn, void *data)

That seems to be only used by the INTEL_IFS selftest stuff which does some
wait_for_sibling_cpu() loop; at a quick glance it seems to do the right
thing for weight := 1 but IMO worth a proper look. That or have the IFS
code not run that when there is no SMT.

>  {
>       const struct cpumask *smt_mask = cpu_smt_mask(cpu);