Skip to content

Commit 990fa73

Browse files
Waiman-LongIngo Molnar
authored andcommitted
locking/rwsem: More optimal RT task handling of null owner
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, there are two cases where we may want to continue spinning: 1) The lock owner is in the process of releasing the lock, sem->owner is cleared but the lock has not been released yet. 2) The lock was free and owner cleared, but another task just comes in and acquire the lock before we try to get it. The new owner may be a spinnable writer. So an RT task is now made to retry one more time to see if it can acquire the lock or continue spinning on the new owning writer. 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 <longman@redhat.com> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> Cc: Borislav Petkov <bp@alien8.de> Cc: Davidlohr Bueso <dave@stgolabs.net> Cc: H. Peter Anvin <hpa@zytor.com> Cc: Linus Torvalds <torvalds@linux-foundation.org> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: Tim Chen <tim.c.chen@linux.intel.com> Cc: Will Deacon <will.deacon@arm.com> Cc: huang ying <huang.ying.caritas@gmail.com> Link: https://lkml.kernel.org/r/20190520205918.22251-10-longman@redhat.com Signed-off-by: Ingo Molnar <mingo@kernel.org>
1 parent 00f3c5a commit 990fa73

File tree

1 file changed

+44
-7
lines changed

1 file changed

+44
-7
lines changed

kernel/locking/rwsem.c

Lines changed: 44 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -566,6 +566,7 @@ static noinline enum owner_state rwsem_spin_on_owner(struct rw_semaphore *sem)
566566
static bool rwsem_optimistic_spin(struct rw_semaphore *sem)
567567
{
568568
bool taken = false;
569+
int prev_owner_state = OWNER_NULL;
569570

570571
preempt_disable();
571572

@@ -583,7 +584,12 @@ static bool rwsem_optimistic_spin(struct rw_semaphore *sem)
583584
* 2) readers own the lock as we can't determine if they are
584585
* actively running or not.
585586
*/
586-
while (rwsem_spin_on_owner(sem) & OWNER_SPINNABLE) {
587+
for (;;) {
588+
enum owner_state owner_state = rwsem_spin_on_owner(sem);
589+
590+
if (!(owner_state & OWNER_SPINNABLE))
591+
break;
592+
587593
/*
588594
* Try to acquire the lock
589595
*/
@@ -593,13 +599,44 @@ static bool rwsem_optimistic_spin(struct rw_semaphore *sem)
593599
}
594600

595601
/*
596-
* When there's no owner, we might have preempted between the
597-
* owner acquiring the lock and setting the owner field. If
598-
* we're an RT task that will live-lock because we won't let
599-
* the owner complete.
602+
* An RT task cannot do optimistic spinning if it cannot
603+
* be sure the lock holder is running or live-lock may
604+
* happen if the current task and the lock holder happen
605+
* to run in the same CPU. However, aborting optimistic
606+
* spinning while a NULL owner is detected may miss some
607+
* opportunity where spinning can continue without causing
608+
* problem.
609+
*
610+
* There are 2 possible cases where an RT task may be able
611+
* to continue spinning.
612+
*
613+
* 1) The lock owner is in the process of releasing the
614+
* lock, sem->owner is cleared but the lock has not
615+
* been released yet.
616+
* 2) The lock was free and owner cleared, but another
617+
* task just comes in and acquire the lock before
618+
* we try to get it. The new owner may be a spinnable
619+
* writer.
620+
*
621+
* To take advantage of two scenarios listed agove, the RT
622+
* task is made to retry one more time to see if it can
623+
* acquire the lock or continue spinning on the new owning
624+
* writer. Of course, if the time lag is long enough or the
625+
* new owner is not a writer or spinnable, the RT task will
626+
* quit spinning.
627+
*
628+
* If the owner is a writer, the need_resched() check is
629+
* done inside rwsem_spin_on_owner(). If the owner is not
630+
* a writer, need_resched() check needs to be done here.
600631
*/
601-
if (!sem->owner && (need_resched() || rt_task(current)))
602-
break;
632+
if (owner_state != OWNER_WRITER) {
633+
if (need_resched())
634+
break;
635+
if (rt_task(current) &&
636+
(prev_owner_state != OWNER_WRITER))
637+
break;
638+
}
639+
prev_owner_state = owner_state;
603640

604641
/*
605642
* The cpu_relax() call is a compiler barrier which forces

0 commit comments

Comments
 (0)