Messages in this thread Patch in this message |  | | | Date | Tue, 19 May 1998 01:53:48 +0200 | | From | Kurt Garloff <> | | Subject | [PATCH] Debug spinlocks |
| |
Hi Linus,
when trying to test my SCSI driver's spinlocks, I found several problems with the spinlock.h DEBUG_SPINLOCKS 2 code. The kernel won't compile with DEBUG_SPINLOCKS set to 2 in spinlock.h and only setting it for my SCSI driver is fatal: The kernel will crash on the unitialized io_request_lock struct ... The DEBUG_SPINLOCKS 2 spin_unlock_irq was totally bogus: It sets the lock instead of freeing it.
So I made a few changes: * changed spin_lock_init to initialize the lock with SPIN_LOCK_UNLOCKED, not just 0; fixed spin_unlock_irq * Added comment in spinlock.h * added the display of the instruction pointer (eip) on detected deadlocks * changed a few #ifdef __SMP__ to #if defined(__SMP__) || DEBUG_SPINLOCKS > 0 * moved the semaphore_wake_lock from smp.c to sys_i386.c (don't know, if this is the correct place) With these changes (patch appended), I was able to compile and debug my spinlocks. One problem remains, though: sched.c:80111e71: spin_lock(sched.c:801ce964) already locked is displayed a number of times, and I didn't spend enough time to track this one down.
(Reading the adresses, you can see, that I have a 2GB kernel ...)
Please add it to 2.1.103. --- Linus, shouldn't we protect the ___strtok variable by a lock? Otherwise parsing of options within the kernel could give bogus results. Well, yes, I could look for all strtok() and do it; but of course I won't do this rather annoying job to hear that it's not necessary and won't go into the kernel anyway afterwards.
-- Kurt Garloff, Dortmund <K.Garloff@ping.de> PGP key on http://student.physik.uni-dortmund.de/homepages/garloff --- linux/include/asm-i386/spinlock.h.old Fri May 8 11:29:57 1998 +++ linux/include/asm-i386/spinlock.h Tue May 19 01:31:36 1998 @@ -3,7 +3,16 @@ #ifndef __SMP__ -#define DEBUG_SPINLOCKS 0 /* 0 == no debugging, 1 == maintain lock state, 2 == full debug */ +/* DEBUG_SPINLOCKS: Define here for global debugging of spinlocks. * + * If you only want to debug a single module's spinlocks, you can * + * do this by defining DEBUG_SPINLOCKS there, if and only if you * + * don't use any of the other kernel's spinlocks there. Note that * + * you will see kernel dumps on trying to (un)lock a spinlock with * + * DEBUG_SPINLOCKS 2, that was initialized with DEBUG_SPINLOCKS 0 * + */ +#ifndef DEBUG_SPINLOCKS +# define DEBUG_SPINLOCKS 0 /* 0 == no debugging, 1 == maintain lock state, 2 == full debug */ +#endif #if (DEBUG_SPINLOCKS < 1) @@ -33,7 +42,7 @@ } spinlock_t; #define SPIN_LOCK_UNLOCKED { 0 } -#define spin_lock_init(x) do { (x)->lock = 0; } while (0) +#define spin_lock_init(x) { spinlock_t __unlocked = SPIN_LOCK_UNLOCKED; *x = __unlocked; } #define spin_trylock(lock) (!test_and_set_bit(0,(lock))) #define spin_lock(x) do { (x)->lock = 1; } while (0) @@ -54,21 +63,30 @@ volatile unsigned int babble; const char *module; } spinlock_t; -#define SPIN_LOCK_UNLOCKED { 0, 25, __BASE_FILE__ } + +#define GETEIP ({ unsigned int __eip; __asm__ __volatile__ ( \ + "\t" \ + " call 0f \n\t" \ + "0: \t" \ + " popl %0 \n" \ + : "=m" (__eip) ); \ + __eip; }) + +#define SPIN_LOCK_UNLOCKED { 0, 64, __BASE_FILE__ } #include <linux/kernel.h> -#define spin_lock_init(x) do { (x)->lock = 0; } while (0) +#define spin_lock_init(x) { spinlock_t __unlocked = SPIN_LOCK_UNLOCKED; *x = __unlocked; } #define spin_trylock(lock) (!test_and_set_bit(0,(lock))) -#define spin_lock(x) do {unsigned long __spinflags; save_flags(__spinflags); cli(); if ((x)->lock&&(x)->babble) {printk("%s: spin_lock(%s:%p) already locked\n", __BASE_FILE__, (x)->module, (x));(x)->babble--;} (x)->lock = 1; restore_flags(__spinflags);} while (0) -#define spin_unlock_wait(x) do {unsigned long __spinflags; save_flags(__spinflags); cli(); if ((x)->lock&&(x)->babble) {printk("%s: spin_unlock_wait(%s:%p) deadlock\n", __BASE_FILE__, (x)->module, (x));(x)->babble--;} restore_flags(__spinflags);} while (0) -#define spin_unlock(x) do {unsigned long __spinflags; save_flags(__spinflags); cli(); if (!(x)->lock&&(x)->babble) {printk("%s: spin_unlock(%s:%p) not locked\n", __BASE_FILE__, (x)->module, (x));(x)->babble--;} (x)->lock = 0; restore_flags(__spinflags);} while (0) -#define spin_lock_irq(x) do {cli(); if ((x)->lock&&(x)->babble) {printk("%s: spin_lock_irq(%s:%p) already locked\n", __BASE_FILE__, (x)->module, (x));(x)->babble--;} (x)->lock = 1;} while (0) -#define spin_unlock_irq(x) do {cli(); if ((x)->lock&&(x)->babble) {printk("%s: spin_lock(%s:%p) already locked\n", __BASE_FILE__, (x)->module, (x));(x)->babble--;} (x)->lock = 1; sti();} while (0) +#define spin_lock(x) do {unsigned long __spinflags; save_flags(__spinflags); cli(); if ((x)->lock&&(x)->babble) {printk("%s:%08x: spin_lock(%s:%p) already locked\n", __BASE_FILE__, GETEIP, (x)->module, (x));(x)->babble--;} (x)->lock = 1; restore_flags(__spinflags);} while (0) +#define spin_unlock_wait(x) do {unsigned long __spinflags; save_flags(__spinflags); cli(); if ((x)->lock&&(x)->babble) {printk("%s:%08x: spin_unlock_wait(%s:%p) deadlock\n", __BASE_FILE__, GETEIP, (x)->module, (x));(x)->babble--;} restore_flags(__spinflags);} while (0) +#define spin_unlock(x) do {unsigned long __spinflags; save_flags(__spinflags); cli(); if (!(x)->lock&&(x)->babble) {printk("%s:%08x: spin_unlock(%s:%p) not locked\n", __BASE_FILE__, GETEIP, (x)->module, (x));(x)->babble--;} (x)->lock = 0; restore_flags(__spinflags);} while (0) +#define spin_lock_irq(x) do {cli(); if ((x)->lock&&(x)->babble) {printk("%s:%08x: spin_lock_irq(%s:%p) already locked\n", __BASE_FILE__, GETEIP, (x)->module, (x));(x)->babble--;} (x)->lock = 1;} while (0) +#define spin_unlock_irq(x) do {cli(); if (!(x)->lock&&(x)->babble) {printk("%s:%08x: spin_unlock_irq(%s:%p) not locked\n", __BASE_FILE__, GETEIP, (x)->module, (x));(x)->babble--;} (x)->lock = 0; sti();} while (0) -#define spin_lock_irqsave(x,flags) do {save_flags(flags); cli(); if ((x)->lock&&(x)->babble) {printk("%s: spin_lock_irqsave(%s:%p) already locked\n", __BASE_FILE__, (x)->module, (x));(x)->babble--;} (x)->lock = 1;} while (0) -#define spin_unlock_irqrestore(x,flags) do {cli(); if (!(x)->lock&&(x)->babble) {printk("%s: spin_unlock_irqrestore(%s:%p) not locked\n", __BASE_FILE__, (x)->module, (x));(x)->babble--;} (x)->lock = 0; restore_flags(flags);} while (0) +#define spin_lock_irqsave(x,flags) do {save_flags(flags); cli(); if ((x)->lock&&(x)->babble) {printk("%s:%08x: spin_lock_irqsave(%s:%p) already locked\n", __BASE_FILE__, GETEIP, (x)->module, (x));(x)->babble--;} (x)->lock = 1;} while (0) +#define spin_unlock_irqrestore(x,flags) do {cli(); if (!(x)->lock&&(x)->babble) {printk("%s:%08x: spin_unlock_irqrestore(%s:%p) not locked\n", __BASE_FILE__, GETEIP, (x)->module, (x));(x)->babble--;} (x)->lock = 0; restore_flags(flags);} while (0) #endif /* DEBUG_SPINLOCKS */ @@ -115,7 +133,7 @@ #define SPIN_LOCK_UNLOCKED { 0 } -#define spin_lock_init(x) do { (x)->lock = 0; } while(0) +#define spin_lock_init(x) { spinlock_t __unlocked = SPIN_LOCK_UNLOCKED ; x = __unlocked; } /* * Simple spin lock operations. There are two variants, one clears IRQ's * on the local processor, one does not. --- linux/kernel/capability.c~ Sun May 10 11:24:00 1998 +++ linux/kernel/capability.c Mon May 18 10:49:38 1998 @@ -25,7 +25,7 @@ copy_to_user(u, k, sizeof(*k)); } -#ifdef __SMP__ +#if defined(__SMP__) || DEBUG_SPINLOCKS > 0 static spinlock_t task_capability_lock; #endif --- linux/kernel/ksyms.c~ Sun May 10 11:24:00 1998 +++ linux/kernel/ksyms.c Mon May 18 15:35:55 1998 @@ -285,7 +285,7 @@ EXPORT_SYMBOL(timer_active); EXPORT_SYMBOL(timer_table); -#ifdef __SMP__ +#if defined (__SMP__) || DEBUG_SPINLOCKS > 0 /* Various random spinlocks we want to export */ EXPORT_SYMBOL(tqueue_lock); EXPORT_SYMBOL(waitqueue_lock); --- linux/arch/i386/kernel/smp.c~ Mon May 18 09:16:47 1998 +++ linux/arch/i386/kernel/smp.c Mon May 18 11:11:20 1998 @@ -57,7 +57,7 @@ #include "irq.h" -spinlock_t semaphore_wake_lock = SPIN_LOCK_UNLOCKED; +extern spinlock_t semaphore_wake_lock = SPIN_LOCK_UNLOCKED; extern unsigned long start_kernel, _etext; extern void update_one_process( struct task_struct *p, --- linux/arch/i386/kernel/sys_i386.c~ Sat Mar 7 12:01:31 1998 +++ linux/arch/i386/kernel/sys_i386.c Mon May 18 11:12:22 1998 @@ -22,6 +22,8 @@ #include <asm/uaccess.h> #include <asm/ipc.h> +spinlock_t semaphore_wake_lock = SPIN_LOCK_UNLOCKED; + /* * sys_pipe() is the normal C calling standard for creating * a pipe. It's not the way unix traditionally does this, though. |  |