New pthread_barrier algorithm to fulfill barrier destruction requirem…

…ents.

The previous barrier implementation did not fulfill the POSIX requirements
for when a barrier can be destroyed.  Specifically, it was possible that
threads that haven't noticed yet that their round is complete still access
the barrier's memory, and that those accesses can happen after the barrier
has been legally destroyed.
The new algorithm does not have this issue, and it avoids using a lock
internally.

ChangeLog

@@ -1,3 +1,30 @@
+2016-01-15  Torvald Riegel  <triegel@redhat.com>
+
+	[BZ #13065]
+	* nptl/pthread_barrier_wait.c (__pthread_barrier_wait): Replace with
+	new implementation.
+	* nptl/pthread_barrier_destroy.c (pthread_barrier_destroy): Likewise.
+	* nptl/pthread_barrier_init.c (__pthread_barrier_init): Adapt.
+	* sysdeps/nptl/internaltypes.h (pthread_barrier): Adapt.
+	(BARRIER_IN_THRESHOLD): New macro.
+	* nptl/pthread_barrierattr_setpshared.c
+	(pthread_barrierattr_setpshared): Clean up.
+	* nptl/tst-barrier4.c: Correct comment.
+	* nptl/tst-barrier5.c: New file.
+	* nptl/Makefile (tests): Add nptl/tst-barrier5.c.
+	(gen-as-const-headers): Remove lowlevelbarrier.sym.
+	* sysdeps/unix/sysv/linux/i386/pthread_barrier_wait.S: Remove.
+	* sysdeps/unix/sysv/linux/x86_64/pthread_barrier_wait.S: Remove.
+	* nptl/lowlevelbarrier.sym: Remove.
+	* nptl/DESIGN-barrier.txt: Remove.
+	* sysdeps/sparc/nptl/pthread_barrier_destroy.c: Remove.
+	* sysdeps/sparc/nptl/pthread_barrier_init.c: Remove.
+	* sysdeps/sparc/nptl/pthread_barrier_wait.c: Remove.
+	* sysdeps/sparc/sparc32/pthread_barrier_wait.c: Replace with build
+	error.
+	* sysdeps/sparc/sparc32/sparcv9/pthread_barrier_wait.c: Use generic
+	implementation.
+
 2016-01-15  Paul E. Murphy  <murphyp@linux.vnet.ibm.com>
 
 	* rt/tst-mqueue5.c (thr): Cleanup misleading comment.

nptl/Makefile

@@ -241,7 +241,7 @@ tests = tst-typesizes \
 	tst-sem1 tst-sem2 tst-sem3 tst-sem4 tst-sem5 tst-sem6 tst-sem7 \
 	tst-sem8 tst-sem9 tst-sem10 tst-sem11 tst-sem12 tst-sem13 tst-sem14 \
 	tst-sem15 \
-	tst-barrier1 tst-barrier2 tst-barrier3 tst-barrier4 \
+	tst-barrier1 tst-barrier2 tst-barrier3 tst-barrier4 tst-barrier5 \
 	tst-align tst-align3 \
 	tst-basic1 tst-basic2 tst-basic3 tst-basic4 tst-basic5 tst-basic6 \
 	tst-basic7 \
@@ -302,7 +302,7 @@ tests-nolibpthread = tst-unload
 
 gen-as-const-headers = pthread-errnos.sym \
 		       lowlevelcond.sym lowlevelrwlock.sym \
-		       lowlevelbarrier.sym unwindbuf.sym \
+		       unwindbuf.sym \
 		       lowlevelrobustlock.sym pthread-pi-defines.sym
 
 

nptl/pthread_barrier_destroy.c

@@ -18,25 +18,44 @@
 
 #include <errno.h>
 #include "pthreadP.h"
-#include <lowlevellock.h>
+#include <atomic.h>
+#include <futex-internal.h>
 
 
 int
 pthread_barrier_destroy (pthread_barrier_t *barrier)
 {
-  struct pthread_barrier *ibarrier;
-  int result = EBUSY;
-
-  ibarrier = (struct pthread_barrier *) barrier;
-
-  lll_lock (ibarrier->lock, ibarrier->private ^ FUTEX_PRIVATE_FLAG);
-
-  if (__glibc_likely (ibarrier->left == ibarrier->init_count))
-    /* The barrier is not used anymore.  */
-    result = 0;
-  else
-    /* Still used, return with an error.  */
-    lll_unlock (ibarrier->lock, ibarrier->private ^ FUTEX_PRIVATE_FLAG);
-
-  return result;
+  struct pthread_barrier *bar = (struct pthread_barrier *) barrier;
+
+  /* Destroying a barrier is only allowed if no thread is blocked on it.
+     Thus, there is no unfinished round, and all modifications to IN will
+     have happened before us (either because the calling thread took part
+     in the most recent round and thus synchronized-with all other threads
+     entering, or the program ensured this through other synchronization).
+     We must wait until all threads that entered so far have confirmed that
+     they have exited as well.  To get the notification, pretend that we have
+     reached the reset threshold.  */
+  unsigned int count = bar->count;
+  unsigned int max_in_before_reset = BARRIER_IN_THRESHOLD
+				   - BARRIER_IN_THRESHOLD % count;
+  /* Relaxed MO sufficient because the program must have ensured that all
+     modifications happen-before this load (see above).  */
+  unsigned int in = atomic_load_relaxed (&bar->in);
+  /* Trigger reset.  The required acquire MO is below.  */
+  if (atomic_fetch_add_relaxed (&bar->out, max_in_before_reset - in) < in)
+    {
+      /* Not all threads confirmed yet that they have exited, so another
+	 thread will perform a reset.  Wait until that has happened.  */
+      while (in != 0)
+	{
+	  futex_wait_simple (&bar->in, in, bar->shared);
+	  in = atomic_load_relaxed (&bar->in);
+	}
+    }
+  /* We must ensure that memory reuse happens after all prior use of the
+     barrier (specifically, synchronize-with the reset of the barrier or the
+     confirmation of threads leaving the barrier).  */
+  atomic_thread_fence_acquire ();
+
+  return 0;
 }

nptl/pthread_barrier_init.c

@@ -18,7 +18,7 @@
 
 #include <errno.h>
 #include "pthreadP.h"
-#include <lowlevellock.h>
+#include <futex-internal.h>
 #include <kernel-features.h>
 
 
@@ -34,8 +34,10 @@ __pthread_barrier_init (pthread_barrier_t *barrier,
 {
   struct pthread_barrier *ibarrier;
 
-  /* XXX EINVAL is not specified by POSIX as a possible error code.  */
-  if (__glibc_unlikely (count == 0))
+  /* XXX EINVAL is not specified by POSIX as a possible error code for COUNT
+     being too large.  See pthread_barrier_wait for the reason for the
+     comparison with BARRIER_IN_THRESHOLD.  */
+  if (__glibc_unlikely (count == 0 || count >= BARRIER_IN_THRESHOLD))
     return EINVAL;
 
   const struct pthread_barrierattr *iattr
@@ -46,15 +48,12 @@ __pthread_barrier_init (pthread_barrier_t *barrier,
   ibarrier = (struct pthread_barrier *) barrier;
 
   /* Initialize the individual fields.  */
-  ibarrier->lock = LLL_LOCK_INITIALIZER;
-  ibarrier->left = count;
-  ibarrier->init_count = count;
-  ibarrier->curr_event = 0;
-
-  /* XXX Don't use FUTEX_SHARED or FUTEX_PRIVATE as long as there are still
-     assembly implementations that expect the value determined below.  */
-  ibarrier->private = (iattr->pshared != PTHREAD_PROCESS_PRIVATE
-		       ? 0 : FUTEX_PRIVATE_FLAG);
+  ibarrier->in = 0;
+  ibarrier->out = 0;
+  ibarrier->count = count;
+  ibarrier->current_round = 0;
+  ibarrier->shared = (iattr->pshared == PTHREAD_PROCESS_PRIVATE
+		      ? FUTEX_PRIVATE : FUTEX_SHARED);
 
   return 0;
 }