Spinlock debugging infrastructure
authorBarret Rhoden <brho@cs.berkeley.edu>
Sat, 24 Oct 2009 22:49:37 +0000 (15:49 -0700)
committerBarret Rhoden <brho@cs.berkeley.edu>
Sat, 24 Oct 2009 22:49:37 +0000 (15:49 -0700)
Changes all locks to be of type spinlock_t, which if SPINLOCK_DEBUG is
defined, will enable some basic debugging for x86.  It's currently
turned on.  Sparc will need to write a couple functions, like
spinlock_debug (even if they don't do anything).

17 files changed:
kern/arch/i386/atomic.h
kern/arch/i386/console.c
kern/arch/i386/cpuinfo.c
kern/arch/i386/smp_boot.c
kern/arch/i386/x86.h
kern/arch/sparc/atomic.h
kern/include/atomic.h
kern/src/atomic.c
kern/src/env.c
kern/src/kmalloc.c
kern/src/printf.c
kern/src/process.c
kern/src/slab.c
kern/src/syscall.c
kern/src/testing.c
user/roslib/inc/atomic.h
user/roslib/src/atomic.c

index ce675e9..41e93ee 100644 (file)
@@ -1,7 +1,12 @@
 #ifndef ROS_INCLUDE_ATOMIC_H
 #define ROS_INCLUDE_ATOMIC_H
 
+/* This enables tracking who last locked a spinlock. */
+#define SPINLOCK_DEBUG
+
 #include <ros/common.h>
+#include <arch/x86.h>
+#include <arch/arch.h>
 
 #define mb() {rmb(); wmb();}
 #define rmb() ({ asm volatile("lfence"); })
 #define wmb_f() ({ asm volatile("sfence"); })
 
 typedef void * RACY atomic_t;
-typedef volatile uint32_t RACY spinlock_t;
+typedef struct spinlock {
+       volatile uint32_t RACY rlock;
+#ifdef SPINLOCK_DEBUG
+       void *call_site;        
+       uint32_t calling_core;
+#endif
+} spinlock_t;
+#define SPINLOCK_INITIALIZER {0}
 
 static inline void atomic_init(atomic_t *number, int32_t val);
 static inline int32_t atomic_read(atomic_t *number);
@@ -19,8 +31,12 @@ static inline void atomic_set(atomic_t *number, int32_t val);
 static inline void atomic_inc(atomic_t *number);
 static inline void atomic_dec(atomic_t *number);
 static inline void atomic_andb(volatile uint8_t RACY* number, uint8_t mask);
-static inline void spin_lock(volatile uint32_t SRACY*COUNT(1) lock);
-static inline void spin_unlock(volatile uint32_t SRACY* lock);
+static inline uint32_t spin_locked(spinlock_t *SAFE lock);
+static inline void __spin_lock(volatile uint32_t SRACY*CT(1) rlock);
+static inline void spin_lock(spinlock_t *lock);
+static inline void spin_unlock(spinlock_t *lock);
+static inline void spinlock_init(spinlock_t *lock);
+void spinlock_debug(spinlock_t *lock);
 
 /* Inlined functions declared above */
 static inline void atomic_init(atomic_t *number, int32_t val)
@@ -56,8 +72,13 @@ static inline void atomic_andb(volatile uint8_t RACY*number, uint8_t mask)
        asm volatile("lock andb %1,%0" : "=m"(*number) : "r"(mask) : "cc");
 }
 
+static inline uint32_t spin_locked(spinlock_t *SAFE lock)
+{
+       // the lock status is the lowest byte of the lock
+       return lock->rlock & 0xff;
+}
 
-static inline void spin_lock(volatile uint32_t* lock)
+static inline void __spin_lock(volatile uint32_t *rlock)
 {
        asm volatile(
                        "1:                       "
@@ -70,12 +91,30 @@ static inline void spin_lock(volatile uint32_t* lock)
                        "       xchgb %%al, %0;       "
                        "       cmpb $0, %%al;        "
                        "       jne 1b;               "
-               : : "m"(*lock) : "eax", "cc");
+               : : "m"(*rlock) : "eax", "cc");
+}
+
+static inline void spin_lock(spinlock_t *lock)
+{
+       __spin_lock(&lock->rlock);
+#ifdef SPINLOCK_DEBUG
+       lock->call_site = (void*)read_eip();
+       lock->calling_core = core_id();
+#endif
+}
+
+static inline void spin_unlock(spinlock_t *lock)
+{
+       lock->rlock = 0;
 }
 
-static inline void spin_unlock(volatile uint32_t* lock)
+static inline void spinlock_init(spinlock_t *lock)
 {
-       *lock = 0;
+       lock->rlock = 0;
+#ifdef SPINLOCK_DEBUG
+       lock->call_site = 0;
+       lock->calling_core = 0;
+#endif
 }
 
 #endif /* !ROS_INCLUDE_ATOMIC_H */
