An RT task can do optimistic spinning only if the lock holder is actually running. If the state of the lock holder isn't known, there is a possibility that high priority of the RT task may block forward progress of the lock holder if it happens to reside on the same CPU. This will lead to deadlock. So we have to make sure that an RT task will not spin on a reader-owned rwsem.
When the owner is temporarily set to NULL, it is more tricky to decide if an RT task should stop spinning as it may be a temporary state where another writer may have just stolen the lock which then failed the task's trylock attempt. So one more retry is allowed to make sure that the lock is not spinnable by an RT task. When testing on a 8-socket IvyBridge-EX system, the one additional retry seems to improve locking performance of RT write locking threads under heavy contentions. The table below shows the locking rates (in kops/s) with various write locking threads before and after the patch. Locking threads Pre-patch Post-patch --------------- --------- ----------- 4 2,753 2,608 8 2,529 2,520 16 1,727 1,918 32 1,263 1,956 64 889 1,343 Signed-off-by: Waiman Long <long...@redhat.com> --- kernel/locking/rwsem-xadd.c | 36 +++++++++++++++++++++++++++++------- 1 file changed, 29 insertions(+), 7 deletions(-) diff --git a/kernel/locking/rwsem-xadd.c b/kernel/locking/rwsem-xadd.c index d38cf76ac17c..de1bf9fea1e2 100644 --- a/kernel/locking/rwsem-xadd.c +++ b/kernel/locking/rwsem-xadd.c @@ -349,6 +349,8 @@ static noinline enum owner_state rwsem_spin_on_owner(struct rw_semaphore *sem) static bool rwsem_optimistic_spin(struct rw_semaphore *sem) { bool taken = false; + bool is_rt_task = rt_task(current); + int prev_owner_state = OWNER_NULL; preempt_disable(); @@ -366,7 +368,12 @@ static bool rwsem_optimistic_spin(struct rw_semaphore *sem) * 2) readers own the lock as we can't determine if they are * actively running or not. */ - while (rwsem_spin_on_owner(sem) & OWNER_SPINNABLE) { + for (;;) { + enum owner_state owner_state = rwsem_spin_on_owner(sem); + + if (!(owner_state & OWNER_SPINNABLE)) + break; + /* * Try to acquire the lock */ @@ -376,13 +383,28 @@ static bool rwsem_optimistic_spin(struct rw_semaphore *sem) } /* - * When there's no owner, we might have preempted between the - * owner acquiring the lock and setting the owner field. If - * we're an RT task that will live-lock because we won't let - * the owner complete. + * An RT task cannot do optimistic spinning if it cannot + * be sure the lock holder is running or live-lock may + * happen if the current task and the lock holder happen + * to run in the same CPU. + * + * When there's no owner or is reader-owned, an RT task + * will stop spinning if the owner state is not a writer + * at the previous iteration of the loop. This allows the + * RT task to recheck if the task that steals the lock is + * a spinnable writer. If so, it can keeps on spinning. + * + * If the owner is a writer, the need_resched() check is + * done inside rwsem_spin_on_owner(). If the owner is not + * a writer, need_resched() check needs to be done here. */ - if (!sem->owner && (need_resched() || rt_task(current))) - break; + if (owner_state != OWNER_WRITER) { + if (need_resched()) + break; + if (is_rt_task && (prev_owner_state != OWNER_WRITER)) + break; + } + prev_owner_state = owner_state; /* * The cpu_relax() call is a compiler barrier which forces -- 2.18.1