Update.

* sysdeps/unix/sysv/linux/i386/i486/pthread_barrier_wait.S
	(pthread_barrier_wait): After wakeup, release lock only when the
	last thread stopped using the barrier object.
	* sysdeps/unix/sysv/linux/x86_64/pthread_barrier_wait.S
	(pthread_barrier_wait): Likewise.
	* sysdeps/pthread/pthread_barrier_wait.c (pthread_barrier_wait):
	Likewise.
	* Makefile (tests): Add tst-barrier4.
	* tst-barrier4.c: New file.

nptl/ChangeLog

@@ -1,5 +1,14 @@
 2004-02-18  Ulrich Drepper  <drepper@redhat.com>
 
+	* sysdeps/unix/sysv/linux/i386/i486/pthread_barrier_wait.S
+	(pthread_barrier_wait): After wakeup, release lock only when the
+	last thread stopped using the barrier object.
+	* sysdeps/unix/sysv/linux/x86_64/pthread_barrier_wait.S
+	(pthread_barrier_wait): Likewise.
+	* sysdeps/pthread/pthread_barrier_wait.c (pthread_barrier_wait):
+	Likewise.
+	* Makefile (tests): Add tst-barrier4.
+	* tst-barrier4.c: New file.
 
 	* sysdeps/unix/sysv/linux/i386/i486/pthread_cond_timedwait.S
 	(__pthread_cond_timedwait): Perform timeout test while holding

nptl/DESIGN-barrier.txt

@@ -1,7 +1,7 @@
 Barriers pseudocode
 ===================
 