index e02af82..a5df0e6 100644 (file)
@@ -152,7 +152,7 @@ lpt_putc(int c)
 #define MAX_SCROLL_LENGTH      20
 #define SCROLLING_CRT_SIZE     (MAX_SCROLL_LENGTH * CRT_SIZE)
 
-static volatile uint32_t SRACY lock = 0;
+static spinlock_t SRACY lock = SPINLOCK_INITIALIZER;
 
 static unsigned SREADONLY addr_6845;
 static uint16_t *SLOCKED(&lock) COUNT(CRT_SIZE) crt_buf;
index 2d22b90..2a59bde 100644 (file)
@@ -146,7 +146,7 @@ void backtrace(void)
                memset(buf, 0, 256);
                strncpy(buf, debuginfo.eip_fn_name, MIN(debuginfo.eip_fn_namelen, 256));
                buf[MIN(debuginfo.eip_fn_namelen, 255)] = 0;
-               cprintf("#%02d [<%x>] in %s+%x(%p) from %s:%d\n", i++,  eip, buf, 
+               cprintf("#%02d [<%p>] in %s+%x(%p) from %s:%d\n", i++,  eip, buf, 
                        debuginfo.eip_fn_addr - (uint32_t)_start, debuginfo.eip_fn_addr, 
                        debuginfo.eip_file, debuginfo.eip_line);
                cprintf("    ebp: %x   Args:", ebp);
@@ -157,3 +157,21 @@ void backtrace(void)
                ebp = (uint32_t*)(*ebp);
        }
 }
+
+/* Like backtrace, this is probably not the best place for this. */
+void spinlock_debug(spinlock_t *lock)
+{
+#ifdef SPINLOCK_DEBUG
+       eipdebuginfo_t debuginfo;
+       char buf[256];
+       uint32_t eip = (uint32_t)lock->call_site;
+
+       debuginfo_eip(eip, &debuginfo);
+       memset(buf, 0, 256);
+       strncpy(buf, debuginfo.eip_fn_name, MIN(debuginfo.eip_fn_namelen, 256));
+       buf[MIN(debuginfo.eip_fn_namelen, 255)] = 0;
+       printk("Lock %p: last locked at [<%p>] in %s(%p) on core %d\n", lock, eip, buf,
+              debuginfo.eip_fn_addr, lock->calling_core);
+#endif
+}
+
index b3d865c..f07f279 100644 (file)
@@ -133,7 +133,7 @@ void smp_boot(void)
        // booting.  Specifically, it's when they turn on paging and have that temp
        // mapping pulled out from under them.  Now, if a core loses, it will spin
        // on the trampoline (which we must be careful to not deallocate)
-       spin_lock(get_smp_bootlock());
+       __spin_lock(get_smp_bootlock());
        cprintf("Num_Cpus Detected: %d\n", num_cpus);
 
        // Remove the mapping of the page used by the trampoline
index 82a70eb..da0c596 100644 (file)
@@ -59,6 +59,7 @@ static __inline uint32_t rcr4(void) __attribute__((always_inline));
 static __inline uint32_t read_eflags(void) __attribute__((always_inline));
 static __inline void write_eflags(uint32_t eflags) __attribute__((always_inline));
 static __inline uint32_t read_ebp(void) __attribute__((always_inline));
+static __inline uint32_t read_eip(void) __attribute__((always_inline));
 static __inline uint32_t read_esp(void) __attribute__((always_inline));
 static __inline void cpuid(uint32_t info, uint32_t *eaxp, uint32_t *ebxp, uint32_t *ecxp, uint32_t *edxp);
 static __inline uint64_t read_msr(uint32_t reg) __attribute__((always_inline));
@@ -255,6 +256,14 @@ read_ebp(void)
 }
 
 static __inline uint32_t
