lkml.org 
[lkml]   [1998]   [May]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
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.
\
 
 \ /
  Last update: 2005-03-22 13:42    [from the cache]
©2003-2011 Jasper Spaans. Advertise on this site