Re: [PATCH v5 12/13] xen/rwlock: raise the number of possible cpus

[Jürgen Groß]

On 18.03.24 17:05, Jan Beulich wrote:

On 18.03.2024 17:00, Jürgen Groß wrote:

On 18.03.24 16:39, Jan Beulich wrote:

On 14.03.2024 08:20, Juergen Gross wrote:

@@ -36,14 +36,16 @@ void queue_write_lock_slowpath(rwlock_t *lock);

static inline bool _is_write_locked_by_me(unsigned int cnts)

   {
-    BUILD_BUG_ON(_QW_CPUMASK < NR_CPUS);
+    BUILD_BUG_ON((_QW_CPUMASK + 1) < NR_CPUS);
+    BUILD_BUG_ON(NR_CPUS * _QR_BIAS > INT_MAX);
       return (cnts & _QW_WMASK) == _QW_LOCKED &&
              (cnts & _QW_CPUMASK) == smp_processor_id();
   }

static inline bool _can_read_lock(unsigned int cnts)

   {
-    return !(cnts & _QW_WMASK) || _is_write_locked_by_me(cnts);
+    return cnts <= INT_MAX &&
+           (!(cnts & _QW_WMASK) || _is_write_locked_by_me(cnts));
   }

I view this as problematic: Code knowing that a write lock is being held
may invoke a function using read_trylock() and expect the lock to be
available there.

So you expect it to be fine that someone is using read_trylock() 32768 times
recursively while holding a lock as a writer? Sure, I can change the condition,
but OTOH ...

Hmm, yes, the reader count (leaving aside nested read_trylock()) is zero
when the lock is held for writing. So yes, I agree the condition is fine,
but may I ask for a brief comment to this effect, for blind people like
me?

Yeah, fine with me. :-)


Juergen