lkml.org 
[lkml]   [2021]   [Nov]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [BUG]locking/rwsem: only clean RWSEM_FLAG_HANDOFF when already set
From
On 11/11/21 15:50, Peter Zijlstra wrote:
> So I suspect that if..
>
> On Thu, Nov 11, 2021 at 02:36:52PM -0500, Waiman Long wrote:
>> static inline bool rwsem_try_write_lock(struct rw_semaphore *sem,
>> - enum writer_wait_state wstate)
>> + struct rwsem_waiter *waiter)
>> {
>> long count, new;
>> + bool first = rwsem_first_waiter(sem) == waiter;
>>
>> lockdep_assert_held(&sem->wait_lock);
>>
>> @@ -546,13 +541,14 @@ static inline bool rwsem_try_write_lock(struct rw_semaphore *sem,
>> do {
>> bool has_handoff = !!(count & RWSEM_FLAG_HANDOFF);
>>
>> - if (has_handoff && wstate == WRITER_NOT_FIRST)
>> + if (has_handoff && !first)
>> return false;
>>
>> new = count;
>>
>> if (count & RWSEM_LOCK_MASK) {
>> - if (has_handoff || (wstate != WRITER_HANDOFF))
>> + if (has_handoff || (!waiter->rt_task &&
>> + !time_after(jiffies, waiter->timeout)))
>> return false;
> we delete this whole condition, and..
I don't think we can take out this if test.
>
>>
>> new |= RWSEM_FLAG_HANDOFF;
>> @@ -889,6 +888,24 @@ rwsem_spin_on_owner(struct rw_semaphore *sem)
>> }
>> #endif
>>
>> +/*
>> + * Common code to handle rwsem flags in out_nolock path with wait_lock held.
>> + */
>> +static inline void rwsem_out_nolock_clear_flags(struct rw_semaphore *sem,
>> + struct rwsem_waiter *waiter)
>> +{
>> + long flags = 0;
>> +
>> + list_del(&waiter->list);
>> + if (list_empty(&sem->wait_list))
>> + flags = RWSEM_FLAG_HANDOFF | RWSEM_FLAG_WAITERS;
>> + else if (waiter->handoff_set)
>> + flags = RWSEM_FLAG_HANDOFF;
> take out this else,
>
>> +
>> + if (flags)
>> + atomic_long_andnot(flags, &sem->count);
>> +}
> We get the inherit thing for free, no?
>
> Once HANDOFF is set, new readers are blocked. And then allow any first
> waiter to acquire the lock, who cares if it was the one responsible for
> having set the HANDOFF bit.

Yes, we can have the policy of inheriting the HANDOFF bit as long as it
is consistent which will be the case here with a common out_nolock
function. I can go with that. I just have to document this fact in the
comment.

Cheers,
Longman

\
 
 \ /
  Last update: 2021-11-11 22:10    [W:0.110 / U:1.440 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site