Messages in this thread | | | Subject | Re: [PATCH 3/5] locking/rwsem: Enable reader optimistic lock stealing | From | Waiman Long <> | Date | Fri, 20 Nov 2020 12:26:58 -0500 |
| |
On 11/20/20 9:36 AM, Peter Zijlstra wrote: > On Tue, Nov 17, 2020 at 10:04:27PM -0500, Waiman Long wrote: >> diff --git a/kernel/locking/rwsem.c b/kernel/locking/rwsem.c >> index ee374ae061c3..930dd4af3639 100644 >> --- a/kernel/locking/rwsem.c >> +++ b/kernel/locking/rwsem.c >> @@ -957,6 +957,12 @@ static inline bool rwsem_reader_phase_trylock(struct rw_semaphore *sem, >> } >> return false; >> } >> + >> +static inline bool osq_is_empty(struct rw_semaphore *sem) >> +{ >> + return !osq_is_locked(&sem->osq); >> +} >> + >> #else >> static inline bool rwsem_can_spin_on_owner(struct rw_semaphore *sem, >> unsigned long nonspinnable) >> @@ -977,6 +983,10 @@ static inline bool rwsem_reader_phase_trylock(struct rw_semaphore *sem, >> return false; >> } >> >> +static inline bool osq_is_empty(sem) >> +{ >> + return false; >> +} > Hurph, the naming seems to suggest this ought to be in osq_lock.h, but > it really is part of rwsem, it captures the lack of osq member for this > configuration. > > How about: rwsem_no_spinners() instead ? Yes, sure. Will make the name change. > >> static inline int >> rwsem_spin_on_owner(struct rw_semaphore *sem, unsigned long nonspinnable) >> { >> @@ -1007,6 +1017,22 @@ rwsem_down_read_slowpath(struct rw_semaphore *sem, int state, long count) >> !(count & RWSEM_WRITER_LOCKED)) >> goto queue; >> >> + /* >> + * Reader optimistic lock stealing >> + * >> + * We can take the read lock directly without doing >> + * rwsem_optimistic_spin() if the conditions are right. >> + * Also wake up other readers if it is the first reader. >> + */ >> + if (!(count & (RWSEM_WRITER_LOCKED | RWSEM_FLAG_HANDOFF)) && >> + osq_is_empty(sem)) { >> + rwsem_set_reader_owned(sem); >> + lockevent_inc(rwsem_rlock_steal); >> + if (rcnt == 1) >> + goto wake_readers; >> + return sem; >> + } > AFAICT this saves at least 3 atomic ops; how common is this case > (you did add a counter but forgot to mention this). > Right, I should have mentioned the counter results.
Below is the relevant counter stats for a test system that have been up for more than 21 hours:
rwsem_opt_rlock=11792583 (optmistically acquired read lock) rwsem_rlock=193357272 (slowpath acquired read lock) rwsem_rlock_steal=44795149 (lock stealing)
So lock stealing represents about 17.9% of the total read lock acquired in non-fast path. I ran some microbenchmark test on the system before, so it may skew a bit to the high side. Anyway, this is not an insignificant amount.
Cheers, Longman
| |