|
Jiri Wiesner |
f08034 |
From 3f5245538a1964ae186ab7e1636020a41aa63143 Mon Sep 17 00:00:00 2001
|
|
Jiri Wiesner |
83a61e |
From: Waiman Long <longman@redhat.com>
|
|
Jiri Wiesner |
f08034 |
Date: Wed, 25 Jan 2023 19:36:26 -0500
|
|
Jiri Wiesner |
f08034 |
Subject: [PATCH] locking/rwsem: Disable preemption in all down_read*() and
|
|
Jiri Wiesner |
83a61e |
up_read() code paths
|
|
Jiri Wiesner |
f08034 |
Git-commit: 3f5245538a1964ae186ab7e1636020a41aa63143
|
|
Jiri Wiesner |
f08034 |
Patch-mainline: v6.3-rc1
|
|
Jiri Wiesner |
f08034 |
References: bsc#1204996
|
|
Jiri Wiesner |
83a61e |
|
|
Jiri Wiesner |
f08034 |
Commit:
|
|
Jiri Wiesner |
f08034 |
|
|
Jiri Wiesner |
f08034 |
91d2a812dfb9 ("locking/rwsem: Make handoff writer optimistically spin on owner")
|
|
Jiri Wiesner |
f08034 |
|
|
Jiri Wiesner |
f08034 |
... assumes that when the owner field is changed to NULL, the lock will
|
|
Jiri Wiesner |
f08034 |
become free soon. But commit:
|
|
Jiri Wiesner |
f08034 |
|
|
Jiri Wiesner |
f08034 |
48dfb5d2560d ("locking/rwsem: Disable preemption while trying for rwsem lock")
|
|
Jiri Wiesner |
f08034 |
|
|
Jiri Wiesner |
f08034 |
... disabled preemption when acquiring rwsem for write.
|
|
Jiri Wiesner |
f08034 |
|
|
Jiri Wiesner |
f08034 |
However, preemption has not yet been disabled when acquiring a read lock
|
|
Jiri Wiesner |
f08034 |
on a rwsem. So a reader can add a RWSEM_READER_BIAS to count without
|
|
Jiri Wiesner |
f08034 |
setting owner to signal a reader, got preempted out by a RT task which
|
|
Jiri Wiesner |
f08034 |
then spins in the writer slowpath as owner remains NULL leading to live lock.
|
|
Jiri Wiesner |
83a61e |
|
|
Jiri Wiesner |
83a61e |
One easy way to fix this problem is to disable preemption at all the
|
|
Jiri Wiesner |
83a61e |
down_read*() and up_read() code paths as implemented in this patch.
|
|
Jiri Wiesner |
83a61e |
|
|
Jiri Wiesner |
83a61e |
Fixes: 91d2a812dfb9 ("locking/rwsem: Make handoff writer optimistically spin on owner")
|
|
Jiri Wiesner |
83a61e |
Reported-by: Mukesh Ojha <quic_mojha@quicinc.com>
|
|
Jiri Wiesner |
83a61e |
Suggested-by: Peter Zijlstra <peterz@infradead.org>
|
|
Jiri Wiesner |
83a61e |
Signed-off-by: Waiman Long <longman@redhat.com>
|
|
Jiri Wiesner |
f08034 |
Signed-off-by: Ingo Molnar <mingo@kernel.org>
|
|
Jiri Wiesner |
f08034 |
Link: https://lore.kernel.org/r/20230126003628.365092-3-longman@redhat.com
|
|
Jiri Wiesner |
83a61e |
Signed-off-by: Jiri Wiesner <jwiesner@suse.de>
|
|
Jiri Wiesner |
83a61e |
---
|
|
Jiri Wiesner |
f08034 |
kernel/locking/rwsem.c | 30 ++++++++++++++++++++++++------
|
|
Jiri Wiesner |
f08034 |
1 file changed, 24 insertions(+), 6 deletions(-)
|
|
Jiri Wiesner |
83a61e |
|
|
Jiri Wiesner |
83a61e |
diff --git a/kernel/locking/rwsem.c b/kernel/locking/rwsem.c
|
|
Jiri Wiesner |
f08034 |
index be2df9ea7c30..84d5b649b95f 100644
|
|
Jiri Wiesner |
83a61e |
--- a/kernel/locking/rwsem.c
|
|
Jiri Wiesner |
83a61e |
+++ b/kernel/locking/rwsem.c
|
|
Jiri Wiesner |
f08034 |
@@ -1091,7 +1091,7 @@ rwsem_down_read_slowpath(struct rw_semaphore *sem, long count, unsigned int stat
|
|
Jiri Wiesner |
83a61e |
/* Ordered by sem->wait_lock against rwsem_mark_wake(). */
|
|
Jiri Wiesner |
83a61e |
break;
|
|
Jiri Wiesner |
83a61e |
}
|
|
Jiri Wiesner |
83a61e |
- schedule();
|
|
Jiri Wiesner |
83a61e |
+ schedule_preempt_disabled();
|
|
Jiri Wiesner |
83a61e |
lockevent_inc(rwsem_sleep_reader);
|
|
Jiri Wiesner |
83a61e |
}
|
|
Jiri Wiesner |
83a61e |
|
|
Jiri Wiesner |
f08034 |
@@ -1253,14 +1253,20 @@ static struct rw_semaphore *rwsem_downgrade_wake(struct rw_semaphore *sem)
|
|
Jiri Wiesner |
83a61e |
*/
|
|
Jiri Wiesner |
83a61e |
static inline int __down_read_common(struct rw_semaphore *sem, int state)
|
|
Jiri Wiesner |
83a61e |
{
|
|
Jiri Wiesner |
83a61e |
+ int ret = 0;
|
|
Jiri Wiesner |
83a61e |
long count;
|
|
Jiri Wiesner |
83a61e |
|
|
Jiri Wiesner |
83a61e |
+ preempt_disable();
|
|
Jiri Wiesner |
83a61e |
if (!rwsem_read_trylock(sem, &count)) {
|
|
Jiri Wiesner |
83a61e |
- if (IS_ERR(rwsem_down_read_slowpath(sem, count, state)))
|
|
Jiri Wiesner |
83a61e |
- return -EINTR;
|
|
Jiri Wiesner |
83a61e |
+ if (IS_ERR(rwsem_down_read_slowpath(sem, count, state))) {
|
|
Jiri Wiesner |
83a61e |
+ ret = -EINTR;
|
|
Jiri Wiesner |
83a61e |
+ goto out;
|
|
Jiri Wiesner |
83a61e |
+ }
|
|
Jiri Wiesner |
83a61e |
DEBUG_RWSEMS_WARN_ON(!is_rwsem_reader_owned(sem), sem);
|
|
Jiri Wiesner |
83a61e |
}
|
|
Jiri Wiesner |
83a61e |
- return 0;
|
|
Jiri Wiesner |
83a61e |
+out:
|
|
Jiri Wiesner |
83a61e |
+ preempt_enable();
|
|
Jiri Wiesner |
83a61e |
+ return ret;
|
|
Jiri Wiesner |
83a61e |
}
|
|
Jiri Wiesner |
83a61e |
|
|
Jiri Wiesner |
83a61e |
static inline void __down_read(struct rw_semaphore *sem)
|
|
Jiri Wiesner |
f08034 |
@@ -1280,19 +1286,23 @@ static inline int __down_read_killable(struct rw_semaphore *sem)
|
|
Jiri Wiesner |
83a61e |
|
|
Jiri Wiesner |
83a61e |
static inline int __down_read_trylock(struct rw_semaphore *sem)
|
|
Jiri Wiesner |
83a61e |
{
|
|
Jiri Wiesner |
83a61e |
+ int ret = 0;
|
|
Jiri Wiesner |
83a61e |
long tmp;
|
|
Jiri Wiesner |
83a61e |
|
|
Jiri Wiesner |
83a61e |
DEBUG_RWSEMS_WARN_ON(sem->magic != sem, sem);
|
|
Jiri Wiesner |
83a61e |
|
|
Jiri Wiesner |
83a61e |
+ preempt_disable();
|
|
Jiri Wiesner |
83a61e |
tmp = atomic_long_read(&sem->count);
|
|
Jiri Wiesner |
83a61e |
while (!(tmp & RWSEM_READ_FAILED_MASK)) {
|
|
Jiri Wiesner |
83a61e |
if (atomic_long_try_cmpxchg_acquire(&sem->count, &tmp,
|
|
Jiri Wiesner |
83a61e |
tmp + RWSEM_READER_BIAS)) {
|
|
Jiri Wiesner |
83a61e |
rwsem_set_reader_owned(sem);
|
|
Jiri Wiesner |
83a61e |
- return 1;
|
|
Jiri Wiesner |
83a61e |
+ ret = 1;
|
|
Jiri Wiesner |
83a61e |
+ break;
|
|
Jiri Wiesner |
83a61e |
}
|
|
Jiri Wiesner |
83a61e |
}
|
|
Jiri Wiesner |
83a61e |
- return 0;
|
|
Jiri Wiesner |
83a61e |
+ preempt_enable();
|
|
Jiri Wiesner |
83a61e |
+ return ret;
|
|
Jiri Wiesner |
83a61e |
}
|
|
Jiri Wiesner |
83a61e |
|
|
Jiri Wiesner |
83a61e |
/*
|
|
Jiri Wiesner |
f08034 |
@@ -1334,6 +1344,7 @@ static inline void __up_read(struct rw_semaphore *sem)
|
|
Jiri Wiesner |
83a61e |
DEBUG_RWSEMS_WARN_ON(sem->magic != sem, sem);
|
|
Jiri Wiesner |
83a61e |
DEBUG_RWSEMS_WARN_ON(!is_rwsem_reader_owned(sem), sem);
|
|
Jiri Wiesner |
83a61e |
|
|
Jiri Wiesner |
83a61e |
+ preempt_disable();
|
|
Jiri Wiesner |
83a61e |
rwsem_clear_reader_owned(sem);
|
|
Jiri Wiesner |
83a61e |
tmp = atomic_long_add_return_release(-RWSEM_READER_BIAS, &sem->count);
|
|
Jiri Wiesner |
83a61e |
DEBUG_RWSEMS_WARN_ON(tmp < 0, sem);
|
|
Jiri Wiesner |
f08034 |
@@ -1342,6 +1353,7 @@ static inline void __up_read(struct rw_semaphore *sem)
|
|
Jiri Wiesner |
83a61e |
clear_nonspinnable(sem);
|
|
Jiri Wiesner |
83a61e |
rwsem_wake(sem);
|
|
Jiri Wiesner |
83a61e |
}
|
|
Jiri Wiesner |
83a61e |
+ preempt_enable();
|
|
Jiri Wiesner |
83a61e |
}
|
|
Jiri Wiesner |
83a61e |
|
|
Jiri Wiesner |
83a61e |
/*
|
|
Jiri Wiesner |
f08034 |
@@ -1661,6 +1673,12 @@ void down_read_non_owner(struct rw_semaphore *sem)
|
|
Jiri Wiesner |
f08034 |
{
|
|
Jiri Wiesner |
f08034 |
might_sleep();
|
|
Jiri Wiesner |
f08034 |
__down_read(sem);
|
|
Jiri Wiesner |
f08034 |
+ /*
|
|
Jiri Wiesner |
f08034 |
+ * The owner value for a reader-owned lock is mostly for debugging
|
|
Jiri Wiesner |
f08034 |
+ * purpose only and is not critical to the correct functioning of
|
|
Jiri Wiesner |
f08034 |
+ * rwsem. So it is perfectly fine to set it in a preempt-enabled
|
|
Jiri Wiesner |
f08034 |
+ * context here.
|
|
Jiri Wiesner |
f08034 |
+ */
|
|
Jiri Wiesner |
f08034 |
__rwsem_set_reader_owned(sem, NULL);
|
|
Jiri Wiesner |
f08034 |
}
|
|
Jiri Wiesner |
f08034 |
EXPORT_SYMBOL(down_read_non_owner);
|
|
Jiri Wiesner |
83a61e |
--
|
|
Jiri Wiesner |
83a61e |
2.35.3
|
|
Jiri Wiesner |
83a61e |
|