Re: [PATCH v6 01/11] x86/apic/x2apic: Fix parallel handling of cluster_mask

[Thomas Gleixner <tglx@linutronix.de>]

David!

On Tue, Feb 07 2023 at 10:57, David Woodhouse wrote:
> On Tue, 2023-02-07 at 00:20 +0100, Thomas Gleixner wrote:
>> +       /*
>> +        * 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?)

It happens on systems which support physical hotplug and AFAIK also
virtualization has support for "physical" hotplug.

The same is true the other way round on phsyical unplug. Then the CPU
is removed from present but is still set in possible.

>  • 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.

Correct.

> 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?

My worry was that virtualization might be able to phsyically hotplug
partial clusters. Whether that's possible I don't know, but in context
of virtualization I always assume the worst case.

Thanks,

        tglx