Re: [PATCH 3/5] locking/rwsem: add down_read[_killable]_nosteal()

[Lorenzo Stoakes (ARM)]

On Tue, Nov 03, 2026 at 12:51:59PM +0100, Peter Zijlstra wrote:
> On Tue, Nov 03, 2026 at 08:27:44AM +0000, Lorenzo Stoakes (ARM) wrote:
> > Since commit 1a728dff855a ("locking/rwsem: Enable reader optimistic lock
> > stealing") a sleeping, queued writer whose handoff timeout hasn't expired
> > can have its lock stolen by a read lock issued before a woken queued writer
> > was able to acquire a lock.
> >
> > To avoid write lock starvation, this is only permitted if no other read
> > locks are held.
> >
> > However, this behaviour defeats the purpose of a reader periodically
> > dropping the lock to allow writers in, as doing so only works if the write
> > lock is acquired first, another read lock is held, or a queued writer's
> > handoff timeout has expired.
> >
> > Provide a means of indicating this intent via down_read_nosteal() and
> > down_read_killable_nosteal().
> >
> > Implement it by exposing a steal parameter to rwsem_down_read_slowpath(),
> > update the starvation logic to modify this parameter and modify the steal
> > branch to check it first.
> >
> > Also drop the duplicated RWSEM_WRITER_LOCKED check and remove the
> > goto as the steal condition makes this redundant.
> >
> > Exposing this function is safe as it only makes the lock more pessimistic,
> > so does not impact lock contention.
>
> Not generally a fan of this -- can't we tweak the steal thing to never
> steal if there are pending writers?

Oh, well, I had assumed I couldn't fiddle with that behaviour across
rwsem's in general, so if I can yeah that's preferable!

I could send that change separately then and drop it out of this series.

Something like this?

diff --git a/kernel/locking/rwsem.c b/kernel/locking/rwsem.c
index b9c180ac1eee..d6572e207523 100644
--- a/kernel/locking/rwsem.c
+++ b/kernel/locking/rwsem.c
@@ -1034,22 +1034,29 @@ rwsem_down_read_slowpath(struct rw_semaphore *sem, long count, unsigned int stat
 	 * Reader optimistic lock stealing.
 	 */
 	if (!(count & (RWSEM_WRITER_LOCKED | RWSEM_FLAG_HANDOFF))) {
-		rwsem_set_reader_owned(sem);
-		lockevent_inc(rwsem_rlock_steal);
+		bool steal = true;

-		/*
-		 * Wake up other readers in the wait queue if it is
-		 * the first reader.
-		 */
-		if ((rcnt == 1) && (count & RWSEM_FLAG_WAITERS)) {
+		if (count & RWSEM_FLAG_WAITERS) {
 			raw_spin_lock_irq(&sem->wait_lock);
-			if (sem->first_waiter)
+			first = sem->first_waiter;
+			if (first && first->type == RWSEM_WAITING_FOR_WRITE) {
+				/* Writers always take precedence. */
+				steal = false;
+			} else if (first && rcnt == 1) {
+				/* First reader, wake other readers in queue. */
 				rwsem_mark_wake(sem, RWSEM_WAKE_READ_OWNED,
 						&wake_q);
+			}
 			raw_spin_unlock_irq(&sem->wait_lock);
-			wake_up_q(&wake_q);
 		}
-		return sem;
+
+		if (steal) {
+			rwsem_set_reader_owned(sem);
+			lockevent_inc(rwsem_rlock_steal);
+			if (!wake_q_empty(&wake_q))
+				wake_up_q(&wake_q);
+			return sem;
+		}
 	}

 queue:
--
Cheers, Lorenzo