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

Re: [Xen-devel] [PATCH v2] rwlock: allow recursive read locking when already locked in write mode


On 20/02/2020 17:31, Roger Pau Monne wrote:
Allow a CPU already holding the lock in write mode to also lock it in
read mode. There's no harm in allowing read locking a rwlock that's
already owned by the caller (ie: CPU) in write mode. Allowing such
accesses is required at least for the CPU maps use-case.

In order to do this reserve 14bits of the lock, this allows to support
up to 16384 CPUs. Also reduce the write lock mask to 2 bits: one to
signal there are pending writers waiting on the lock and the other to
signal the lock is owned in write mode. Note the write related data
is using 16bits, this is done in order to be able to clear it (and
thus release the lock) using a 16bit atomic write.

This reduces the maximum number of concurrent readers from 16777216 to
65536, I think this should still be enough, or else the lock field
can be expanded from 32 to 64bits if all architectures support atomic
operations on 64bit integers.

FWIW, arm32 is able to support atomic operations on 64-bit integers.

  static inline void _write_unlock(rwlock_t *lock)
-    /*
-     * If the writer field is atomic, it can be cleared directly.
-     * Otherwise, an atomic subtraction will be used to clear it.
-     */
-    atomic_sub(_QW_LOCKED, &lock->cnts);
+    /* Since the writer field is atomic, it can be cleared directly. */
+    ASSERT(_is_write_locked_by_me(atomic_read(&lock->cnts)));
+    BUILD_BUG_ON(_QR_SHIFT != 16);
+    write_atomic((uint16_t *)&lock->cnts, 0);

I think this is an abuse to cast an atomic_t() directly into a uint16_t. You would at least want to use &lock->cnts.counter here.

The rest of the code looks good to me.


Julien Grall

Xen-devel mailing list



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