lkml.org 
[lkml]   [2023]   [Feb]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH 3/6] locking/rwsem: Rework writer wakeup
From
On 2/26/23 11:51, Peter Zijlstra wrote:
> On Sun, Feb 26, 2023 at 04:04:35PM +0100, Peter Zijlstra wrote:
>> On Thu, Feb 23, 2023 at 01:26:45PM +0100, Peter Zijlstra wrote:
>>> @@ -1072,7 +1067,7 @@ rwsem_down_read_slowpath(struct rw_semap
>>> for (;;) {
>>> set_current_state(state);
>>> if (!smp_load_acquire(&waiter.task)) {
>>> - /* Matches rwsem_mark_wake()'s smp_store_release(). */
>>> + /* Matches rwsem_waiter_wake()'s smp_store_release(). */
>>> break;
>>> }
>>> if (signal_pending_state(state, current)) {
>>> @@ -1143,54 +1138,36 @@ rwsem_down_write_slowpath(struct rw_sema
>>> } else {
>>> atomic_long_or(RWSEM_FLAG_WAITERS, &sem->count);
>> Found it; if we remove the try_write_lock below, then at least this
>> new-waiter path needs to still do a trylock.
>>
>> Let me go test the other patches on top of all this and push out a fresh
>> set if that all still works.
> queue.git locking/core
>
> We'll see what the robots make of it.

From your new patch 3:

@@ -1151,55 +1154,39 @@ rwsem_down_write_slowpath(struct rw_semaphore
*sem, int state)
                }
        } else {
                atomic_long_or(RWSEM_FLAG_WAITERS, &sem->count);
+               if (rwsem_try_write_lock(sem, &waiter))
+                       waiter.task = NULL;
        }
+       raw_spin_unlock_irq(&sem->wait_lock);

        /* wait until we successfully acquire the lock */
-       set_current_state(state);
        trace_contention_begin(sem, LCB_F_WRITE);

        for (;;) {
-               if (rwsem_try_write_lock(sem, &waiter)) {
-                       /* rwsem_try_write_lock() implies ACQUIRE on
success */
+               set_current_state(state);
+               if (!smp_load_acquire(&waiter.task)) {
+                       /* Matches rwsem_waiter_wake()'s
smp_store_release(). */
                        break;
                }
-

The additional rwsem_try_write_lock() call seems to address the missed
wakeup problem AFAICT.

I do have some concern that early lock transfer to a lock owner that has
not been woken up yet may suppress writer lock stealing from optimistic
spinning causing some performance regression in some cases. Let's see if
the test robot report anything.

Cheers,
Longman

\
 
 \ /
  Last update: 2023-03-27 00:36    [W:0.078 / U:0.040 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site