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

[Valentin Schneider]

On 11/05/26 20:07, Shrikanth Hegde wrote:
> On 5/11/26 6:23 PM, Valentin Schneider wrote:
>> 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.
>>
>
> Ok. Fair enough. Even cpu bringup path too could use the same opt.
> Something like below?

LGTM

> ---
>
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 084ec3987d7c..add0fcc8ba90 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -6494,6 +6494,10 @@ static void sched_core_cpu_starting(unsigned int cpu)
>          struct rq *rq = cpu_rq(cpu), *core_rq = NULL;
>          int t;
>
> +       /* No point in doing anything further if SMT is not active */
> +       if (!sched_smt_active())
> +               return;
> +
>          guard(core_lock)(&cpu);
>
>          WARN_ON_ONCE(rq->core != rq);
> @@ -6533,6 +6537,10 @@ static void sched_core_cpu_deactivate(unsigned int cpu)
>          struct rq *rq = cpu_rq(cpu), *core_rq = NULL;
>          int t;
>
> +       /* No point in doing anything further if SMT is not active */
> +       if (!sched_smt_active())
> +               return;
> +
>          guard(core_lock)(&cpu);
>
>          /* if we're the last man standing, nothing to do */
>
>
>>
>>> @@ -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.
>>
>
> Right. intel_ifs is the only user.
> INTEL_IFS depends on SMP. SMP select CONFIG_SMT on x86.
>
> Symbol: INTEL_IFS [=n]
> Depends on: X86_PLATFORM_DEVICES [=y] && X86 [=y] && CPU_SUP_INTEL [=y] && 64BIT [=y] && SMP [=y]
>

Ah, I had looked at that config but didn't expand on SMP implying SMT.

> So, maybe leave this as is, to avoid the code bloat for CONFIG_SCHED_SMT=n as there is no user?
> Maybe i will add a comment about it.
>
> from ./bloat-o-meter. it adds about 260
> stop_core_cpuslocked                           -     260    +260
>
> Does that make sense?

Yup!