Skip to content

Commit 80bad6f

Browse files
PlaidCatPlaidCat
committed
locking/rwsem: Prevent non-first waiter from spinning in down_write() slowpath
jira LE-1907 Rebuild_History Non-Buildable kernel-rt-5.14.0-284.30.1.rt14.315.el9_2 commit-author Waiman Long <longman@redhat.com> commit b613c7f Empty-Commit: Cherry-Pick Conflicts during history rebuild. Will be included in final tarball splat. Ref for failed cherry-pick at: ciq/ciq_backports/kernel-rt-5.14.0-284.30.1.rt14.315.el9_2/b613c7f3.failed 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 RT 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 A non-first waiter cannot really acquire the rwsem even if it mistakenly believes that it can spin on OWNER_NULL value. If that waiter happens to be an RT task running on the same CPU as the first waiter, it can block the first waiter from acquiring the rwsem leading to live lock. Fix this problem by making sure that a non-first waiter cannot spin in the slowpath loop without sleeping. Fixes: d257cc8 ("locking/rwsem: Make handoff bit handling more consistent") Signed-off-by: Waiman Long <longman@redhat.com> Signed-off-by: Ingo Molnar <mingo@kernel.org> Tested-by: Mukesh Ojha <quic_mojha@quicinc.com> Reviewed-by: Mukesh Ojha <quic_mojha@quicinc.com> Cc: stable@vger.kernel.org Link: https://lore.kernel.org/r/20230126003628.365092-2-longman@redhat.com (cherry picked from commit b613c7f) Signed-off-by: Jonathan Maple <jmaple@ciq.com> # Conflicts: # kernel/locking/rwsem.c
1 parent a02e215 commit 80bad6f

File tree

1 file changed

+127
-0
lines changed

1 file changed

+127
-0
lines changed

