Re: [RFC PATCH 1/2] cpu/hotplug: Skip disabled CPUs in cpuhp_smt_enable

[Salil Mehta]

[Sincere Apologies, sending it again as Plain Text; earlier reply got
filtered perhaps due to HTML content]

Hi Bradley,

Many thanks for taking a look.


On Tue, Sep 29, 2026 at 9:16 PM Bradley Morgan <brads@xxxxxxxxxxxxxx> wrote:
>
> On 29 September 2026 20:41:29 BST, salil.mehta@xxxxxxxxxx wrote:
> >From: Salil Mehta <salil.mehta@xxxxxxxxxx>
> >
> >The CPU enabled mask describes whether a present CPU may currently be
> >brought online. cpuhp_smt_enable() is an online operation, but currently
> >walks all present CPUs and only filters CPUs that are already online or
> >belong to offline NUMA nodes.
> >
> >This can make it attempt _cpu_up() for a present CPU which firmware has
> >not enabled and which has not yet been registered as a CPU device.
> >
> >Use the enabled mask for the policy decision it was introduced to
> >represent. This keeps present-but-disabled CPUs out of the SMT bring-up
> >path while still allowing registered offline SMT threads to be brought
> >back online.
> >
> >Present and enabled are separate generic CPU states, so callers which
> >intend to bring CPUs online should not assume that every present CPU is
> >enabled.
>
> Ok.
>
> >
> >Signed-off-by: Salil Mehta <salil.mehta@xxxxxxxxxx>
> >---
> > kernel/cpu.c | 5 +++--
> > 1 file changed, 3 insertions(+), 2 deletions(-)
> >
> >diff --git a/kernel/cpu.c b/kernel/cpu.c
> >index b3c8553d7bd6..988b7a1e8298 100644
> >--- a/kernel/cpu.c
> >+++ b/kernel/cpu.c
> >@@ -2706,8 +2706,9 @@ int cpuhp_smt_enable(void)
> >       cpu_maps_update_begin();
> >       cpu_smt_control = CPU_SMT_ENABLED;
> >       for_each_present_cpu(cpu) {
> >-              /* Skip online CPUs and CPUs on offline nodes */
> >-              if (cpu_online(cpu) || !node_online(cpu_to_node(cpu)))
> >+              /* Skip online/disabled CPUs and CPUs on offline nodes */
> >+              if (cpu_online(cpu) || !cpu_enabled(cpu) ||
> >+                  !node_online(cpu_to_node(cpu)))
>
> Ehh, have you had a issue with this code? Like something e.g: a splat.

No, I am not reporting a new splat or crash on current upstream. This RFC
is a proactive polite enquiry about the design choice, together with a
proposed alternative. If you check the the cover letter it provides the
context, including reproduction of the original warning with the earlier
present-mask semantics restored and the results with the proposed fix.

Just for the context, there is also an outstanding QEMU Arm vCPU hotplug
series awaiting upstream acceptance. The distinction between a CPU being
present and being enabled has been an important part of the model we have
been explaining to the QEMU community. Changing those semantics now could
complicate that work by changing the assumptions on which the interface
and its explanation have been based.

The underlying CPU architectural requirement remains that all resources
associated with the possible vCPUs are described at boot; virtual CPU
hotplug does not dynamically add or remove those resources. The patch in
contention is not changing that assumption even now but are we changing
the contract between the ACPI and the kernel or misrepresenting what has
been discovered already?

My understanding from the earlier design discussions related to support
of the vCPU Hotplug on ARM was that toggling the present mask to represent
firmware enablement was considered and ultimately rejected. The intention
was to keep the kernel’s representation consistent with the ACPI/firmware
model: a CPU can remain present while firmware controls whether it is
enabled. The separate cpu_enabled_mask was introduced to represent that
distinction.

My concern is therefore about the compatibility implications of changing
this established, userspace-visible meaning. I cannot currently point to
a specific upper-layer consumer that breaks, but neither is it
straightforward to establish that no consumers depend on it.

Catalin also initially raised the possibility of breaking other things by
no longer marking these CPUs present in the discussion of Jinjie’s
original patch, although he subsequently proposed a present-mask change
himself:

https://lore.kernel.org/lkml/aeNxKpHzTQX4_kId@xxxxxxx/

What I would like to understand is why changing the present-mask semantics
is preferable to using the existing enabled mask in cpuhp_smt_enable().
To me, checking eligibility at this caller appears to address the original
warning more directly while preserving the present/enabled distinction.

However, I may be missing a deeper constraint or a trade-off discussed
while I was away from this work. That is why I posted this as an RFC, and
I would appreciate understanding the reasoning.

Hope this explanation helps.

Many thanks,
Salil.