Re: [PATCH v1] cpu: Add missing check to cpuhp_smt_enable()

Rafael J. Wysocki
2025-09-05T13:27:49Z

On Fri, Sep 5, 2025 at 3:13 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
>
> On Fri, Sep 5, 2025 at 9:39 AM Thomas Gleixner <tglx@linutronix.de> wrote:
> >
> > On Fri, Aug 29 2025 at 22:01, Rafael J. Wysocki wrote:
> > > --- a/kernel/cpu.c
> > > +++ b/kernel/cpu.c
> > > @@ -2710,6 +2710,12 @@
> > > cpu_maps_update_begin();
> > > cpu_smt_control = CPU_SMT_ENABLED;
> > > for_each_present_cpu(cpu) {
> > > + /*
> > > + * Avoid accidentally onlining primary thread CPUs that have
> > > + * been taken offline.
> > > + */
> > > + if (topology_is_primary_thread(cpu))
> > > + continue;
> >
> > Hmm. That's kinda solving the problem, but I think the proper solution
> > would be to implement topology_is_core_online() for x86.
> >
> > The above is preventing the primary thread to be onlined, but then
> > onlines the corresponding hyperthread, which does not really make sense.
>
> Well, manual online can be used for onlining the secondary thread of a
> core where the primary thread is offline, so this is technically
> possible already.
>
> > Something like the completely untested below.
>
> So given the above, shouldn't topology_is_core_online() check if any
> thread in the given core is online?

Besides, this would cause the siblings of offline SMT threads to be
skipped while enabling SMT via sysfs (using
/sys/devices/system/cpu/smt/control), but I'm not sure if this is the
expectation in the field today. The current behavior is to online all
secondary SMT threads (and more, but that part is quite arguably
broken).

That's why I decided to use a simpler patch which doesn't go that far.