On Tue, Feb 07 2023 at 11:27, David Woodhouse wrote:
> On Tue, 2023-02-07 at 10:57 +0000, David Woodhouse wrote:
>> • 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?
>
> As I get my head around that, I think the code needs to change too.
> What if we *unplug* the only CPU in a cluster (present→possible), then
> add a new one in the same cluster? The new one would get a new
> cluster_mask. Which is kind of OK for now but then if we re-add the
> original CPU it'd continue to use its old cluster_mask.
Indeed.
> Now, that's kind of weird if it's physical CPUs because that cluster is
> within a given chip, isn't it? But with virtualization maybe that's
> something that could happen, and it doesn't hurt to be completely safe
> by using for_each_possible_cpu() instead?
Yes. Virtualization does aweful things....
> Now looks like this:
> /*
> * On post boot hotplug for a CPU which was not present at boot time,
> * iterate over all possible CPUs (even those which are not present
> * any more) to find any existing cluster mask.
> */
> for_each_possible_cpu(cpu_i) {
Looks good!
tglx
