On Tue, 2023-02-07 at 00:20 +0100, Thomas Gleixner wrote:
>
>
> TBH. The logic of this code is anything but obvious. Something like the
> uncompiled below perhaps?
Looks sane to me. I'll tweak the comments a bit and give it a spin;
thanks.
...
> + * At boot time CPU present mask is stable. If the cluster is not
> + * yet initialized, allocate the mask and propagate it to all
> + * siblings in this cluster.
> */
> - if (cluster_hotplug_mask) {
> - if (cluster_hotplug_mask->node == node)
> - return 0;
> - kfree(cluster_hotplug_mask);
> - }
> + if (system_state < SYSTEM_RUNNING)
> + goto alloc;
> +
> + /*
> + * On post boot hotplug iterate over the present CPUs to handle the
> + * case of partial clusters as they might be presented by
> + * virtualization.
> + */
> + for_each_present_cpu(cpu_i) {
So... if this CPU was *present* at boot time (and if any other CPU in
this cluster was present), it will already have a cluster_mask.
Which means we get here in two cases:
• This CPU wasn't actually present (was just 'possible') at boot time.
(Is that actually a thing that happens?)
• This CPU was present but no other CPU in this cluster was actually
brought up at boot time so the cluster_mask wasn't allocated.
The code looks right, I don't grok the comment about partial clusters
and virtualization, and would have worded it something along the above
lines?
Attachment:
smime.p7s
Description: S/MIME cryptographic signature
