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 <carlos@redhat.com>
Signed-off-by: Torvald Riegel <triegel@redhat.com>
Signed-off-by: Rik Prohaska <prohaska7@gmail.com>
Co-authored-by: Torvald Riegel <triegel@redhat.com>
Co-authored-by: Rik Prohaska <prohaska7@gmail.com>

1 files changed, 16 insertions, 9 deletions

diff --git a/nptl/pthread_rwlock_tryrdlock.c b/nptl/pthread_rwlock_tryrdlock.c
index 368862f..2f94f17 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;