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] -=-=-=-=-=-=-=-=-=-=-=-