Inlines spinlocks when not using SPINLOCK_DEBUG
authorBarret Rhoden <brho@cs.berkeley.edu>
Thu, 20 Dec 2012 03:20:34 +0000 (19:20 -0800)
committerBarret Rhoden <brho@cs.berkeley.edu>
Thu, 20 Dec 2012 03:20:34 +0000 (19:20 -0800)
This one's for Andrew.  =)  Additionally, this moves the responsibility
for memory barriers to the architectures, which cuts down on the number
of full mb()s needed by RISCV (esp since Andrew was already doing one in
there).

Also, I imagine sparc wasn't compiling, so this might work for them too,
if anyone still uses it.

kern/arch/i686/atomic.h
kern/arch/sparc/atomic.h
kern/include/atomic.h
kern/src/atomic.c

index 5dfbfad..839cb3e 100644 (file)
@@ -158,6 +158,7 @@ static inline void __spin_lock_raw(volatile uint32_t *rlock)
                        "       cmpb $0, %%al;        "
                        "       jne 1b;               "
                : : "m"(*rlock) : "eax", "cc");
                        "       cmpb $0, %%al;        "
                        "       jne 1b;               "
                : : "m"(*rlock) : "eax", "cc");
+       cmb();  /* need cmb(), the CPU mb() was handled by the xchg */
 }
 
 static inline void __spin_lock(spinlock_t *lock)
 }
 
 static inline void __spin_lock(spinlock_t *lock)
@@ -167,6 +168,9 @@ static inline void __spin_lock(spinlock_t *lock)
 
 static inline void __spin_unlock(spinlock_t *lock)
 {
 
 static inline void __spin_unlock(spinlock_t *lock)
 {
+       /* Need to prevent the compiler from reordering older stores. */
+       wmb();
+       rwmb(); /* x86 makes both of these a cmb() */
        lock->rlock = 0;
 }
 
        lock->rlock = 0;
 }
 
index b55d037..4612e49 100644 (file)
@@ -10,7 +10,7 @@ static inline void spin_lock_irqsave(spinlock_t *lock);
 static inline void spin_unlock_irqsave(spinlock_t *lock);
 /* Same deal with spin locks.  Note that Sparc can deadlock on an atomic op
  * called from interrupt context (TODO) */
 static inline void spin_unlock_irqsave(spinlock_t *lock);
 /* Same deal with spin locks.  Note that Sparc can deadlock on an atomic op
  * called from interrupt context (TODO) */
-static inline void spin_lock(spinlock_t *lock);
+static inline void __spin_lock(spinlock_t *lock);
 
 /* Actual functions */
 static inline void atomic_init(atomic_t *number, long val)
 
 /* Actual functions */
 static inline void atomic_init(atomic_t *number, long val)
@@ -32,7 +32,7 @@ static inline long atomic_fetch_and_add(atomic_t *number, long val)
        long retval;
        /* this is pretty clever.  the lower 8 bits (i.e byte 3)
         * of the atomic_t serve as a spinlock.  let's acquire it. */
        long retval;
        /* this is pretty clever.  the lower 8 bits (i.e byte 3)
         * of the atomic_t serve as a spinlock.  let's acquire it. */
-       spin_lock((spinlock_t*)number);
+       __spin_lock((spinlock_t*)number);
        retval = atomic_read(number);
        /* compute new counter value. */
        val += retval;
        retval = atomic_read(number);
        /* compute new counter value. */
        val += retval;
@@ -49,7 +49,7 @@ static inline void atomic_add(atomic_t *number, long val)
 static inline void atomic_set(atomic_t *number, long val)
 {
        // this works basically the same as atomic_add... but without the add
 static inline void atomic_set(atomic_t *number, long val)
 {
        // this works basically the same as atomic_add... but without the add
-       spin_lock((spinlock_t*)number);
+       __spin_lock((spinlock_t*)number);
        atomic_init(number,val);
 }
 
        atomic_init(number,val);
 }
 