‎ciq/ciq_backports/kernel-rt-5.14.0-284.30.1.rt14.315.el9_2/b613c7f3.failed‎

Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,127 @@
1+
locking/rwsem: Prevent non-first waiter from spinning in down_write() slowpath
2+
3+
jira LE-1907
4+
Rebuild_History Non-Buildable kernel-rt-5.14.0-284.30.1.rt14.315.el9_2
5+
commit-author Waiman Long <longman@redhat.com>
6+
commit b613c7f31476c44316bfac1af7cac714b7d6bef9
7+
Empty-Commit: Cherry-Pick Conflicts during history rebuild.
8+
Will be included in final tarball splat. Ref for failed cherry-pick at:
9+
ciq/ciq_backports/kernel-rt-5.14.0-284.30.1.rt14.315.el9_2/b613c7f3.failed
10+
11+
A non-first waiter can potentially spin in the for loop of
12+
rwsem_down_write_slowpath() without sleeping but fail to acquire the
13+
lock even if the rwsem is free if the following sequence happens:
14+
15+
Non-first RT waiter First waiter Lock holder
16+
------------------- ------------ -----------
17+
Acquire wait_lock
18+
rwsem_try_write_lock():
19+
Set handoff bit if RT or
20+
wait too long
21+
Set waiter->handoff_set
22+
Release wait_lock
23+
Acquire wait_lock
24+
Inherit waiter->handoff_set
25+
Release wait_lock
26+
Clear owner
27+
Release lock
28+
if (waiter.handoff_set) {
29+
rwsem_spin_on_owner(();
30+
if (OWNER_NULL)
31+
goto trylock_again;
32+
}
33+
trylock_again:
34+
Acquire wait_lock
35+
rwsem_try_write_lock():
36+
if (first->handoff_set && (waiter != first))
37+
return false;
38+
Release wait_lock
39+
40+
A non-first waiter cannot really acquire the rwsem even if it mistakenly
41+
believes that it can spin on OWNER_NULL value. If that waiter happens
42+
to be an RT task running on the same CPU as the first waiter, it can
43+
block the first waiter from acquiring the rwsem leading to live lock.
44+
Fix this problem by making sure that a non-first waiter cannot spin in
45+
the slowpath loop without sleeping.
46+
47+
Fixes: d257cc8cb8d5 ("locking/rwsem: Make handoff bit handling more consistent")
48+
Signed-off-by: Waiman Long <longman@redhat.com>
49+
Signed-off-by: Ingo Molnar <mingo@kernel.org>
50+
Tested-by: Mukesh Ojha <quic_mojha@quicinc.com>
51+
Reviewed-by: Mukesh Ojha <quic_mojha@quicinc.com>
52+
Cc: stable@vger.kernel.org
53+
Link: https://lore.kernel.org/r/20230126003628.365092-2-longman@redhat.com
54+
(cherry picked from commit b613c7f31476c44316bfac1af7cac714b7d6bef9)
55+
Signed-off-by: Jonathan Maple <jmaple@ciq.com>
56+
57+
# Conflicts:
58+
# kernel/locking/rwsem.c
59+
diff --cc kernel/locking/rwsem.c
60+
index 186ad9eda88d,be2df9ea7c30..000000000000
61+
--- a/kernel/locking/rwsem.c
62+
+++ b/kernel/locking/rwsem.c
63+
@@@ -554,13 -616,26 +554,35 @@@ static inline bool rwsem_try_write_lock
64+
do {
65+
bool has_handoff = !!(count & RWSEM_FLAG_HANDOFF);
66+
67+
++<<<<<<< HEAD
68+
+ if (has_handoff && wstate == WRITER_NOT_FIRST)
69+
+ return false;
70+
++=======
71+
+ if (has_handoff) {
72+
+ /*
73+
+ * Honor handoff bit and yield only when the first
74+
+ * waiter is the one that set it. Otherwisee, we
75+
+ * still try to acquire the rwsem.
76+
+ */
77+
+ if (first->handoff_set && (waiter != first))
78+
+ return false;
79+
+ }
80+
++>>>>>>> b613c7f31476 (locking/rwsem: Prevent non-first waiter from spinning in down_write() slowpath)
81+
82+
new = count;
83+
84+
if (count & RWSEM_LOCK_MASK) {
85+
++<<<<<<< HEAD
86+
+ if (has_handoff || (wstate != WRITER_HANDOFF))
87+
++=======
88+
+ /*
89+
+ * A waiter (first or not) can set the handoff bit
90+
+ * if it is an RT task or wait in the wait queue
91+
+ * for too long.
92+
+ */
93+
+ if (has_handoff || (!rt_task(waiter->task) &&
94+
+ !time_after(jiffies, waiter->timeout)))
95+
++>>>>>>> b613c7f31476 (locking/rwsem: Prevent non-first waiter from spinning in down_write() slowpath)
96+
return false;
97+
98+
new |= RWSEM_FLAG_HANDOFF;
99+
@@@ -574,12 -649,21 +596,19 @@@
100+
} while (!atomic_long_try_cmpxchg_acquire(&sem->count, &count, new));
101+
102+
/*
103+
- * We have either acquired the lock with handoff bit cleared or
104+
- * set the handoff bit.
105+
+ * We have either acquired the lock with handoff bit cleared or set
106+
+ * the handoff bit. Only the first waiter can have its handoff_set
107+
+ * set here to enable optimistic spinning in slowpath loop.
108+
*/
109+
++<<<<<<< HEAD
110+
+ if (new & RWSEM_FLAG_HANDOFF)
111+
++=======
112+
+ if (new & RWSEM_FLAG_HANDOFF) {
113+
+ first->handoff_set = true;
114+
+ lockevent_inc(rwsem_wlock_handoff);
115+
++>>>>>>> b613c7f31476 (locking/rwsem: Prevent non-first waiter from spinning in down_write() slowpath)
116+
return false;
117+
- }
118+
119+
- /*
120+
- * Have rwsem_try_write_lock() fully imply rwsem_del_waiter() on
121+
- * success.
122+
- */
123+
- list_del(&waiter->list);
124+
rwsem_set_owner(sem);
125+
return true;
126+
}
127+
* Unmerged path kernel/locking/rwsem.c

0 commit comments

Comments
 (0)