In message: [linux-yocto][v5.15/standard/base][PATCH] locking/rwsem: Disable
preemption while trying for rwsem lock
on 10/03/2024 Li Wang via lists.yoctoproject.org wrote:
> From: Gokul krishna Krishnakumar <quic_gokuk...@quicinc.com>
>
> commit 48dfb5d2560d36fb16c7d430c229d1604ea7d185 upstream
>
> 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:
> 1. CFS task (affined to a particular CPU) takes rwsem lock.
>
> 2. CFS task gets preempted by a RT task before setting owner.
>
> 3. 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.
Any thoughts on why this hasn't been picked up by -stable ?
Bruce
>
> Signed-off-by: Gokul krishna Krishnakumar <quic_gokuk...@quicinc.com>
> Signed-off-by: Mukesh Ojha <quic_mo...@quicinc.com>
> Signed-off-by: Peter Zijlstra (Intel) <pet...@infradead.org>
> Reviewed-by: Waiman Long <long...@redhat.com>
> Link:
> https://lore.kernel.org/r/1662661467-24203-1-git-send-email-quic_mo...@quicinc.com
> Signed-off-by: Beniamin Sandu <beniamin.sa...@windriver.com>
> Signed-off-by: Li Wang <li.w...@windriver.com>
> ---
> 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 f0287a16b4ec..4a38d32b89fa 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
> + * preempt disable section as the atomic op that changes sem->count.
> */
> static inline void rwsem_set_owner(struct rw_semaphore *sem)
> {
> + 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;
> }
>
> /*
> @@ -1341,8 +1349,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.25.1
>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#13673):
https://lists.yoctoproject.org/g/linux-yocto/message/13673
Mute This Topic: https://lists.yoctoproject.org/mt/104857955/21656
Group Owner: linux-yocto+ow...@lists.yoctoproject.org
Unsubscribe: https://lists.yoctoproject.org/g/linux-yocto/unsub
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