@@ -71,7 +71,7 @@ static inline bool atomic_add_not_zero(atomic_t *number, long val)
        bool retval = FALSE;
        /* this is pretty clever.  the lower 8 bits (i.e byte 3)
         * of the atomic_t serve as a spinlock.  let's acquire it. */
        bool retval = FALSE;
        /* this is pretty clever.  the lower 8 bits (i.e byte 3)
         * of the atomic_t serve as a spinlock.  let's acquire it. */
-       spin_lock((spinlock_t*)number);
+       __spin_lock((spinlock_t*)number);
        num = atomic_read(number);
        if (num) {
                num += val;
        num = atomic_read(number);
        if (num) {
                num += val;
@@ -89,7 +89,7 @@ static inline bool atomic_sub_and_test(atomic_t *number, long val)
        bool retval = FALSE;
        /* this is pretty clever.  the lower 8 bits (i.e byte 3)
         * of the atomic_t serve as a spinlock.  let's acquire it. */
        bool retval = FALSE;
        /* this is pretty clever.  the lower 8 bits (i.e byte 3)
         * of the atomic_t serve as a spinlock.  let's acquire it. */
-       spin_lock((spinlock_t*)number);
+       __spin_lock((spinlock_t*)number);
        num = atomic_read(number);
        num -= val;
        retval = num ? FALSE : TRUE;
        num = atomic_read(number);
        num -= val;
        retval = num ? FALSE : TRUE;
@@ -103,7 +103,7 @@ static inline void atomic_and(atomic_t *number, long mask)
        long val;
        /* this is pretty clever.  the lower 8 bits (i.e byte 3)
         * of the atomic_t serve as a spinlock.  let's acquire it. */
        long val;
        /* this is pretty clever.  the lower 8 bits (i.e byte 3)
         * of the atomic_t serve as a spinlock.  let's acquire it. */
-       spin_lock((spinlock_t*)number);
+       __spin_lock((spinlock_t*)number);
        val = atomic_read(number);
        /* compute new counter value. */
        val &= mask;
        val = atomic_read(number);
        /* compute new counter value. */
        val &= mask;
@@ -116,7 +116,7 @@ static inline void atomic_or(atomic_t *number, long mask)
        long val;
        /* this is pretty clever.  the lower 8 bits (i.e byte 3)
         * of the atomic_t serve as a spinlock.  let's acquire it. */
        long val;
        /* this is pretty clever.  the lower 8 bits (i.e byte 3)
         * of the atomic_t serve as a spinlock.  let's acquire it. */
-       spin_lock((spinlock_t*)number);
+       __spin_lock((spinlock_t*)number);
        val = atomic_read(number);
        /* compute new counter value. */
        val |= mask;
        val = atomic_read(number);
        /* compute new counter value. */
        val |= mask;
@@ -173,15 +173,16 @@ static inline bool spin_locked(spinlock_t*SAFE lock)
        return (bool)reg;
 }
 
        return (bool)reg;
 }
 
-static inline void spin_lock(spinlock_t*SAFE lock)
+static inline void __spin_lock(spinlock_t*SAFE lock)
 {
        while(spin_trylock(lock))
                while(spin_locked(lock));
 {
        while(spin_trylock(lock))
                while(spin_locked(lock));
+       cmb();
 }
 
 }
 
-static inline void spin_unlock(spinlock_t*SAFE lock)
+static inline void __spin_unlock(spinlock_t*SAFE lock)
 {
 {
-       wmb();
+       mb();
        lock->rlock = 0;
 }
 
        lock->rlock = 0;
 }
 
index f724af9..29c09dc 100644 (file)
@@ -63,10 +63,30 @@ extern inline void __spin_lock(spinlock_t *lock);
 extern inline void __spin_unlock(spinlock_t *lock);
 extern inline void spinlock_debug(spinlock_t *lock);
 
 extern inline void __spin_unlock(spinlock_t *lock);
 extern inline void spinlock_debug(spinlock_t *lock);
 
