Re: [PATCH 4/5] locking/rwsem: Wake up all waiting readers if RWSEM_WAKE_READ_OWNED
On 11/17/20 11:53 PM, Davidlohr Bueso wrote:
On Tue, 17 Nov 2020, Waiman Long wrote:
The rwsem wakeup logic has been modified by commit d3681e269fff
("locking/rwsem: Wake up almost all readers in wait queue") to wake up
all readers in the wait queue if the first waiter is a reader. In the
case of RWSEM_WAKE_READ_OWNED, not all readers can be woken up if the
first waiter happens to be a writer. Complete the logic by waking up
all readers even for this case.
While rwsems are certainly not fifo, I'm concerned this would give too
much priority to the readers by having the reader owned lock just skip
over the first waiter. And I'd say most users are more concerned about
the writer side. Basically this would affect the phase-fair properties.
The idea of phase-fair is that when a reader acquires the lock, all the
current readers are allowed to join. Other readers that come after that
will not be allowed to join the read phase until the next round. In that
sense, waking up all readers in the wait queue doesn't violate this
fact. Patch 2 will guarantee the later constraint though it has the
exception that if the reader count reach 0, it will allow reader to
proceed. I am relying on the handoff mechanism to make sure that there
will be no lock starvation.
Cheers,
Longman
Re: [PATCH 4/5] locking/rwsem: Wake up all waiting readers if RWSEM_WAKE_READ_OWNED
On Tue, 17 Nov 2020, Waiman Long wrote:
The rwsem wakeup logic has been modified by commit d3681e269fff
("locking/rwsem: Wake up almost all readers in wait queue") to wake up
all readers in the wait queue if the first waiter is a reader. In the
case of RWSEM_WAKE_READ_OWNED, not all readers can be woken up if the
first waiter happens to be a writer. Complete the logic by waking up
all readers even for this case.
While rwsems are certainly not fifo, I'm concerned this would give too
much priority to the readers by having the reader owned lock just skip
over the first waiter. And I'd say most users are more concerned about
the writer side. Basically this would affect the phase-fair properties.
Thanks,
Davidlohr
[PATCH 4/5] locking/rwsem: Wake up all waiting readers if RWSEM_WAKE_READ_OWNED
The rwsem wakeup logic has been modified by commit d3681e269fff
("locking/rwsem: Wake up almost all readers in wait queue") to wake up
all readers in the wait queue if the first waiter is a reader. In the
case of RWSEM_WAKE_READ_OWNED, not all readers can be woken up if the
first waiter happens to be a writer. Complete the logic by waking up
all readers even for this case.
Signed-off-by: Waiman Long
---
kernel/locking/rwsem.c | 10 +++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/kernel/locking/rwsem.c b/kernel/locking/rwsem.c
index 930dd4af3639..23654e3950b5 100644
--- a/kernel/locking/rwsem.c
+++ b/kernel/locking/rwsem.c
@@ -426,7 +426,12 @@ static void rwsem_mark_wake(struct rw_semaphore *sem,
lockevent_inc(rwsem_wake_writer);
}
- return;
+ /*
+* If rwsem has already been owned by reader, wake up other
+* readers in the wait queue even if first one is a writer.
+*/
+ if (wake_type != RWSEM_WAKE_READ_OWNED)
+ return;
}
/*
@@ -1052,8 +1057,7 @@ rwsem_down_read_slowpath(struct rw_semaphore *sem, int
state, long count)
if (rwsem_optimistic_spin(sem, false)) {
/* rwsem_optimistic_spin() implies ACQUIRE on success */
/*
-* Wake up other readers in the wait list if the front
-* waiter is a reader.
+* Wake up other readers in the wait queue.
*/
wake_readers:
if ((atomic_long_read(>count) & RWSEM_FLAG_WAITERS)) {
--
2.18.1
