From: Peter Dinda <pdinda@northwestern.edu>
Date: Sun, 26 May 2013 22:36:24 +0000 (-0500)
Subject: Enhancements to lock checking and its configurability
X-Git-Url: http://v3vee.org/palacios/gitweb/gitweb.cgi?a=commitdiff_plain;h=49852ca1e8280641ffb7ef2986c03bd443bd02b8;p=palacios.releases.git

Enhancements to lock checking and its configurability
---

diff --git a/Kconfig b/Kconfig
index b2479b3..4f4a3d5 100644
--- a/Kconfig
+++ b/Kconfig
@@ -430,6 +430,14 @@ config DEBUG_MEM_ERRORS
     help
       This turns on debugging for memory translations and lookups
 
+config DEBUG_LOCKS
+    bool "Lock debugging (if host supports it)"
+    default n
+    depends on DEBUG_ON
+    help
+      This turns on lock debugging for locks in Palacios and in its host-specifc glue code.  This requires host support.
+
+
 endmenu
 
 
diff --git a/linux_module/Makefile b/linux_module/Makefile
index a5b541a..64ded27 100644
--- a/linux_module/Makefile
+++ b/linux_module/Makefile
@@ -20,8 +20,7 @@ v3vee-y := 	palacios-stubs.o \
 		allow_devmem.o \
 		util-queue.o \
 	        util-hashtable.o \
-		linux-exts.o \
-                lockcheck.o
+		linux-exts.o 
 
 v3vee-$(V3_CONFIG_CONSOLE) += iface-console.o
 v3vee-$(V3_CONFIG_FILE) += iface-file.o
@@ -50,6 +49,9 @@ v3vee-$(V3_CONFIG_EXT_GUARD_MODULES) += iface-guard-mods.o
 
 v3vee-$(V3_CONFIG_HOST_PCI) += iface-host-pci.o
 
+v3vee-$(V3_CONFIG_DEBUG_LOCKS) += lockcheck.o
+
+
 v3vee-objs := $(v3vee-y) ../libv3vee.a
 obj-m := v3vee.o
 
diff --git a/linux_module/lockcheck.c b/linux_module/lockcheck.c
index 1c55e0e..bfa0566 100644
--- a/linux_module/lockcheck.c
+++ b/linux_module/lockcheck.c
@@ -39,10 +39,22 @@ typedef struct {
 } lockcheck_state_t;
 
 
-static spinlock_t lock;
-
+// This lock is currently used only to control
+// allocation of entries in the global state
+static spinlock_t lock; 
 static lockcheck_state_t state[NUM_LOCKS];
 
+static void printlock(char *prefix, lockcheck_state_t *l);
+
+
+typedef struct {
+  u32 top;               // next open slot 0..
+  void *lock[LOCK_STACK_DEPTH]; // the stack
+  char irq[LOCK_STACK_DEPTH];   // locked with irqsave?
+} lock_stack_t;
+
+static DEFINE_PER_CPU(lock_stack_t, lock_stack);
+
 static lockcheck_state_t *get_lock_entry(void)
 {
   int i;
@@ -90,6 +102,68 @@ static void free_lock_entry(lockcheck_state_t *l)
 }
 
 
