From 5fc9ed4c4058bfbdf51ad6e7aac7d209b580e8c4 Mon Sep 17 00:00:00 2001 From: Carlos O'Donell Date: Mon, 21 Jan 2019 22:50:12 -0500 Subject: nptl: Fix pthread_rwlock_try*lock stalls (Bug 23844) For a full analysis of both the pthread_rwlock_tryrdlock() stall and the pthread_rwlock_trywrlock() stall see: https://sourceware.org/bugzilla/show_bug.cgi?id=23844#c14 In the pthread_rwlock_trydlock() function we fail to inspect for PTHREAD_RWLOCK_FUTEX_USED in __wrphase_futex and wake the waiting readers. In the pthread_rwlock_trywrlock() function we write 1 to __wrphase_futex and loose the setting of the PTHREAD_RWLOCK_FUTEX_USED bit, again failing to wake waiting readers during unlock. The fix in the case of pthread_rwlock_trydlock() is to check for PTHREAD_RWLOCK_FUTEX_USED and wake the readers. The fix in the case of pthread_rwlock_trywrlock() is to only write 1 to __wrphase_futex if we installed the write phase, since all other readers would be spinning waiting for this step. We add two new tests, one exercises the stall for pthread_rwlock_trywrlock() which is easy to exercise, and one exercises the stall for pthread_rwlock_trydlock() which is harder to exercise. The pthread_rwlock_trywrlock() test fails consistently without the fix, and passes after. The pthread_rwlock_tryrdlock() test fails roughly 5-10% of the time without the fix, and passes all the time after. Signed-off-by: Carlos O'Donell Signed-off-by: Torvald Riegel Signed-off-by: Rik Prohaska Co-authored-by: Torvald Riegel Co-authored-by: Rik Prohaska --- nptl/pthread_rwlock_tryrdlock.c | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) (limited to 'nptl/pthread_rwlock_tryrdlock.c') diff --git a/nptl/pthread_rwlock_tryrdlock.c b/nptl/pthread_rwlock_tryrdlock.c index 368862ff07..2f94f17f36 100644 --- a/nptl/pthread_rwlock_tryrdlock.c +++ b/nptl/pthread_rwlock_tryrdlock.c @@ -94,15 +94,22 @@ __pthread_rwlock_tryrdlock (pthread_rwlock_t *rwlock) /* Same as in __pthread_rwlock_rdlock_full: We started the read phase, so we are also responsible for updating the write-phase futex. Relaxed MO is sufficient. - Note that there can be no other reader that we have to wake - because all other readers will see the read phase started by us - (or they will try to start it themselves); if a writer started - the read phase, we cannot have started it. Furthermore, we - cannot discard a PTHREAD_RWLOCK_FUTEX_USED flag because we will - overwrite the value set by the most recent writer (or the readers - before it in case of explicit hand-over) and we know that there - are no waiting readers. */ - atomic_store_relaxed (&rwlock->__data.__wrphase_futex, 0); + We have to do the same steps as a writer would when handing over the + read phase to use because other readers cannot distinguish between + us and the writer. + Note that __pthread_rwlock_tryrdlock callers will not have to be + woken up because they will either see the read phase started by us + or they will try to start it themselves; however, callers of + __pthread_rwlock_rdlock_full just increase the reader count and then + check what state the lock is in, so they cannot distinguish between + us and a writer that acquired and released the lock in the + meantime. */ + if ((atomic_exchange_relaxed (&rwlock->__data.__wrphase_futex, 0) + & PTHREAD_RWLOCK_FUTEX_USED) != 0) + { + int private = __pthread_rwlock_get_private (rwlock); + futex_wake (&rwlock->__data.__wrphase_futex, INT_MAX, private); + } } return 0; -- cgit 1.4.1