Re: [Xen-devel] [PATCH 1/2] smp: convert the cpu maps lock into a rw lock

[Jan Beulich <jbeulich@xxxxxxxx>]

On 13.02.2020 12:32, Roger Pau Monne wrote:
> Most users of the cpu maps just care about the maps not changing while
> the lock is being held, but don't actually modify the maps.
> 
> Convert the lock into a rw lock, and take the lock in read mode in
> get_cpu_maps and in write mode in cpu_hotplug_begin. This will lower
> the contention around the lock, since plug and unplug operations that
> take the lock in write mode are not that common.
> 
> Note that the read lock can be taken recursively (as it's a shared
> lock), and hence will keep the same behavior as the previously used
> recursive lock. As for the write lock, it's only used by CPU
> plug/unplug operations, and the lock is never taken recursively in
> that case.
> 
> While there also change get_cpu_maps return type to bool.
> 
> Reported-by: Julien Grall <julien@xxxxxxx>
> Suggested-also-by: Jan Beulich <jbeulich@xxxxxxxx>
> Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

I'm afraid I can't see how offlining a CPU would now work.
Condensed to just the relevant calls, the sequence from
cpu_down() is

cpu_hotplug_begin() (i.e. lock taken in write mode)
stop_machine_run()
-> get_cpu_maps() (lock unavailable to readers)

Other than recursive spin locks, rw locks don't currently
have a concept of permitting in a reader when this CPU
already holds the lock in write mode. Hence I can't see
how the get_cpu_maps() above would now ever succeed. Am I
missing anything, or does the patch need reverting until
the read_trylock() got enhanced to cope with this?

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel