Compiler memory barriers and pthread barrier fix
authorBarret Rhoden <brho@cs.berkeley.edu>
Mon, 3 May 2010 00:32:11 +0000 (17:32 -0700)
committerKevin Klues <klueska@cs.berkeley.edu>
Thu, 3 Nov 2011 00:35:47 +0000 (17:35 -0700)
Also touches up some loose ends in kernel synch.  The pthread bearier
works on the core i7's, etc.

kern/arch/i686/atomic.h
kern/arch/i686/ros/membar.h
kern/arch/sparc/ros/membar.h
kern/include/atomic.h
kern/src/Makefrag
kern/src/atomic.c
kern/src/kfs.c
user/include/pthread.h
user/parlib/pthread.c

index 34bcae6..70716a8 100644 (file)
@@ -2,6 +2,7 @@
 #define ROS_INCLUDE_ATOMIC_H
 
 #include <ros/common.h>
+#include <ros/arch/membar.h>
 #include <arch/x86.h>
 #include <arch/arch.h>
 
@@ -130,6 +131,9 @@ static inline void spin_lock(spinlock_t *lock)
 
 static inline void spin_unlock(spinlock_t *lock)
 {
+       /* Need to prevent the compiler (and some arches) from reordering older
+        * stores */
+       wmb();
        lock->rlock = 0;
 }
 
index 3d7d020..85732a4 100644 (file)
@@ -1,11 +1,12 @@
 #ifndef _ARCH_MEMBAR_H
 #define _ARCH_MEMBAR_H
 
-#define mb() ({ asm volatile("mfence"); })
-#define rmb() ({ asm volatile("lfence"); })
-#define wmb() 
+/* Adding "memory" to keep the compiler from fucking with us */
+#define mb() ({ asm volatile("mfence" ::: "memory"); })
+#define rmb() ({ asm volatile("lfence" ::: "memory"); })
+#define wmb() ({ asm volatile("" ::: "memory"); })
 /* Force a wmb, used in cases where an IPI could beat a write, even though
- *  * write-orderings are respected. */
+ *  * write-orderings are respected. TODO: this probably do what you want. */
 #define wmb_f() ({ asm volatile("sfence"); })
 
 #endif
index 2e8cc34..79fef5b 100644 (file)
@@ -3,7 +3,7 @@
 
 #define mb() {rmb(); wmb();}
 #define rmb()
-#define wmb() ({ __asm__ __volatile__ ("stbar"); })
+#define wmb() ({ __asm__ __volatile__ ("stbar" ::: "memory"); })
 /* Force a wmb, used in cases where an IPI could beat a write, even though
  *  * write-orderings are respected.  (Used by x86) */
 #define wmb_f() wmb()
index 3dc56c0..898547f 100644 (file)
@@ -141,6 +141,9 @@ static inline void __seq_start_write(seq_ctr_t *seq_ctr)
        assert(*seq_ctr % 2 == 0);
 #endif
        (*seq_ctr)++;
+       /* We're the only writer, so we need to prevent the compiler (and some
+        * arches) from reordering writes before this point. */
+       wmb();
 }
 
 static inline void __seq_end_write(seq_ctr_t *seq_ctr)
@@ -148,6 +151,9 @@ static inline void __seq_end_write(seq_ctr_t *seq_ctr)
 #ifdef _CONFIG_SEQLOCK_DEBUG_
        assert(*seq_ctr % 2 == 1);
 #endif
+       /* Need to prevent the compiler (and some arches) from reordering older
+        * stores */
+       wmb();
        (*seq_ctr)++;
 }
 
index fa36147..bcade7a 100644 (file)
@@ -58,6 +58,7 @@ KERN_APPFILES += \
                  $(TESTS_DIR)/hello \
                  $(TESTS_DIR)/mhello \
                  $(TESTS_DIR)/pthread_test \
+                 $(TESTS_DIR)/pthread_barrier_test \
                  $(TESTS_DIR)/idle \
                  $(TESTS_DIR)/fillmeup \
                  $(TESTS_DIR)/msr_get_cores \
index 1724d09..006ece6 100644 (file)
@@ -139,7 +139,7 @@ void waiton_barrier(barrier_t* barrier)
        } else {
                spin_unlock_irqsave(&barrier->lock);
                reset_barrier(barrier);
-               // if we need to wmb(), it'll be here
+               wmb();
                barrier->ready++;
        }
 }
index ca6de29..8562747 100644 (file)
@@ -45,6 +45,7 @@ DECL_PROG(mproctests);
 DECL_PROG(hello);
 DECL_PROG(mhello);
 DECL_PROG(pthread_test);
+DECL_PROG(pthread_barrier_test);
 DECL_PROG(idle);
 DECL_PROG(fillmeup);
 DECL_PROG(msr_get_cores);
@@ -64,6 +65,7 @@ struct kfs_entry kfs[MAX_KFS_FILES] = {
        KFS_PENTRY(hello)
        KFS_PENTRY(mhello)
        KFS_PENTRY(pthread_test)
+       KFS_PENTRY(pthread_barrier_test)
        KFS_PENTRY(idle)
        KFS_PENTRY(fillmeup)
        KFS_PENTRY(msr_get_cores)
index 3690aef..185fe9e 100644 (file)
@@ -69,8 +69,6 @@ typedef struct
 #define MAX_PTHREADS 32
 typedef struct
 {
-  int in_use[MAX_PTHREADS];
-  int next_slot;
   volatile int sense;
   int count;
   int nprocs;
index 84f49fc..65cf504 100644 (file)
@@ -406,8 +406,9 @@ int pthread_mutex_trylock(pthread_mutex_t* m)
 
 int pthread_mutex_unlock(pthread_mutex_t* m)
 {
-  // Need a memory barrier here to serialize all memory writes before unlocking
-  mb(); 
+  /* Need to prevent the compiler (and some arches) from reordering older
+   * stores */
+  wmb();
   m->lock = 0;
   return 0;
 }
@@ -577,9 +578,7 @@ int pthread_once(pthread_once_t* once_control, void (*init_routine)(void))
 
 int pthread_barrier_init(pthread_barrier_t* b, const pthread_barrierattr_t* a, int count)
 {
-  memset(b->in_use,0,sizeof(b->in_use));
   b->nprocs = b->count = count;
-  b->next_slot = 0;
   b->sense = 0;
   pthread_mutex_init(&b->pmutex, 0);
   return 0;
@@ -588,17 +587,6 @@ int pthread_barrier_init(pthread_barrier_t* b, const pthread_barrierattr_t* a, i
 int pthread_barrier_wait(pthread_barrier_t* b)
 {
   unsigned int spinner = 0;
-  int id = b->next_slot;
-  int old_id = b->next_slot;
-
-  //allocate a slot
-
-  while (atomic_swap (& (b->in_use[id]), SLOT_IN_USE) == SLOT_IN_USE)
-  {
-    id = (id + 1) % MAX_PTHREADS;
-       assert (old_id != id);  // do not want to wrap around
-  }
-  b->next_slot = (id + 1) % MAX_PTHREADS;
   int ls = !b->sense;
 
   pthread_mutex_lock(&b->pmutex);
@@ -608,9 +596,8 @@ int pthread_barrier_wait(pthread_barrier_t* b)
   if(count == 0)
   {
     printd("Thread %d is last to hit the barrier, resetting...\n", pthread_self()->id);
-    memset(b->in_use,0,sizeof(b->in_use));
     b->count = b->nprocs;
-    b->next_slot = 0;
+       wmb();
     b->sense = ls;
     return PTHREAD_BARRIER_SERIAL_THREAD;
   }