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?
Jan
