Messages in this thread Patch in this message | | | Date | Tue, 27 Sep 2022 20:33:23 +0530 | Subject | Re: locking/rwsem: RT throttling issue due to RT task hogging the cpu | From | Mukesh Ojha <> |
| |
I was thinking if below patch can also help on this issue.
--------------------------------->O---------------------------
diff --git a/kernel/locking/rwsem.c b/kernel/locking/rwsem.c index 65f0262..dbe3e16 100644 --- a/kernel/locking/rwsem.c +++ b/kernel/locking/rwsem.c @@ -628,8 +628,8 @@ static inline bool rwsem_try_write_lock(struct rw_semaphore *sem, new = count;
if (count & RWSEM_LOCK_MASK) { - if (has_handoff || (!rt_task(waiter->task) && - !time_after(jiffies, waiter->timeout))) + if (has_handoff || (rt_task(waiter->task) && waiter != first) || + (!rt_task(waiter->task) && !time_after(jiffies, waiter->timeout))) return false;
-Mukesh
On 9/26/2022 5:16 PM, Mukesh Ojha wrote: > Hi, > > Any comments on this issue would be helpful. > > Thanks, > Mukesh > > On 9/20/2022 9:49 PM, Mukesh Ojha wrote: >> Hi, >> >> We are observing one issue where, sem->owner is not set and >> sem->count=6 [1] which means both RWSEM_FLAG_WAITERS and >> RWSEM_FLAG_HANDOFF bits are set. And if unfold the sem->wait_list we >> see the following order of process waiting [2] where [a] is waiting >> for write, while [b],[c] are waiting for read and [d] is the RT task >> for which waiter.handoff_set=true and it is continuously running on >> cpu7 and not letting the first write waiter [a] on cpu7. >> >> [1] >> >> sem = 0xFFFFFFD57DDC6680 -> ( >> count = (counter = 6), >> owner = (counter = 0), >> >> [2] >> >> [a] kworker/7:0 pid: 32516 ==> [b] iptables-restor pid: 18625 ==> >> [c]HwBinder:1544_3 pid: 2024 ==> [d] RenderEngine pid: 2032 cpu: 7 >> prio:97 (RT task) >> >> >> Sometime back, Waiman has suggested this which could help in RT task >> leaving the cpu. >> >> https://lore.kernel.org/all/8c33f989-8870-08c6-db12-521de634b34e@redhat.com/ >> >> >> --------------------------------->O---------------------------- >> >> From c6493edd7a5e4f597ea55ff0eb3f1d763b335dfc Mon Sep 17 00:00:00 2001 >> 2 From: Waiman Long <longman@redhat.com> >> 3 Date: Tue, 20 Sep 2022 20:50:45 +0530 >> 4 Subject: [PATCH] locking/rwsem: Yield the cpu after doing handoff >> optimistic >> 5 spinning >> 6 >> 7 It is possible the new lock owner (writer) can be preempted >> before setting >> 8 the owner field and if the current(e.g RT task) waiter is the >> task that >> 9 preempts the new lock owner, it will hand_off spin loop for a >> long time. >> 10 Avoid wasting cpu time and delaying the release of the lock by >> yielding >> 11 the cpu if handoff optimistic spinning has been done multiple >> times with >> 12 NULL owner. >> 13 >> 14 Signed-off-by: Waiman Long <longman@redhat.com> >> 15 Signed-off-by: Mukesh Ojha <quic_mojha@quicinc.com> >> 16 --- >> 17 kernel/locking/rwsem.c | 15 ++++++++++++++- >> 18 1 file changed, 14 insertions(+), 1 deletion(-) >> 19 >> 20 diff --git a/kernel/locking/rwsem.c b/kernel/locking/rwsem.c >> 21 index 65f0262..a875758 100644 >> 22 --- a/kernel/locking/rwsem.c >> 23 +++ b/kernel/locking/rwsem.c >> 24 @@ -361,6 +361,8 @@ enum rwsem_wake_type { >> 25 */ >> 26 #define MAX_READERS_WAKEUP 0x100 >> 27 >> 28 +#define MAX_HANDOFF_SPIN 10 >> 29 + >> 30 static inline void >> 31 rwsem_add_waiter(struct rw_semaphore *sem, struct rwsem_waiter >> *waiter) >> 32 { >> 33 @@ -1109,6 +1111,7 @@ rwsem_down_write_slowpath(struct >> rw_semaphore *sem, int state) >> 34 { >> 35 struct rwsem_waiter waiter; >> 36 DEFINE_WAKE_Q(wake_q); >> 37 + int handoff_spins = 0; >> 38 >> 39 /* do optimistic spinning and steal lock if possible */ >> 40 if (rwsem_can_spin_on_owner(sem) && >> rwsem_optimistic_spin(sem)) { >> 41 @@ -1167,6 +1170,14 @@ rwsem_down_write_slowpath(struct >> rw_semaphore *sem, int state) >> 42 * has just released the lock, OWNER_NULL will be >> returned. >> 43 * In this case, we attempt to acquire the lock >> again >> 44 * without sleeping. >> 45 + * >> 46 + * It is possible the new lock owner (writer) can >> be preempted >> 47 + * before setting the owner field and if the >> current(e.g RT task) >> 48 + * waiter is the task that preempts the new lock >> owner, it will >> 49 + * spin in this loop for a long time. Avoid >> wasting cpu time >> 50 + * and delaying the release of the lock by >> yielding the cpu if >> 51 + * handoff optimistic spinning has been done >> multiple times with >> 52 + * NULL owner. >> 53 */ >> 54 if (waiter.handoff_set) { >> 55 enum owner_state owner_state; >> 56 @@ -1175,8 +1186,10 @@ rwsem_down_write_slowpath(struct >> rw_semaphore *sem, int state) >> 57 owner_state = rwsem_spin_on_owner(sem); >> 58 preempt_enable(); >> 59 >> 60 - if (owner_state == OWNER_NULL) >> 61 + if ((owner_state == OWNER_NULL) && >> (handoff_spins < MAX_HANDOFF_SPIN)) { >> 62 + handoff_spins++; >> 63 goto trylock_again; >> 64 + } >> 65 } >> 66 >> 67 schedule(); >> 68 -- >> 69 2.7.4 >> 70 >> >> >> -Mukesh
| |