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
