Upadte.

2000-04-11  Ulrich Drepper  <drepper@redhat.com>

	* internals.h: Define MEMORY_BARRIER as empty if not defined already.
	* spinlock.c (__pthread_lock): Add memory barriers.
	(__pthread_unlock): Likewise.
	* sysdeps/alpha/pt-machine.h (MEMORY_BARRIER): Define using mb
	instruction.
	(RELEASE): Not needed anymore.
	(__compare_and_swap): Mark asm as modifying memory.
	* sysdeps/powerpc/pt-machine.h (sync): Remove.  Replace with definition
	of MEMORY_BARRIER.
	(__compare_and_swap): Use MEMORY_BARRIER instead of sync.
	* sysdeps/sparc/sparc32/pt-machine.h (RELEASE): Not needed anymore.
	(MEMORY_BARRIER): Define using stbar.
	* sysdeps/sparc/sparc64/pt-machine.h (MEMORY_BARRIER): Define using
	stbar.
	(__compare_and_swap): Use MEMORY_BARRIER to ensure ordering.
	Patch by Xavier Leroy <Xavier.Leroy@inria.fr> based on comments by
	Mike Burrows <m3b@pa.dec.com>.

7 files changed, 61 insertions, 17 deletions

diff --git a/linuxthreads/ChangeLog b/linuxthreads/ChangeLog
index 86e02aa..9802ccf 100644
--- a/linuxthreads/ChangeLog
+++ b/linuxthreads/ChangeLog
@@ -1,3 +1,23 @@
+2000-04-11  Ulrich Drepper  <drepper@redhat.com>
+
+	* internals.h: Define MEMORY_BARRIER as empty if not defined already.
+	* spinlock.c (__pthread_lock): Add memory barriers.
+	(__pthread_unlock): Likewise.
+	* sysdeps/alpha/pt-machine.h (MEMORY_BARRIER): Define using mb
+	instruction.
+	(RELEASE): Not needed anymore.
+	(__compare_and_swap): Mark asm as modifying memory.
+	* sysdeps/powerpc/pt-machine.h (sync): Remove.  Replace with definition
+	of MEMORY_BARRIER.
+	(__compare_and_swap): Use MEMORY_BARRIER instead of sync.
+	* sysdeps/sparc/sparc32/pt-machine.h (RELEASE): Not needed anymore.
+	(MEMORY_BARRIER): Define using stbar.
+	* sysdeps/sparc/sparc64/pt-machine.h (MEMORY_BARRIER): Define using
+	stbar.
+	(__compare_and_swap): Use MEMORY_BARRIER to ensure ordering.
+	Patch by Xavier Leroy <Xavier.Leroy@inria.fr> based on comments by
+	Mike Burrows <m3b@pa.dec.com>.
+
 2000-04-09  Ulrich Drepper  <drepper@redhat.com>
 
 	* signals.c (sigaction): Fix return value for the case SIG is one
diff --git a/linuxthreads/internals.h b/linuxthreads/internals.h
index 8cb9b68..3984512 100644
--- a/linuxthreads/internals.h
+++ b/linuxthreads/internals.h
@@ -357,6 +357,13 @@ static inline pthread_descr thread_self (void)
 #endif
 }
 
+/* If MEMORY_BARRIER isn't defined in pt-machine.h, assume the architecture
+   doesn't need a memory barrier instruction (e.g. Intel x86) */
+
+#ifndef MEMORY_BARRIER
+#define MEMORY_BARRIER()
+#endif
+
 /* Max number of times we must spin on a spinlock calling sched_yield().
    After MAX_SPIN_COUNT iterations, we put the calling thread to sleep. */
 
diff --git a/linuxthreads/spinlock.c b/linuxthreads/spinlock.c
index b1a99d9..e1c40c0 100644
--- a/linuxthreads/spinlock.c
+++ b/linuxthreads/spinlock.c
@@ -54,6 +54,9 @@ void internal_function __pthread_lock(struct _pthread_fastlock * lock,
     if (self != NULL) {
       ASSERT(self->p_nextlock == NULL);
       THREAD_SETMEM(self, p_nextlock, (pthread_descr) oldstatus);
+      /* Make sure the store in p_nextlock completes before performing
+         the compare-and-swap */
+      MEMORY_BARRIER();
     }
   } while(! compare_and_swap(&lock->__status, oldstatus, newstatus,
                              &lock->__spinlock));
@@ -108,8 +111,17 @@ again:
       maxprio = thr->p_priority;
     }
     ptr = &(thr->p_nextlock);
+    /* Prevent reordering of the load of lock->__status above and the
+       load of *ptr below, as well as reordering of *ptr between
+       several iterations of the while loop.  Some processors (e.g.
+       multiprocessor Alphas) could perform such reordering even though
+       the loads are dependent. */
+    MEMORY_BARRIER();
     thr = *ptr;
   }
+  /* Prevent reordering of the load of lock->__status above and
+     thr->p_nextlock below */
+  MEMORY_BARRIER();
   /* Remove max prio thread from waiting list. */
   if (maxptr == (pthread_descr *) &lock->__status) {
     /* If max prio thread is at head, remove it with compare-and-swap
@@ -124,6 +136,9 @@ again:
     thr = *maxptr;
     *maxptr = thr->p_nextlock;
   }
+  /* Prevent reordering of store to *maxptr above and store to thr->p_nextlock
+     below */
+  MEMORY_BARRIER();
   /* Wake up the selected waiting thread */
   thr->p_nextlock = NULL;
   restart(thr);
@@ -149,6 +164,8 @@ int __pthread_compare_and_swap(long * ptr, long oldval, long newval,
   } else {
     res = 0;
   }
+  /* Prevent reordering of store to *ptr above and store to *spinlock below */
+  MEMORY_BARRIER();
   *spinlock = 0;
   return res;
 }
