[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

On 20.02.20 09:13, Jan Beulich wrote:
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)
-> get_cpu_maps() (lock unavailable to readers)

I've already pointed that out in another thread. :-)

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?

I think this can be handled locally in get_cpu_maps() and
cpu_hotplug_begin() with the use of a variable holding the cpu (or
NR_CPUS) of the cpu holding the write lock. get_cpu_maps() can just
succeed in case this variable contains smp_processor_id().


Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.