On 12.06.2024 10:09, Roger Pau Monné wrote:
> On Tue, Jun 11, 2024 at 09:42:39AM +0200, Jan Beulich wrote:
>> On 10.06.2024 16:20, Roger Pau Monne wrote:
>>> Due to the current rwlock logic, if the CPU calling get_cpu_maps() does so
>>> from
>>> a cpu_hotplug_{begin,done}() region the function will still return success,
>>> because a CPU taking the rwlock in read mode after having taken it in write
>>> mode is allowed. Such behavior however defeats the purpose of
>>> get_cpu_maps(),
>>
>> I'm not happy to see you still have this claim here. The behavior may (appear
>> to) defeat the purpose here, but as expressed previously I don't view that as
>> being a general pattern.
>
> Right. What about replacing the paragraph with:
>
> "Due to the current rwlock logic, if the CPU calling get_cpu_maps() does so
> from
> a cpu_hotplug_{begin,done}() region the function will still return success,
> because a CPU taking the rwlock in read mode after having taken it in write
> mode is allowed. Such corner case makes using get_cpu_maps() alone
> not enough to prevent using the shorthand in CPU hotplug regions."
Thanks.
> I think the rest is of the commit message is not controversial.
Indeed.
>>> --- a/xen/arch/x86/smp.c
>>> +++ b/xen/arch/x86/smp.c
>>> @@ -88,7 +88,7 @@ void send_IPI_mask(const cpumask_t *mask, int vector)
>>> * the system have been accounted for.
>>> */
>>> if ( system_state > SYS_STATE_smp_boot &&
>>> - !unaccounted_cpus && !disabled_cpus &&
>>> + !unaccounted_cpus && !disabled_cpus && !cpu_in_hotplug_context()
>>> &&
>>> /* NB: get_cpu_maps lock requires enabled interrupts. */
>>> local_irq_is_enabled() && (cpus_locked = get_cpu_maps()) &&
>>> (park_offline_cpus ||
>>
>> Along with changing the condition you also want to update the comment to
>> reflect the code adjustment.
>
> I've assumed the wording in the commet that says: "no CPU hotplug or
> unplug operations are taking place" would already cover the addition
> of the !cpu_in_hotplug_context() check.
Hmm, yes, you're right. Just needs a release-ack then to go in (with the
description adjustment).
Jan