+read_eip(void)
+{
+        uint32_t eip;
+        asm volatile("call 1f; 1: popl %0" : "=r"(eip));
+        return eip;
+}
+
+static __inline uint32_t
 read_esp(void)
 {
         uint32_t esp;
index 90be16c..63a9263 100644 (file)
@@ -22,6 +22,7 @@ static inline void atomic_add(atomic_t* number, int32_t inc);
 static inline void atomic_inc(atomic_t* number);
 static inline void atomic_dec(atomic_t* number);
 static inline uint32_t spin_trylock(spinlock_t*SAFE lock);
+static inline uint32_t spin_locked(spinlock_t*SAFE lock);
 static inline void spin_lock(spinlock_t*SAFE lock);
 static inline void spin_unlock(spinlock_t*SAFE lock);
 
index 6abc513..4338729 100644 (file)
@@ -7,9 +7,9 @@
 #include <arch/arch.h>
 
 static inline void
-(SLOCK(0) spin_lock_irqsave)(volatile uint32_t SRACY*SAFE lock);
+(SLOCK(0) spin_lock_irqsave)(spinlock_t SRACY*SAFE lock);
 static inline void
-(SUNLOCK(0) spin_unlock_irqsave)(volatile uint32_t SRACY*SAFE lock);
+(SUNLOCK(0) spin_unlock_irqsave)(spinlock_t SRACY*SAFE lock);
 
 /*********************** Checklist stuff **********************/
 typedef struct checklist_mask {
@@ -20,7 +20,7 @@ typedef struct checklist_mask {
 
 // mask contains an unspecified array, so it needs to be at the bottom
 struct checklist {
-       volatile uint32_t lock;
+       spinlock_t lock;
        checklist_mask_t mask;
        // eagle-eyed readers may know why this might have been needed. 2009-09-04
        //volatile uint8_t (COUNT(BYTES_FOR_BITMASK(size)) bits)[];
@@ -30,7 +30,7 @@ typedef struct checklist RACY checklist_t;
 #define ZEROS_ARRAY(size) {[0 ... ((size)-1)] 0}
 
 #define DEFAULT_CHECKLIST_MASK(sz) {(sz), ZEROS_ARRAY(BYTES_FOR_BITMASK(sz))}
-#define DEFAULT_CHECKLIST(sz) {0, DEFAULT_CHECKLIST_MASK(sz)}
+#define DEFAULT_CHECKLIST(sz) {SPINLOCK_INITIALIZER, DEFAULT_CHECKLIST_MASK(sz)}
 #define INIT_CHECKLIST(nm, sz) \
        checklist_t nm = DEFAULT_CHECKLIST(sz);
 #define INIT_CHECKLIST_MASK(nm, sz)    \
@@ -57,7 +57,7 @@ void down_checklist(checklist_t* list);
 
 /* Barrier: currently made for everyone barriering.  Change to use checklist */
 struct barrier {
-       volatile uint32_t lock;
+       spinlock_t lock;
        uint32_t init_count;
        uint32_t current_count;
     volatile uint8_t ready;
@@ -71,25 +71,25 @@ void waiton_barrier(barrier_t* barrier);
 
 // If ints are enabled, disable them and note it in the top bit of the lock
 // There is an assumption about releasing locks in order here...
-static inline void spin_lock_irqsave(volatile uint32_t*SAFE lock)
+static inline void spin_lock_irqsave(spinlock_t *SAFE lock)
 {
        uint32_t irq_en;
        irq_en = irq_is_enabled();
        disable_irq();
        spin_lock(lock);
        if (irq_en)
-               *lock |= 0x80000000;
+               lock->rlock |= 0x80000000;
 }
 
 // if the high bit of the lock is set, then re-enable interrupts
 // (note from asw: you're lucky this works, you little-endian jerks)
-static inline void spin_unlock_irqsave(volatile uint32_t*SAFE lock)
+static inline void spin_unlock_irqsave(spinlock_t *SAFE lock)
 {
-       if (*lock & 0x80000000) {
-               *lock = 0;
+       if (lock->rlock & 0x80000000) {
+               spin_unlock(lock);
                enable_irq();
        } else
-               *lock = 0;
+               spin_unlock(lock);
 }
 
 #endif /* !ROS_KERN_ATOMIC_H */
index 09a0d86..729b7f3 100644 (file)
@@ -90,8 +90,7 @@ int release_checklist(checklist_t* list)
 // peaks in and sees if the list is locked with it's spinlock
 int checklist_is_locked(checklist_t* list)
 {
-       // remember the lock status is the lowest byte of the lock
-       return list->lock & 0xff;
+       return spin_locked(&list->lock);
 }
 
 // no synch guarantees - just looks at the list
@@ -115,7 +114,7 @@ void down_checklist(checklist_t* list)
 /* Barriers */
 void init_barrier(barrier_t* barrier, uint32_t count)
 {
-       barrier->lock = 0;
+       spinlock_init(&barrier->lock);
        barrier->init_count = count;
        barrier->current_count = count;
        barrier->ready = 0;
index ec270a6..5572477 100644 (file)
@@ -272,7 +272,7 @@ env_alloc(env_t **newenv_store, envid_t parent_id)
        e->env_id = generation | (e - envs);
 
        // Set the basic status variables.
-    e->proc_lock = 0;
+    spinlock_init(&e->proc_lock);
        e->env_parent_id = parent_id;
        proc_set_state(e, PROC_CREATED);
        e->env_runs = 0;
index 17fbc94..9ff4592 100644 (file)
@@ -23,7 +23,7 @@
 char *RO BND(end, maxaddrpa_ptr + IVY_KERNBASE) boot_freemem;
 
 //List of physical pages used by kmalloc
-static spinlock_t pages_list_lock = 0;
+static spinlock_t pages_list_lock = SPINLOCK_INITIALIZER;
 static page_list_t LCKD(&pages_list_lock)pages_list;
 
 /*
index 2bb2bb3..86d104b 100644 (file)
@@ -12,7 +12,7 @@
 #include <stdio.h>
 #include <stdarg.h>
 
-spinlock_t output_lock = 0;
+spinlock_t output_lock = SPINLOCK_INITIALIZER;
 
 void putch(int ch, int **cnt)
 {
index 0e55bbc..6fc2633 100644 (file)
 
 /* Process Lists */
 struct proc_list proc_freelist = TAILQ_HEAD_INITIALIZER(proc_freelist);
-spinlock_t freelist_lock = 0;
+spinlock_t freelist_lock = SPINLOCK_INITIALIZER;
 struct proc_list proc_runnablelist = TAILQ_HEAD_INITIALIZER(proc_runnablelist);
-spinlock_t runnablelist_lock = 0;
+spinlock_t runnablelist_lock = SPINLOCK_INITIALIZER;
 
 /* Tracks which cores are idle, similar to the vcoremap.  Each value is the
  * physical coreid of an unallocated core. */
-spinlock_t idle_lock = 0;
+spinlock_t idle_lock = SPINLOCK_INITIALIZER;
 uint32_t LCKD(&idle_lock) (RO idlecoremap)[MAX_NUM_CPUS];
 uint32_t LCKD(&idle_lock) num_idlecores = 0;
 
@@ -780,6 +780,8 @@ void print_proc_info(pid_t pid)
                return;
        }
        spin_lock_irqsave(&p->proc_lock);
+       spinlock_debug(&p->proc_lock);
+       printk("struct proc: %p\n", p);
        printk("PID: %d\n", p->env_id);
        printk("PPID: %d\n", p->env_parent_id);
        printk("State: 0x%08x\n", p->state);
@@ -802,4 +804,3 @@ void print_proc_info(pid_t pid)
        print_trapframe(&p->env_tf);
        spin_unlock_irqsave(&p->proc_lock);
 }
-
index e6d1ab4..92c6334 100644 (file)
@@ -30,7 +30,7 @@ static void __kmem_cache_create(struct kmem_cache *kc, const char *name,
 {
        assert(kc);
        assert(align);
-       kc->cache_lock = 0;
+       spinlock_init(&kc->cache_lock);
        kc->name = name;
        kc->obj_size = obj_size;
        kc->align = align;
@@ -60,7 +60,7 @@ static void __kmem_cache_create(struct kmem_cache *kc, const char *name,
 
 void kmem_cache_init(void)
 {
-       kmem_caches_lock = 0;
+       spinlock_init(&kmem_caches_lock);
        SLIST_INIT(&kmem_caches);
        /* We need to call the __ version directly to bootstrap the global
         * kmem_cache_cache. */
index 56a7058..8f0b38e 100644 (file)
@@ -233,7 +233,7 @@ static void sys_cache_buster(env_t* e, uint32_t num_writes, uint32_t num_pages,
        #define MAX_PAGES               32
        #define INSERT_ADDR     (UINFO + 2*PGSIZE) // should be free for these tests
        uint32_t* buster = (uint32_t*)BUSTER_ADDR;
-       static uint32_t buster_lock = 0;
+       static spinlock_t buster_lock = SPINLOCK_INITIALIZER;
        uint64_t ticks = -1;
        page_t* a_page[MAX_PAGES];
 
index 916cd04..000f6ce 100644 (file)
@@ -664,7 +664,7 @@ void test_hello_world_handler(trapframe_t *tf, void* data)
                trapno, core_id(), tf);
 }
 
-uint32_t print_info_lock = 0;
+spinlock_t print_info_lock = SPINLOCK_INITIALIZER;
 
 void test_print_info_handler(trapframe_t *tf, void* data)
 {
index a9cf2d6..a8b6355 100644 (file)
@@ -4,7 +4,7 @@
 #include <arch/atomic.h>
 
 typedef struct barrier {
-       volatile uint32_t lock;
+       spinlock_t lock;
        uint32_t init_count;
        uint32_t current_count;
     volatile uint8_t ready;
index daa95c2..a6484ab 100644 (file)
@@ -4,7 +4,7 @@
 
 void init_barrier(barrier_t* barrier, uint32_t count)
 {
-       barrier->lock = 0;
+       spinlock_init(&barrier->lock);
        barrier->init_count = count;
        barrier->current_count = count;
        barrier->ready = 0;