Messages in this thread | | | Date | Thu, 11 Nov 2021 16:09:35 -0500 | Subject | Re: [BUG]locking/rwsem: only clean RWSEM_FLAG_HANDOFF when already set | From | Waiman Long <> |
| |
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
| |