Re: [PATCH] locking/rwsem: don't let a read lock steal from a queued writer

[Waiman Long <longman@redhat.com>]

On 11/5/26 3:24 PM, Lorenzo Stoakes (ARM) wrote:
> Since commit 1a728dff855a ("locking/rwsem: Enable reader optimistic lock
> stealing"), a racing read lock can steal an rwsem from under a queued
> writer whose handoff timeout hasn't expired yet.
>
> This is most problematic when an rwsem read lock is released then
> reacquired in order to relieve lock contention, as instead it will simply
> have the lock handed right back to it ahead of the intended beneficiary.
>
> Naturally, this behaviour rather defeats the object of doing this kind of
> thing, so this is less than ideal.

If there is only one reader doing this release and then acquire 
operation, it will leave its task_struct value in the owner field. So we 
can check if the owner field  to determine if we should skip the lock 
stealing operation.

Something like

diff --git a/kernel/locking/rwsem.c b/kernel/locking/rwsem.c
index b9c180ac1eee..1f0fffd75c18 100644
--- a/kernel/locking/rwsem.c
+++ b/kernel/locking/rwsem.c
@@ -1019,15 +1019,17 @@ rwsem_down_read_slowpath(struct rw_semaphore 
*sem, long count, unsigned int stat
         long adjustment = -RWSEM_READER_BIAS;
         long rcnt = (count >> RWSEM_READER_SHIFT);
         struct rwsem_waiter waiter, *first;
+       long owner = atomic_long_read(&sem->owner);
         DEFINE_WAKE_Q(wake_q);

         /*
          * To prevent a constant stream of readers from starving a sleeping
          * writer, don't attempt optimistic lock stealing if the lock is
-        * very likely owned by readers.
+        * very likely owned by readers or when the current task is the
+        * previous owning reader.
          */
-       if ((atomic_long_read(&sem->owner) & RWSEM_READER_OWNED) &&
-           (rcnt > 1) && !(count & RWSEM_WRITER_LOCKED))
+       if ((owner & RWSEM_READER_OWNED) && ((rcnt > 1) ||
+           (current == (struct task_struct *)(owner & 
~RWSEM_OWNER_FLAGS_MASK))))
                 goto queue;

         /*

------------------------------------------------------------------------------------------------------------------------

Will this help?

Cheers,
Longman