diff options
author | Prateek Sood <prsood@codeaurora.org> | 2017-09-07 10:30:58 -0400 |
---|---|---|
committer | Ingo Molnar <mingo@kernel.org> | 2017-09-29 04:10:20 -0400 |
commit | 9c29c31830a4eca724e137a9339137204bbb31be (patch) | |
tree | 633605dec4ffbc387640e12136eba911ea1a4cc4 | |
parent | c74aef2d06a9f59cece89093eecc552933cba72a (diff) |
locking/rwsem-xadd: Fix missed wakeup due to reordering of load
If a spinner is present, there is a chance that the load of
rwsem_has_spinner() in rwsem_wake() can be reordered with
respect to decrement of rwsem count in __up_write() leading
to wakeup being missed:
spinning writer up_write caller
--------------- -----------------------
[S] osq_unlock() [L] osq
spin_lock(wait_lock)
sem->count=0xFFFFFFFF00000001
+0xFFFFFFFF00000000
count=sem->count
MB
sem->count=0xFFFFFFFE00000001
-0xFFFFFFFF00000001
spin_trylock(wait_lock)
return
rwsem_try_write_lock(count)
spin_unlock(wait_lock)
schedule()
Reordering of atomic_long_sub_return_release() in __up_write()
and rwsem_has_spinner() in rwsem_wake() can cause missing of
wakeup in up_write() context. In spinning writer, sem->count
and local variable count is 0XFFFFFFFE00000001. It would result
in rwsem_try_write_lock() failing to acquire rwsem and spinning
writer going to sleep in rwsem_down_write_failed().
The smp_rmb() will make sure that the spinner state is
consulted after sem->count is updated in up_write context.
Signed-off-by: Prateek Sood <prsood@codeaurora.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: dave@stgolabs.net
Cc: longman@redhat.com
Cc: parri.andrea@gmail.com
Cc: sramana@codeaurora.org
Link: http://lkml.kernel.org/r/1504794658-15397-1-git-send-email-prsood@codeaurora.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
-rw-r--r-- | kernel/locking/rwsem-xadd.c | 27 |
1 files changed, 27 insertions, 0 deletions
diff --git a/kernel/locking/rwsem-xadd.c b/kernel/locking/rwsem-xadd.c index 02f660666ab8..1fefe6dcafd7 100644 --- a/kernel/locking/rwsem-xadd.c +++ b/kernel/locking/rwsem-xadd.c | |||
@@ -613,6 +613,33 @@ struct rw_semaphore *rwsem_wake(struct rw_semaphore *sem) | |||
613 | DEFINE_WAKE_Q(wake_q); | 613 | DEFINE_WAKE_Q(wake_q); |
614 | 614 | ||
615 | /* | 615 | /* |
616 | * __rwsem_down_write_failed_common(sem) | ||
617 | * rwsem_optimistic_spin(sem) | ||
618 | * osq_unlock(sem->osq) | ||
619 | * ... | ||
620 | * atomic_long_add_return(&sem->count) | ||
621 | * | ||
622 | * - VS - | ||
623 | * | ||
624 | * __up_write() | ||
625 | * if (atomic_long_sub_return_release(&sem->count) < 0) | ||
626 | * rwsem_wake(sem) | ||
627 | * osq_is_locked(&sem->osq) | ||
628 | * | ||
629 | * And __up_write() must observe !osq_is_locked() when it observes the | ||
630 | * atomic_long_add_return() in order to not miss a wakeup. | ||
631 | * | ||
632 | * This boils down to: | ||
633 | * | ||
634 | * [S.rel] X = 1 [RmW] r0 = (Y += 0) | ||
635 | * MB RMB | ||
636 | * [RmW] Y += 1 [L] r1 = X | ||
637 | * | ||
638 | * exists (r0=1 /\ r1=0) | ||
639 | */ | ||
640 | smp_rmb(); | ||
641 | |||
642 | /* | ||
616 | * If a spinner is present, it is not necessary to do the wakeup. | 643 | * If a spinner is present, it is not necessary to do the wakeup. |
617 | * Try to do wakeup only if the trylock succeeds to minimize | 644 | * Try to do wakeup only if the trylock succeeds to minimize |
618 | * spinlock contention which may introduce too much delay in the | 645 | * spinlock contention which may introduce too much delay in the |