-    int pthread_barrier_wait(barrier_t * barrier);
+    int pthread_barrier_wait(barrier_t *barrier);
 
 struct barrier_t {
 
@@ -21,29 +21,27 @@ struct barrier_t {
 pthread_barrier_wait(barrier_t *barrier)
 {
   unsigned int event;
+  result = 0;
 
   lll_lock(barrier->lock);
   if (!--barrier->left) {
-    barrier->left = barrier->init_count;   
     barrier->curr_event++;
     futex_wake(&barrier->curr_event, INT_MAX)
-    lll_unlock(barrier->lock);
 
-    return BARRIER_SERIAL_THREAD;
+    result = BARRIER_SERIAL_THREAD;
+  } else {
+    event = barrier->curr_event;
+    do {
+      lll_unlock(barrier->lock);
+
+      futex_wait(&barrier->curr_event, event)
+
+      lll_lock(barrier->lock);
+    } while (event == barrier->curr_event);
   }
 
-  event = barrier->curr_event;
-  for (;;) {
+  if (atomic_exchange_and_add (barrier->left, 1) == barrier->init_count - 1)
     lll_unlock(barrier->lock);
 
-    futex_wait(&barrier->curr_event, event)
-
-    lll_lock(barrier->lock);
-    if (event != barrier->curr_event)
-      break;
-  }
-  lll_unlock(barrier->lock);
-
-  return 0;
+  return result;
 }
-

nptl/Makefile

@@ -201,7 +201,7 @@ tests = tst-attr1 tst-attr2 tst-attr3 \
 	tst-key1 tst-key2 tst-key3 tst-key4 \
 	tst-sem1 tst-sem2 tst-sem3 tst-sem4 tst-sem5 tst-sem6 tst-sem7 \
 	tst-sem8 tst-sem9 \
-	tst-barrier1 tst-barrier2 tst-barrier3 \
+	tst-barrier1 tst-barrier2 tst-barrier3 tst-barrier4 \
 	tst-align \
 	tst-basic1 tst-basic2 tst-basic3 tst-basic4 tst-basic5 tst-basic6 \
 	tst-kill1 tst-kill2 tst-kill3 tst-kill4 tst-kill5 tst-kill6 \

nptl/sysdeps/pthread/pthread_barrier_wait.c

@@ -1,4 +1,4 @@
-/* Copyright (C) 2003 Free Software Foundation, Inc.
+/* Copyright (C) 2003, 2004 Free Software Foundation, Inc.
    This file is part of the GNU C Library.
    Contributed by Martin Schwidefsky <schwidefsky@de.ibm.com>, 2003.
 
@@ -29,6 +29,7 @@ pthread_barrier_wait (barrier)
      pthread_barrier_t *barrier;
 {
   struct pthread_barrier *ibarrier = (struct pthread_barrier *) barrier;
+  int result = 0;
 
   /* Make sure we are alone.  */
   lll_lock (ibarrier->lock);
@@ -39,41 +40,42 @@ pthread_barrier_wait (barrier)
   /* Are these all?  */
   if (ibarrier->left == 0)
     {
-      /* Yes.  Restore the barrier to be empty.  */
-      ibarrier->left = ibarrier->init_count;
-
-      /* Increment the event counter to avoid invalid wake-ups and
+      /* Yes. Increment the event counter to avoid invalid wake-ups and
 	 tell the current waiters that it is their turn.  */
       ++ibarrier->curr_event;
 
       /* Wake up everybody.  */
       lll_futex_wake (&ibarrier->curr_event, INT_MAX);
 
-      /* The barrier is open for business again.  */
-      lll_unlock (ibarrier->lock);
-
       /* This is the thread which finished the serialization.  */
-      return PTHREAD_BARRIER_SERIAL_THREAD;
+      result = PTHREAD_BARRIER_SERIAL_THREAD;
     }
-
-  /* The number of the event we are waiting for.  The barrier's event
-     number must be bumped before we continue.  */
-  unsigned int event = ibarrier->curr_event;
-  do
+  else
     {
-      /* Before suspending, make the barrier available to others.  */
-      lll_unlock (ibarrier->lock);
+      /* The number of the event we are waiting for.  The barrier's event
+	 number must be bumped before we continue.  */
+      unsigned int event = ibarrier->curr_event;
+      do
+	{
+	  /* Before suspending, make the barrier available to others.  */
+	  lll_unlock (ibarrier->lock);
 
-      /* Wait for the event counter of the barrier to change.  */
-      lll_futex_wait (&ibarrier->curr_event, event);
+	  /* Wait for the event counter of the barrier to change.  */
+	  lll_futex_wait (&ibarrier->curr_event, event);
 
-      /* We are going to access shared data.  */
-      lll_lock (ibarrier->lock);
+	  /* We are going to access shared data.  */
+	  lll_lock (ibarrier->lock);
+	}
+      while (event == ibarrier->curr_event);
     }
-  while (event == ibarrier->curr_event);
 
-  /* We are done, let others use the barrier.  */
-  lll_unlock (ibarrier->lock);
+  /* Make sure the init_count is stored locally or in a register.  */
+  unsigned int init_count = ibarrier->init_count;
 
-  return 0;
+  /* If this was the last woken thread, unlock.  */
+  if (atomic_exchange_and_add (ibarrier->left, 1) == init_count - 1)
+    /* We are done.  */
+    lll_unlock (ibarrier->lock);
+
+  return result;
 }

nptl/sysdeps/unix/sysv/linux/i386/i486/pthread_barrier_wait.S

@@ -1,4 +1,4 @@
-/* Copyright (C) 2002, 2003 Free Software Foundation, Inc.
+/* Copyright (C) 2002, 2003, 2004 Free Software Foundation, Inc.
    This file is part of the GNU C Library.
    Contributed by Ulrich Drepper <drepper@redhat.com>, 2002.
 
@@ -84,16 +84,32 @@ pthread_barrier_wait:
 #endif
 	je,pn	8b
 
+	/* Increment LEFT.  If this brings the count back to the
+	   initial count unlock the object.  */
+	movl	$1, %edx
+	movl	INIT_COUNT(%ebx), %ecx
+	LOCK
+	xaddl	%edx, LEFT(%ebx)
+	subl	$1, %ecx
+	cmpl	%ecx, %edx
+	jne,pt	10f
+
+	/* Release the mutex.  We cannot release the lock before
+	   waking the waiting threads since otherwise a new thread might
+	   arrive and gets waken up, too.  */
+	LOCK
+	subl	$1, MUTEX(%ebx)
+	jne	9f
+
 	/* Note: %esi is still zero.  */
-	movl	%esi, %eax		/* != PTHREAD_BARRIER_SERIAL_THREAD */
+10:	movl	%esi, %eax		/* != PTHREAD_BARRIER_SERIAL_THREAD */
 
 	popl	%esi
 	popl	%ebx
 	ret
 
 	/* The necessary number of threads arrived.  */
-3:	movl	INIT_COUNT(%ebx), %eax
-	movl	%eax, LEFT(%ebx)
+3:
 #if CURR_EVENT == 0
 	addl	$1, (%ebx)
 #else
@@ -107,6 +123,16 @@ pthread_barrier_wait:
 	movl	$SYS_futex, %eax
 	ENTER_KERNEL
 
+	/* Increment LEFT.  If this brings the count back to the
+	   initial count unlock the object.  */
+	movl	$1, %edx
+	movl	INIT_COUNT(%ebx), %ecx
+	LOCK
+	xaddl	%edx, LEFT(%ebx)
+	subl	$1, %ecx
+	cmpl	%ecx, %edx
+	jne,pt	5f
+
 	/* Release the mutex.  We cannot release the lock before
 	   waking the waiting threads since otherwise a new thread might
 	   arrive and gets waken up, too.  */
@@ -130,4 +156,8 @@ pthread_barrier_wait:
 6:	leal	MUTEX(%ebx), %eax
 	call	__lll_mutex_unlock_wake
 	jmp	7b
+
+9:	leal	MUTEX(%ebx), %eax
+	call	__lll_mutex_unlock_wake
+	jmp	10b
 	.size	pthread_barrier_wait,.-pthread_barrier_wait

nptl/sysdeps/unix/sysv/linux/x86_64/pthread_barrier_wait.S

@@ -1,4 +1,4 @@
-/* Copyright (C) 2002, 2003 Free Software Foundation, Inc.
+/* Copyright (C) 2002, 2003, 2004 Free Software Foundation, Inc.
    This file is part of the GNU C Library.
    Contributed by Ulrich Drepper <drepper@redhat.com>, 2002.
 
@@ -78,14 +78,29 @@ pthread_barrier_wait:
 #endif
 	je	8b
 
-	/* Note: %esi is still zero.  */
-	movl	%esi, %eax		/* != PTHREAD_BARRIER_SERIAL_THREAD */
+	/* Increment LEFT.  If this brings the count back to the
+	   initial count unlock the object.  */
+	movl	$1, %edx
+	movl	INIT_COUNT(%rdi), %eax
+	LOCK
+	xaddl	%edx, LEFT(%rdi)
+	subl	$1, %eax
+	cmpl	%eax, %edx
+	jne,pt	10f
+
+	/* Release the mutex.  We cannot release the lock before
+	   waking the waiting threads since otherwise a new thread might
+	   arrive and gets waken up, too.  */
+	LOCK
+	decl	MUTEX(%rdi)
+	jne	9f
+
+10:	xorl	%eax, %eax		/* != PTHREAD_BARRIER_SERIAL_THREAD */
 
 	retq
 
 	/* The necessary number of threads arrived.  */
-3:	movl	INIT_COUNT(%rdi), %eax
-	movl	%eax, LEFT(%rdi)
+3:
 #if CURR_EVENT == 0
 	incl	(%rdi)
 #else
@@ -99,6 +114,16 @@ pthread_barrier_wait:
 	movq	$SYS_futex, %rax
 	syscall
 
+	/* Increment LEFT.  If this brings the count back to the
+	   initial count unlock the object.  */
+	movl	$1, %edx
+	movl	INIT_COUNT(%rdi), %eax
+	LOCK
+	xaddl	%edx, LEFT(%rdi)
+	subl	$1, %eax
+	cmpl	%eax, %edx
+	jne,pt	5f
+
 	/* Release the mutex.  We cannot release the lock before
 	   waking the waiting threads since otherwise a new thread might
 	   arrive and gets waken up, too.  */
@@ -117,11 +142,14 @@ pthread_barrier_wait:
 
 4:	addq	$MUTEX, %rdi
 	callq	__lll_mutex_unlock_wake
-	subq	$MUTEX, %rdi
 	jmp	5b
 
 6:	addq	$MUTEX, %rdi
 	callq	__lll_mutex_unlock_wake
 	subq	$MUTEX, %rdi
 	jmp	7b
+
+9:	addq	$MUTEX, %rdi
+	callq	__lll_mutex_unlock_wake
+	jmp	10b
 	.size	pthread_barrier_wait,.-pthread_barrier_wait

nptl/tst-barrier4.c

@@ -0,0 +1,102 @@
+/* Copyright (C) 2004 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+   Contributed by Ulrich Drepper <drepper@redhat.com>, 2004.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, write to the Free
+   Software Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
+   02111-1307 USA.  */
+
+/* This is a test for behavior not guaranteed by POSIX.  */
+#include <errno.h>
+#include <pthread.h>
+#include <stdio.h>
+
+
+static pthread_barrier_t b1;
+static pthread_barrier_t b2;
+
+
+#define N 20
+
+static void *
+tf (void *arg)
+{
+  int round = 0;
+
+  while (round++ < 30)
+    {
+      if (pthread_barrier_wait (&b1) == PTHREAD_BARRIER_SERIAL_THREAD)
+	{
+	  pthread_barrier_destroy (&b1);
+	  if (pthread_barrier_init (&b1, NULL, N) != 0)
+	    {
+	      puts ("tf: 1st barrier_init failed");
+	      exit (1);
+	    }
+	}
+
+      if (pthread_barrier_wait (&b2) == PTHREAD_BARRIER_SERIAL_THREAD)
+	{
+	  pthread_barrier_destroy (&b2);
+	  if (pthread_barrier_init (&b2, NULL, N) != 0)
+	    {
+	      puts ("tf: 2nd barrier_init failed");
+	      exit (1);
+	    }
+	}
+    }
+
+  return NULL;
+}
+
+
+static int
+do_test (void)
+{
+  int cnt;
+
+  if (pthread_barrier_init (&b1, NULL, N) != 0)
+    {
+      puts ("1st barrier_init failed");
+      return 1;
+    }
+
+  if (pthread_barrier_init (&b2, NULL, N) != 0)
+    {
+      puts ("2nd barrier_init failed");
+      return 1;
+    }
+
+  pthread_t th[N - 1];
+  for (cnt = 0; cnt < N - 1; ++cnt)
+    if (pthread_create (&th[cnt], NULL, tf, NULL) != 0)
+      {
+	puts ("pthread_create failed");
+	return 1;
+      }
+
+  tf (NULL);
+
+  for (cnt = 0; cnt < N - 1; ++cnt)
+    if (pthread_join (th[cnt], NULL) != 0)
+      {
+	puts ("pthread_join failed");
+	return 1;
+      }
+
+  return 0;
+}
+
+#define TEST_FUNCTION do_test ()
+#include "../test-skeleton.c"