Spinlock depth checking
authorBarret Rhoden <brho@cs.berkeley.edu>
Fri, 4 May 2012 22:38:03 +0000 (15:38 -0700)
committerBarret Rhoden <brho@cs.berkeley.edu>
Wed, 5 Sep 2012 21:43:57 +0000 (14:43 -0700)
This tracks how many locks we acquired on a pcpu basis.  Before we
kthread/block or anything like that, the number better be 0.  Not only
is it bad for perf, but you could deadlock.

Needed this to hunt down a deadlock bug (asserted in some place where I
thought I had no locks), though it had nothing to do with sleeping.

It only does work when CONFIG_SPINLOCK_DEBUG is on, though you can
always test for it (and you'll get 0).

I didn't compile it for RISCV.  YMMV.

kern/arch/i686/atomic.h
kern/arch/riscv/atomic.h
kern/include/atomic.h
kern/include/smp.h
kern/src/atomic.c
kern/src/kthread.c
kern/src/smp.c

index db50465..d181d91 100644 (file)
@@ -166,12 +166,16 @@ static inline void spin_lock(spinlock_t *lock)
 #ifdef __CONFIG_SPINLOCK_DEBUG__
        lock->call_site = (void RACY*CT(1))TC(read_eip());
        lock->calling_core = core_id();
+       increase_lock_depth(lock->calling_core);
 #endif
        cmb();  /* need cmb(), the CPU mb() was handled by the xchgb */
 }
 
 static inline void spin_unlock(spinlock_t *lock)
 {
+#ifdef __CONFIG_SPINLOCK_DEBUG__
+       decrease_lock_depth(lock->calling_core);
+#endif
        /* Need to prevent the compiler (and some arches) from reordering older
         * stores. */
        wmb();
index fd94e9e..0741afa 100644 (file)
@@ -117,10 +117,16 @@ static inline void spin_lock(spinlock_t *lock)
        while(spin_trylock(lock))
                while(lock->rlock);
        mb();
+#ifdef __CONFIG_SPINLOCK_DEBUG__
+       increase_lock_depth(core_id());
+#endif
 }
 
 static inline void spin_unlock(spinlock_t *lock)
 {
+#ifdef __CONFIG_SPINLOCK_DEBUG__
+       decrease_lock_depth(core_id());
+#endif
        mb();
        lock->rlock = 0;
 }
index 279daa2..a25ccaf 100644 (file)
@@ -59,6 +59,11 @@ static inline void spin_lock_irqsave(spinlock_t *lock);
 static inline void spin_unlock_irqsave(spinlock_t *lock);
 static inline bool spin_lock_irq_enabled(spinlock_t *lock);
 
+/* Used for spinlock debugging.  When we move spinlocks into .c, we can move
+ * these too */
+void increase_lock_depth(uint32_t coreid);
+void decrease_lock_depth(uint32_t coreid);
+
 /* Hash locks (array of spinlocks).  Most all users will want the default one,
  * so point your pointer to one of them, though you could always kmalloc a
  * bigger one.  In the future, they might be growable, etc, which init code may
index 0d9741c..4b444f4 100644 (file)
@@ -34,6 +34,7 @@ struct per_cpu_info {
        struct syscall *cur_sysc;       /* ptr is into cur_proc's address space */
        struct kthread *spare;          /* useful when restarting */
        struct timer_chain tchain;      /* for the per-core alarm */
+       unsigned int lock_depth;
 
 #ifdef __SHARC__
        // held spin-locks. this will have to go elsewhere if multiple kernel
index d7fcaf0..e8e1147 100644 (file)
 #include <string.h>
 #include <assert.h>
 #include <hashtable.h>
+#include <smp.h>
+
+void increase_lock_depth(uint32_t coreid)
+{
+       per_cpu_info[coreid].lock_depth++;
+}
+
+void decrease_lock_depth(uint32_t coreid)
+{
+       per_cpu_info[coreid].lock_depth--;
+}
 
 /* Inits a hashlock. */
 void hashlock_init(struct hashlock *hl, unsigned int nr_entries)
index 29020e4..ac7425d 100644 (file)
@@ -34,6 +34,8 @@ void sleep_on(struct semaphore *sem)
 
        /* interrups would be messy here */
        disable_irqsave(&irq_state);
+       /* Make sure we aren't holding any locks (only works if SPINLOCK_DEBUG) */
+       assert(!pcpui->lock_depth);
        /* Try to down the semaphore.  If there is a signal there, we can skip all
         * of the sleep prep and just return. */
        spin_lock(&sem->lock);  /* no need for irqsave, since we disabled ints */
index e500739..96412f0 100644 (file)
@@ -97,11 +97,9 @@ void smp_percpu_init(void)
        STAILQ_INIT(&per_cpu_info[coreid].routine_amsgs);
        /* Initialize the per-core timer chain */
        init_timer_chain(&per_cpu_info[coreid].tchain, set_pcpu_alarm_interrupt);
-
 #ifdef __CONFIG_KTHREAD_POISON__
-/* TODO: KTHR-STACK */
-uintptr_t *poison = (uintptr_t*)ROUNDDOWN(get_stack_top() - 1, PGSIZE);
-*poison = 0xdeadbeef;
+       /* TODO: KTHR-STACK */
+       uintptr_t *poison = (uintptr_t*)ROUNDDOWN(get_stack_top() - 1, PGSIZE);
+       *poison = 0xdeadbeef;
 #endif /* __CONFIG_KTHREAD_POISON__ */
-
 }