+static void lock_stack_print(void)
+{
+  u32 i;
+  char buf[64];
+  lock_stack_t *mystack = &(get_cpu_var(lock_stack));
+  u32 cpu = get_cpu();  put_cpu();
+  
+  if ((mystack->top)>0) { 
+    for (i=mystack->top; i>0;i--) {
+      snprintf(buf,64,"LOCK STACK (cpu=%u, index=%u, irq=%d)",cpu, i-1, (int)(mystack->irq[i-1]));
+      printlock(buf,find_lock_entry(mystack->lock[i-1]));
+    }
+  }
+  put_cpu_var(lock_stack);
+}
+
+
+static void lock_stack_lock(void *lock, char irq)
+{
+  lock_stack_t *mystack = &(get_cpu_var(lock_stack));
+  u32 cpu = get_cpu();  put_cpu();
+
+  if (mystack->top>=(LOCK_STACK_DEPTH-1)) {
+    put_cpu_var(lock_stack);
+    DEBUG("LOCKCHECK: Locking lock 0x%p on cpu %u exceeds stack limit of %d\n",lock,cpu,LOCK_STACK_DEPTH);
+    lock_stack_print();
+  } else {
+    mystack->lock[mystack->top] = lock;
+    mystack->irq[mystack->top] = irq;
+    mystack->top++;
+    put_cpu_var(lock_stack);
+  }
+}
+
+static void lock_stack_unlock(void *lock, char irq)
+{
+  lock_stack_t *mystack = &(get_cpu_var(lock_stack));
+  u32 cpu = get_cpu(); put_cpu();
+
+  if (mystack->top==0) {
+    put_cpu_var(lock_stack);
+    DEBUG("LOCKCHECK: Unlocking lock 0x%p on cpu %u when lock stack is empty\n",lock,cpu);
+  } else {
+    if (mystack->lock[mystack->top-1] != lock) { 
+      void *otherlock=mystack->lock[mystack->top-1];
+      put_cpu_var(lock_stack);
+      DEBUG("LOCKCHECK: Unlocking lock 0x%p on cpu %u when top of stack is lock 0x%p\n",lock,cpu, otherlock);
+      lock_stack_print();
+    } else {
+      if (irq!=mystack->irq[mystack->top-1]) {
+	char otherirq = mystack->irq[mystack->top-1];
+	put_cpu_var(lock_stack);
+	DEBUG("LOCKCHECK: Unlocking lock 0x%p on cpu %u with irq=%d, but was locked with irq=%d\n",lock,cpu,irq,otherirq);
+	lock_stack_print();
+      } else {
+	mystack->top--;
+	put_cpu_var(lock_stack);
+      }
+    }
+  }
+
+}
 
 void palacios_lockcheck_init()
 {
@@ -127,6 +201,10 @@ static void clear_trace(void **trace)
 
 static void printlock(char *prefix, lockcheck_state_t *l)
 {
+  if (!l || !(l->lock) ) { 
+    DEBUG("LOCKCHECK: %s: lock 0x%p BOGUS\n",prefix,l);
+    return;
+  }
   if (l->lock) { 
     DEBUG("LOCKCHECK: %s: lock 0x%p, allocator=" 
 	  backtrace_format
@@ -297,6 +375,8 @@ void palacios_lockcheck_lock(void *lock)
 
   find_multiple_locks_held();
 
+  lock_stack_lock(lock,0);
+
 }
 void palacios_lockcheck_unlock(void *lock)
 {
@@ -313,9 +393,12 @@ void palacios_lockcheck_unlock(void *lock)
   if (l->irqcount!=0) { 
     printlock("LOCKCHECK: BAD IRQCOUNT AT UNLOCK",l);
   }
+
+  lock_stack_unlock(lock,0);
   
   l->lockcount--;
   backtrace(l->lastunlocker);
+
 }
 
 void palacios_lockcheck_lock_irqsave(void *lock,unsigned long flags)
@@ -341,6 +424,9 @@ void palacios_lockcheck_lock_irqsave(void *lock,unsigned long flags)
 
   find_multiple_irqs_held();
 
+  lock_stack_lock(lock,1);
+
+
 }
 
 void palacios_lockcheck_unlock_irqrestore(void *lock,unsigned long flags)
@@ -361,6 +447,9 @@ void palacios_lockcheck_unlock_irqrestore(void *lock,unsigned long flags)
   
   l->irqcount--;
   l->lastunlockflags = flags;
+
+  lock_stack_unlock(lock,1);
+
   backtrace(l->lastirqunlocker);
   
 }
diff --git a/linux_module/lockcheck.h b/linux_module/lockcheck.h
index 7151bd0..2fd2e92 100644
--- a/linux_module/lockcheck.h
+++ b/linux_module/lockcheck.h
@@ -1,10 +1,21 @@
 #ifndef _lockcheck
 #define _lockcheck
 
-#define CHECK_LOCKS  0
-#define NUM_LOCKS    1024
 
-#if CHECK_LOCKS
+#ifdef V3_CONFIG_DEBUG_LOCKS
+
+// Maxmimum number of locks to handle
+#define NUM_LOCKS        1024
+// Maximum number of locks that can be simultaneously
+// held on each CPU
+#define LOCK_STACK_DEPTH 16
+
+//
+// The following macros are used
+// in the stub functions to call back to the lock
+// checker - if lock checking is not enabled, these 
+// turn into nothing
+//
 #define LOCKCHECK_INIT() palacios_lockcheck_init()
 #define LOCKCHECK_ALLOC(lock) palacios_lockcheck_alloc(lock)
 #define LOCKCHECK_FREE(lock)  palacios_lockcheck_free(lock)
@@ -13,7 +24,21 @@
 #define LOCKCHECK_LOCK_IRQSAVE(lock, flags)  palacios_lockcheck_lock_irqsave(lock,flags)
 #define LOCKCHECK_UNLOCK_IRQRESTORE(lock, flags) palacios_lockcheck_unlock_irqrestore(lock,flags)
 #define LOCKCHECK_DEINIT() palacios_lockcheck_deinit()
+
+void palacios_lockcheck_init(void);
+void palacios_lockcheck_alloc(void *lock);
+void palacios_lockcheck_free(void *lock);
+void palacios_lockcheck_lock(void *lock);
+void palacios_lockcheck_unlock(void *lock);
+void palacios_lockcheck_lock_irqsave(void *lock,unsigned long flags);
+void palacios_lockcheck_unlock_irqrestore(void *lock,unsigned long flags);
+void palacios_lockcheck_deinit(void);
+
 #else
+
+//
+// The following is what happens when lock checking is not on
+//
 #define LOCKCHECK_INIT()
 #define LOCKCHECK_ALLOC(lock) 
 #define LOCKCHECK_FREE(lock)  
@@ -22,15 +47,8 @@
 #define LOCKCHECK_LOCK_IRQSAVE(lock, flags)  
 #define LOCKCHECK_UNLOCK_IRQRESTORE(lock, flags) 
 #define LOCKCHECK_DEINIT()
+
 #endif
 
-void palacios_lockcheck_init(void);
-void palacios_lockcheck_alloc(void *lock);
-void palacios_lockcheck_free(void *lock);
-void palacios_lockcheck_lock(void *lock);
-void palacios_lockcheck_unlock(void *lock);
-void palacios_lockcheck_lock_irqsave(void *lock,unsigned long flags);
-void palacios_lockcheck_unlock_irqrestore(void *lock,unsigned long flags);
-void palacios_lockcheck_deinit(void);
 
 #endif