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--M9NhX3UHpAaciwkO Content-Type: text/plain; charset=us-ascii Content-Description: spinlock-debug.patch Content-Disposition: attachment; filename="spinlock-debug.patch"
--- 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.
--M9NhX3UHpAaciwkO--
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.rutgers.edu