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