diff --git a/linuxthreads/sysdeps/alpha/pt-machine.h b/linuxthreads/sysdeps/alpha/pt-machine.h
index e59c690..25ba634 100644
--- a/linuxthreads/sysdeps/alpha/pt-machine.h
+++ b/linuxthreads/sysdeps/alpha/pt-machine.h
@@ -32,6 +32,10 @@
 register char *stack_pointer __asm__("$30");
 
 
+/* Memory barrier; default is to do nothing */
+#define MEMORY_BARRIER() __asm__ __volatile__("mb" : : : "memory")
+
+
 /* Spinlock implementation; required.  */
 PT_EI long int
 testandset (int *spinlock)
@@ -55,11 +59,6 @@ testandset (int *spinlock)
   return ret;
 }
 
-/* Spinlock release; default is just set to zero.  */
-#define RELEASE(spinlock) \
-  __asm__ __volatile__("mb" : : : "memory"); \
-  *spinlock = 0
-
 
 /* Begin allocating thread stacks at this address.  Default is to allocate
    them just below the initial program stack.  */
@@ -102,7 +101,8 @@ __compare_and_swap (long int *p, long int oldval, long int newval)
 	"2:\tmb\n"
 	"/* End compare & swap */"
 	: "=&r"(ret), "=m"(*p)
-	: "r"(oldval), "r"(newval), "m"(*p));
+	: "r"(oldval), "r"(newval), "m"(*p)
+        : "memory");
 
   return ret;
 }
diff --git a/linuxthreads/sysdeps/powerpc/pt-machine.h b/linuxthreads/sysdeps/powerpc/pt-machine.h
index 578369a..c7d0670 100644
--- a/linuxthreads/sysdeps/powerpc/pt-machine.h
+++ b/linuxthreads/sysdeps/powerpc/pt-machine.h
@@ -27,12 +27,7 @@
 
 /* For multiprocessor systems, we want to ensure all memory accesses
    are completed before we reset a lock.  */
-#if 0
-/* on non multiprocessor systems, you can just: */
-#define sync() /* nothing */
-#else
-#define sync() __asm__ __volatile__ ("sync")
-#endif
+#define MEMORY_BARRIER() __asm__ __volatile__ ("sync" : : : "memory")
 
 /* Get some notion of the current stack.  Need not be exactly the top
    of the stack, just something somewhere in the current frame.  */
@@ -64,6 +59,6 @@ __compare_and_swap (long int *p, long int oldval, long int newval)
 	: "=&r"(ret)
 	: "r"(p), "r"(newval), "r"(oldval)
 	: "cr0", "memory");
-  sync();
+  MEMORY_BARRIER();
   return ret == 0;
 }
diff --git a/linuxthreads/sysdeps/sparc/sparc32/pt-machine.h b/linuxthreads/sysdeps/sparc/sparc32/pt-machine.h
index 69af857..ddf4932 100644
--- a/linuxthreads/sysdeps/sparc/sparc32/pt-machine.h
+++ b/linuxthreads/sysdeps/sparc/sparc32/pt-machine.h
@@ -37,9 +37,8 @@ testandset (int *spinlock)
 }
 
 
-/* Spinlock release; default is just set to zero.  */
-#define RELEASE(spinlock) \
-  __asm__ __volatile__("stbar; stb %1,%0" : "=m"(*(spinlock)) : "r"(0));
+/* Memory barrier; default is to do nothing */
+#define MEMORY_BARRIER() __asm__ __volatile__("stbar" : : : "memory")
 
 
 /* Get some notion of the current stack.  Need not be exactly the top
diff --git a/linuxthreads/sysdeps/sparc/sparc64/pt-machine.h b/linuxthreads/sysdeps/sparc/sparc64/pt-machine.h
index 5560003..02167bd 100644
--- a/linuxthreads/sysdeps/sparc/sparc64/pt-machine.h
+++ b/linuxthreads/sysdeps/sparc/sparc64/pt-machine.h
@@ -37,6 +37,12 @@ testandset (int *spinlock)
 }
 
 
+/* Memory barrier; default is to do nothing */
+/* FIXME: is stbar OK, or should we use the more general membar instruction?
+   If so, which mode to pass to membar? */
+#define MEMORY_BARRIER() __asm__ __volatile__("stbar" : : : "memory")
+
+
 /* Get some notion of the current stack.  Need not be exactly the top
    of the stack, just something somewhere in the current frame.  */
 #define CURRENT_STACK_FRAME  stack_pointer
@@ -66,7 +72,7 @@ __compare_and_swap (long int *p, long int oldval, long int newval)
   __asm__ __volatile__ ("casx	[%4], %2, %0"
 			: "=r"(readval), "=m"(*p)
 			: "r"(oldval), "m"(*p), "r"(p), "0"(newval));
-
+  MEMORY_BARRIER();
   return readval == oldval;
 }