On 3/5/24 06:04, Aaro Koskinen wrote:
Hi,
It seems this patch (commit 48dfb5d2560d) is missing from at least 5.15 stable tree.
Based on quick test, it seems to fix an issue where system locks up easily when RT throttling is disabled, and also it applies cleanly, so I think it should be good to have it 5.15?
You need to cc stable as the locking maintainers are not responsible for merging patches to stable trees.
Cheers, Longman
A.
On Thu, Sep 08, 2022 at 11:54:27PM +0530, Mukesh Ojha wrote:
From: Gokul krishna Krishnakumar quic_gokukris@quicinc.com
Make the region inside the rwsem_write_trylock non preemptible.
We observe RT task is hogging CPU when trying to acquire rwsem lock which was acquired by a kworker task but before the rwsem owner was set.
Here is the scenario:
CFS task (affined to a particular CPU) takes rwsem lock.
CFS task gets preempted by a RT task before setting owner.
RT task (FIFO) is trying to acquire the lock, but spinning until
RT throttling happens for the lock as the lock was taken by CFS task.
This patch attempts to fix the above issue by disabling preemption until owner is set for the lock. While at it also fix the issues at the places where rwsem_{set,clear}_owner() are called.
This also adds lockdep annotation of preemption disable in rwsem_{set,clear}_owner() on Peter Z. suggestion.
Signed-off-by: Gokul krishna Krishnakumar quic_gokukris@quicinc.com Signed-off-by: Mukesh Ojha quic_mojha@quicinc.com
Changes in v2:
- Remove preempt disable code in rwsem_try_write_lock_unqueued()
- Addressed suggestion from Peter Z.
- Modified commit text
kernel/locking/rwsem.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/kernel/locking/rwsem.c b/kernel/locking/rwsem.c index 65f0262..4487359 100644 --- a/kernel/locking/rwsem.c +++ b/kernel/locking/rwsem.c @@ -133,14 +133,19 @@
- the owner value concurrently without lock. Read from owner, however,
- may not need READ_ONCE() as long as the pointer value is only used
- for comparison and isn't being dereferenced.
- Both rwsem_{set,clear}_owner() functions should be in the same
*/ static inline void rwsem_set_owner(struct rw_semaphore *sem) {
- preempt disable section as the atomic op that changes sem->count.
- lockdep_assert_preemption_disabled(); atomic_long_set(&sem->owner, (long)current); }
static inline void rwsem_clear_owner(struct rw_semaphore *sem) {
- lockdep_assert_preemption_disabled(); atomic_long_set(&sem->owner, 0); }
@@ -251,13 +256,16 @@ static inline bool rwsem_read_trylock(struct rw_semaphore *sem, long *cntp) static inline bool rwsem_write_trylock(struct rw_semaphore *sem) { long tmp = RWSEM_UNLOCKED_VALUE;
- bool ret = false;
- preempt_disable(); if (atomic_long_try_cmpxchg_acquire(&sem->count, &tmp, RWSEM_WRITER_LOCKED)) { rwsem_set_owner(sem);
return true;
ret = true;
- return false;
- preempt_enable();
- return ret; }
/* @@ -1352,8 +1360,10 @@ static inline void __up_write(struct rw_semaphore *sem) DEBUG_RWSEMS_WARN_ON((rwsem_owner(sem) != current) && !rwsem_test_oflags(sem, RWSEM_NONSPINNABLE), sem);
- preempt_disable(); rwsem_clear_owner(sem); tmp = atomic_long_fetch_add_release(-RWSEM_WRITER_LOCKED, &sem->count);
- preempt_enable(); if (unlikely(tmp & RWSEM_FLAG_WAITERS)) rwsem_wake(sem); }
-- 2.7.4
Stable team,
Please cherry-pick this patch into v5.15 stable:
locking/rwsem: Disable preemption while trying for rwsem lock
commit 48dfb5d2560d36fb16c7d430c229d1604ea7d185
It fixes the following bug present in v5.15:
From: Gokul krishna Krishnakumar quic_gokukris@quicinc.com We observe RT task is hogging CPU when trying to acquire rwsem lock which was acquired by a kworker task but before the rwsem owner was set.
Here is the scenario:
CFS task (affined to a particular CPU) takes rwsem lock.
CFS task gets preempted by a RT task before setting owner.
RT task (FIFO) is trying to acquire the lock, but spinning until
RT throttling happens for the lock as the lock was taken by CFS task.
If the RT throttling is disabled, the RT task will remain looping forever in the kernel. If the system is UP, it will lock up completely.
The issue can be easily reproduced by running RT task and normal task which are affined to the same CPU core.
A.
On Tue, Mar 05, 2024 at 09:55:47AM -0500, Waiman Long wrote:
On 3/5/24 06:04, Aaro Koskinen wrote:
Hi,
It seems this patch (commit 48dfb5d2560d) is missing from at least 5.15 stable tree.
Based on quick test, it seems to fix an issue where system locks up easily when RT throttling is disabled, and also it applies cleanly, so I think it should be good to have it 5.15?
You need to cc stable as the locking maintainers are not responsible for merging patches to stable trees.
Cheers, Longman
A.
On Thu, Sep 08, 2022 at 11:54:27PM +0530, Mukesh Ojha wrote:
From: Gokul krishna Krishnakumar quic_gokukris@quicinc.com
Make the region inside the rwsem_write_trylock non preemptible.
We observe RT task is hogging CPU when trying to acquire rwsem lock which was acquired by a kworker task but before the rwsem owner was set.
Here is the scenario:
CFS task (affined to a particular CPU) takes rwsem lock.
CFS task gets preempted by a RT task before setting owner.
RT task (FIFO) is trying to acquire the lock, but spinning until
RT throttling happens for the lock as the lock was taken by CFS task.
This patch attempts to fix the above issue by disabling preemption until owner is set for the lock. While at it also fix the issues at the places where rwsem_{set,clear}_owner() are called.
This also adds lockdep annotation of preemption disable in rwsem_{set,clear}_owner() on Peter Z. suggestion.
Signed-off-by: Gokul krishna Krishnakumar quic_gokukris@quicinc.com Signed-off-by: Mukesh Ojha quic_mojha@quicinc.com
Changes in v2:
- Remove preempt disable code in rwsem_try_write_lock_unqueued()
- Addressed suggestion from Peter Z.
- Modified commit text
kernel/locking/rwsem.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/kernel/locking/rwsem.c b/kernel/locking/rwsem.c index 65f0262..4487359 100644 --- a/kernel/locking/rwsem.c +++ b/kernel/locking/rwsem.c @@ -133,14 +133,19 @@
- the owner value concurrently without lock. Read from owner, however,
- may not need READ_ONCE() as long as the pointer value is only used
- for comparison and isn't being dereferenced.
- Both rwsem_{set,clear}_owner() functions should be in the same
*/ static inline void rwsem_set_owner(struct rw_semaphore *sem) {
- preempt disable section as the atomic op that changes sem->count.
- lockdep_assert_preemption_disabled(); atomic_long_set(&sem->owner, (long)current); } static inline void rwsem_clear_owner(struct rw_semaphore *sem) {
- lockdep_assert_preemption_disabled(); atomic_long_set(&sem->owner, 0); }
@@ -251,13 +256,16 @@ static inline bool rwsem_read_trylock(struct rw_semaphore *sem, long *cntp) static inline bool rwsem_write_trylock(struct rw_semaphore *sem) { long tmp = RWSEM_UNLOCKED_VALUE;
- bool ret = false;
- preempt_disable(); if (atomic_long_try_cmpxchg_acquire(&sem->count, &tmp, RWSEM_WRITER_LOCKED)) { rwsem_set_owner(sem);
return true;
ret = true;
- return false;
- preempt_enable();
- return ret; } /*
@@ -1352,8 +1360,10 @@ static inline void __up_write(struct rw_semaphore *sem) DEBUG_RWSEMS_WARN_ON((rwsem_owner(sem) != current) && !rwsem_test_oflags(sem, RWSEM_NONSPINNABLE), sem);
- preempt_disable(); rwsem_clear_owner(sem); tmp = atomic_long_fetch_add_release(-RWSEM_WRITER_LOCKED, &sem->count);
- preempt_enable(); if (unlikely(tmp & RWSEM_FLAG_WAITERS)) rwsem_wake(sem); }
-- 2.7.4
On Tue, Apr 02, 2024 at 08:30:32PM +0300, Aaro Koskinen wrote:
Stable team,
Please cherry-pick this patch into v5.15 stable:
locking/rwsem: Disable preemption while trying for rwsem lock
commit 48dfb5d2560d36fb16c7d430c229d1604ea7d185
It fixes the following bug present in v5.15:
From: Gokul krishna Krishnakumar quic_gokukris@quicinc.com We observe RT task is hogging CPU when trying to acquire rwsem lock which was acquired by a kworker task but before the rwsem owner was set.
Here is the scenario:
CFS task (affined to a particular CPU) takes rwsem lock.
CFS task gets preempted by a RT task before setting owner.
RT task (FIFO) is trying to acquire the lock, but spinning until
RT throttling happens for the lock as the lock was taken by CFS task.
If the RT throttling is disabled, the RT task will remain looping forever in the kernel. If the system is UP, it will lock up completely.
The issue can be easily reproduced by running RT task and normal task which are affined to the same CPU core.
Does this only affect the -rt patchset, or is this an issue with a "clean" kernel release?
thanks,
greg k-h
Hi,
On Tue, Apr 02, 2024 at 07:42:03PM +0200, Greg KH wrote:
On Tue, Apr 02, 2024 at 08:30:32PM +0300, Aaro Koskinen wrote:
Stable team,
Please cherry-pick this patch into v5.15 stable:
locking/rwsem: Disable preemption while trying for rwsem lock
commit 48dfb5d2560d36fb16c7d430c229d1604ea7d185
It fixes the following bug present in v5.15:
From: Gokul krishna Krishnakumar quic_gokukris@quicinc.com We observe RT task is hogging CPU when trying to acquire rwsem lock which was acquired by a kworker task but before the rwsem owner was set.
Here is the scenario:
CFS task (affined to a particular CPU) takes rwsem lock.
CFS task gets preempted by a RT task before setting owner.
RT task (FIFO) is trying to acquire the lock, but spinning until
RT throttling happens for the lock as the lock was taken by CFS task.
If the RT throttling is disabled, the RT task will remain looping forever in the kernel. If the system is UP, it will lock up completely.
The issue can be easily reproduced by running RT task and normal task which are affined to the same CPU core.
Does this only affect the -rt patchset, or is this an issue with a "clean" kernel release?
It's with a normal "clean" kernel.
A.
On Tue, Apr 02, 2024 at 09:25:11PM +0300, Aaro Koskinen wrote:
Hi,
On Tue, Apr 02, 2024 at 07:42:03PM +0200, Greg KH wrote:
On Tue, Apr 02, 2024 at 08:30:32PM +0300, Aaro Koskinen wrote:
Stable team,
Please cherry-pick this patch into v5.15 stable:
locking/rwsem: Disable preemption while trying for rwsem lock
commit 48dfb5d2560d36fb16c7d430c229d1604ea7d185
It fixes the following bug present in v5.15:
From: Gokul krishna Krishnakumar quic_gokukris@quicinc.com We observe RT task is hogging CPU when trying to acquire rwsem lock which was acquired by a kworker task but before the rwsem owner was set.
Here is the scenario:
CFS task (affined to a particular CPU) takes rwsem lock.
CFS task gets preempted by a RT task before setting owner.
RT task (FIFO) is trying to acquire the lock, but spinning until
RT throttling happens for the lock as the lock was taken by CFS task.
If the RT throttling is disabled, the RT task will remain looping forever in the kernel. If the system is UP, it will lock up completely.
The issue can be easily reproduced by running RT task and normal task which are affined to the same CPU core.
Does this only affect the -rt patchset, or is this an issue with a "clean" kernel release?
It's with a normal "clean" kernel.
Fair enough, now queued up, thanks.
greg k-h
linux-stable-mirror@lists.linaro.org
-
Aaro Koskinen -
Greg KH -
Waiman Long