From: Waiman Long <longman@redhat.com>
To: Peter Zijlstra <peterz@infradead.org>,
Ingo Molnar <mingo@redhat.com>, Will Deacon <will@kernel.org>,
Boqun Feng <boqun.feng@gmail.com>
Cc: linux-kernel@vger.kernel.org, john.p.donnelly@oracle.com,
"Hillf Danton" <hdanton@sina.com>,
"Mukesh Ojha" <quic_mojha@quicinc.com>,
"Ting11 Wang 王婷" <wangting11@xiaomi.com>,
"Waiman Long" <longman@redhat.com>
Subject: [PATCH v3 1/5] locking/rwsem: Prevent non-first waiter from spinning in down_write() slowpath
Date: Mon, 17 Oct 2022 17:13:52 -0400 [thread overview]
Message-ID: <20221017211356.333862-2-longman@redhat.com> (raw)
In-Reply-To: <20221017211356.333862-1-longman@redhat.com>
A non-first waiter can potentially spin in the for loop of
rwsem_down_write_slowpath() without sleeping but fail to acquire the
lock even if the rwsem is free if the following sequence happens:
Non-first waiter First waiter Lock holder
---------------- ------------ -----------
Acquire wait_lock
rwsem_try_write_lock():
Set handoff bit if RT or
wait too long
Set waiter->handoff_set
Release wait_lock
Acquire wait_lock
Inherit waiter->handoff_set
Release wait_lock
Clear owner
Release lock
if (waiter.handoff_set) {
rwsem_spin_on_owner(();
if (OWNER_NULL)
goto trylock_again;
}
trylock_again:
Acquire wait_lock
rwsem_try_write_lock():
if (first->handoff_set && (waiter != first))
return false;
Release wait_lock
It is especially problematic if the non-first waiter is an RT task and
it is running on the same CPU as the first waiter as this can lead to
live lock.
Fixes: d257cc8cb8d5 ("locking/rwsem: Make handoff bit handling more consistent")
Reviewed-and-tested-by: Mukesh Ojha <quic_mojha@quicinc.com>
Signed-off-by: Waiman Long <longman@redhat.com>
---
kernel/locking/rwsem.c | 19 +++++++++----------
1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/kernel/locking/rwsem.c b/kernel/locking/rwsem.c
index 44873594de03..be2df9ea7c30 100644
--- a/kernel/locking/rwsem.c
+++ b/kernel/locking/rwsem.c
@@ -624,18 +624,16 @@ static inline bool rwsem_try_write_lock(struct rw_semaphore *sem,
*/
if (first->handoff_set && (waiter != first))
return false;
-
- /*
- * First waiter can inherit a previously set handoff
- * bit and spin on rwsem if lock acquisition fails.
- */
- if (waiter == first)
- waiter->handoff_set = true;
}
new = count;
if (count & RWSEM_LOCK_MASK) {
+ /*
+ * A waiter (first or not) can set the handoff bit
+ * if it is an RT task or wait in the wait queue
+ * for too long.
+ */
if (has_handoff || (!rt_task(waiter->task) &&
!time_after(jiffies, waiter->timeout)))
return false;
@@ -651,11 +649,12 @@ static inline bool rwsem_try_write_lock(struct rw_semaphore *sem,
} while (!atomic_long_try_cmpxchg_acquire(&sem->count, &count, new));
/*
- * We have either acquired the lock with handoff bit cleared or
- * set the handoff bit.
+ * We have either acquired the lock with handoff bit cleared or set
+ * the handoff bit. Only the first waiter can have its handoff_set
+ * set here to enable optimistic spinning in slowpath loop.
*/
if (new & RWSEM_FLAG_HANDOFF) {
- waiter->handoff_set = true;
+ first->handoff_set = true;
lockevent_inc(rwsem_wlock_handoff);
return false;
}
--
2.31.1
next prev parent reply other threads:[~2022-10-17 21:15 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-10-17 21:13 [PATCH v3 0/5] lockinig/rwsem: Fix rwsem bugs & enable true lock handoff Waiman Long
2022-10-17 21:13 ` Waiman Long [this message]
2022-10-24 13:17 ` [PATCH v3 1/5] locking/rwsem: Prevent non-first waiter from spinning in down_write() slowpath Peter Zijlstra
2022-10-24 13:50 ` Waiman Long
2022-10-17 21:13 ` [PATCH v3 2/5] locking/rwsem: Limit # of null owner retries for handoff writer Waiman Long
2022-10-24 13:31 ` Peter Zijlstra
2022-10-24 15:55 ` Waiman Long
2022-10-25 11:22 ` Peter Zijlstra
2022-10-25 11:48 ` Peter Zijlstra
2022-10-25 19:55 ` Waiman Long
2022-10-25 20:14 ` Peter Zijlstra
2022-10-26 1:44 ` Waiman Long
[not found] ` <20221025145843.2953-1-hdanton@sina.com>
2022-10-25 19:00 ` Waiman Long
2022-10-17 21:13 ` [PATCH v3 3/5] locking/rwsem: Change waiter->hanodff_set to a handoff_state enum Waiman Long
2022-10-17 21:13 ` [PATCH v3 4/5] locking/rwsem: Enable direct rwsem lock handoff Waiman Long
2022-10-17 21:13 ` [PATCH v3 5/5] locking/rwsem: Update handoff lock events tracking Waiman Long
[not found] ` <20221018111424.1007-1-hdanton@sina.com>
2022-10-18 14:13 ` [PATCH v3 4/5] locking/rwsem: Enable direct rwsem lock handoff Mukesh Ojha
2022-10-18 17:37 ` Waiman Long
[not found] ` <20221018235138.1088-1-hdanton@sina.com>
2022-10-19 0:39 ` Waiman Long
[not found] ` <20221019022934.1166-1-hdanton@sina.com>
2022-10-19 2:49 ` Waiman Long
[not found] ` <20221019070559.1220-1-hdanton@sina.com>
2022-10-19 15:02 ` Waiman Long
2022-10-24 16:18 ` Waiman Long
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20221017211356.333862-2-longman@redhat.com \
--to=longman@redhat.com \
--cc=boqun.feng@gmail.com \
--cc=hdanton@sina.com \
--cc=john.p.donnelly@oracle.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=peterz@infradead.org \
--cc=quic_mojha@quicinc.com \
--cc=wangting11@xiaomi.com \
--cc=will@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.