+/* So we can inline a __spin_lock if we want.  Even though we don't need this
+ * if we're debugging, its helpful to keep the include at the same place for
+ * all builds. */
+#include <arch/atomic.h>
+
+#ifdef __CONFIG_SPINLOCK_DEBUG__
 /* Arch indep, in k/s/atomic.c */
 void spin_lock(spinlock_t *lock);
 void spin_unlock(spinlock_t *lock);
 
 /* Arch indep, in k/s/atomic.c */
 void spin_lock(spinlock_t *lock);
 void spin_unlock(spinlock_t *lock);
 
+#else
+/* Just inline the arch-specific __ versions */
+static inline void spin_lock(spinlock_t *lock)
+{
+       __spin_lock(lock);
+}
+
+static inline void spin_unlock(spinlock_t *lock)
+{
+       __spin_unlock(lock);
+}
+
+#endif /* __CONFIG_SPINLOCK_DEBUG__ */
+
 /* Inlines, defined below */
 static inline void spinlock_init(spinlock_t *lock);
 static inline void spinlock_init_irqsave(spinlock_t *lock);
 /* Inlines, defined below */
 static inline void spinlock_init(spinlock_t *lock);
 static inline void spinlock_init_irqsave(spinlock_t *lock);
index 5a3408d..9a49bfc 100644 (file)
@@ -23,10 +23,9 @@ static void decrease_lock_depth(uint32_t coreid)
        per_cpu_info[coreid].lock_depth--;
 }
 
        per_cpu_info[coreid].lock_depth--;
 }
 
-/* TODO: make this inline if we aren't doing DEBUG? */
+#ifdef __CONFIG_SPINLOCK_DEBUG__
 void spin_lock(spinlock_t *lock)
 {
 void spin_lock(spinlock_t *lock)
 {
-#ifdef __CONFIG_SPINLOCK_DEBUG__
        uint32_t coreid = core_id();
        struct per_cpu_info *pcpui = &per_cpu_info[coreid];
        /* TODO: don't print directly.  If we have a lock on the print path that
        uint32_t coreid = core_id();
        struct per_cpu_info *pcpui = &per_cpu_info[coreid];
        /* TODO: don't print directly.  If we have a lock on the print path that
@@ -47,23 +46,17 @@ void spin_lock(spinlock_t *lock)
        lock->calling_core = coreid;
        /* TODO consider merging this with __ctx_depth (unused field) */
        increase_lock_depth(lock->calling_core);
        lock->calling_core = coreid;
        /* TODO consider merging this with __ctx_depth (unused field) */
        increase_lock_depth(lock->calling_core);
-#else
        __spin_lock(lock);
        __spin_lock(lock);
-#endif
-       cmb();  /* need cmb(), the CPU mb() was handled by the arch-specific xchg */
+       /* Memory barriers are handled by the particular arches */
 }
 
 void spin_unlock(spinlock_t *lock)
 {
 }
 
 void spin_unlock(spinlock_t *lock)
 {
-#ifdef __CONFIG_SPINLOCK_DEBUG__
        decrease_lock_depth(lock->calling_core);
        decrease_lock_depth(lock->calling_core);
-#endif
-       /* Need to prevent the compiler (and some arches) from reordering older
-        * stores. */
-       wmb();
-       rwmb(); /* x86 makes both of these a cmb() */
+       /* Memory barriers are handled by the particular arches */
        __spin_unlock(lock);
 }
        __spin_unlock(lock);
 }
+#endif /* __CONFIG_SPINLOCK_DEBUG__ */
 
 /* Inits a hashlock. */
 void hashlock_init(struct hashlock *hl, unsigned int nr_entries)
 
 /* Inits a hashlock. */
 void hashlock_init(struct hashlock *hl, unsigned int nr_entries)