Re: [PATCH v6 7/8] xen/rwlock: raise the number of possible cpus

[Jan Beulich <jbeulich@xxxxxxxx>]

On 27.03.2024 16:22, Juergen Gross wrote:
> @@ -36,14 +36,21 @@ 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);
> +    /*
> +     * If write locked by the caller, no other readers are possible.
> +     * Not allowing the lock holder to read_lock() another 32768 times ought
> +     * to be fine.
> +     */
> +    return cnts <= INT_MAX &&
> +           (!(cnts & _QW_WMASK) || _is_write_locked_by_me(cnts));
>  }

What is the 32768 in the comment relating to? INT_MAX is quite a bit higher,
yet the comparison against it is the only thing you add. Whereas the reader
count is, with the sign bit unused, 17 bits, though (bits 14..30). I think
even in such a comment rather than using a literal number the corresponding
expression would better be stated.

Jan