interrupt checks for spinlocks

William Lee Irwin III (wli@holomorphy.com)
Sun, 3 Nov 2002 14:08:16 -0800


I recently thought about interrupt disablement and locking again, or
at least such as has gone about in various places, and I got scared.

Hence, a wee addition to CONFIG_DEBUG_SPINLOCK:

(1) check that spinlocks are not taken in interrupt context without
interrupts disabled
(2) taint spinlocks taken in interrupt context
(3) check for tainted spinlocks taken without interrupts disabled
(4) check for spinlocking calls unconditionally disabling (and
hence later re-enabling) interrupts with interrupts disabled

The only action taken is printk() and dump_stack(). No arch code has
been futzed with to provide irq tainting yet. Looks like a good way
to shake out lurking bugs to me (somewhat like may_sleep() etc.).

vs. 2.5.x-bk as of 2PM PST 3 Nov

Bill

diff -urN linux-virgin/include/linux/spinlock.h linux-wli/include/linux/spinlock.h
--- linux-virgin/include/linux/spinlock.h Sun Aug 25 10:25:45 2002
+++ linux-wli/include/linux/spinlock.h Sun Nov 3 13:58:47 2002
@@ -38,6 +38,48 @@
#include <asm/spinlock.h>

/*
+ * if a lock is ever taken in interrupt context, it must always be
+ * taken with interrupts disabled. If a locking call is made that
+ * unconditionally disables and then re-enables interrupts, it must
+ * be made with interrupts enabled.
+ */
+#ifndef irq_tainted_lock
+#define irq_tainted_lock(lock) 0
+#endif
+
+#ifndef irq_taint_lock
+#define irq_taint_lock(lock) do { } while (0)
+#define
+
+#ifndef CONFIG_DEBUG_SPINLOCK
+#define check_spinlock_irq(lock) do { } while (0)
+#define check_spinlock_irqs_disabled(lock) do { } while (0)
+#else
+#define check_spinlock_irq(lock) \
+ do { \
+ if (irqs_disabled()) { \
+ printk("spinlock taken unconditionally " \
+ "re-enabling interrupts\n"); \
+ dump_stack(); \
+ } \
+ } while (0)
+#define check_spinlock_irqs_disabled(lock) \
+ do { \
+ if (in_interrupt() && !irqs_disabled()) { \
+ printk("spinlock taken in interrupt context " \
+ "without disabling interrupts\n"); \
+ dump_stack(); \
+ } else if (in_interrupt()) \
+ irq_taint_lock(lock); \
+ else if (irq_tainted_lock(lock)) { \
+ printk("spinlock taken in process context " \
+ "without disabling interrupts\n"); \
+ dump_stack(); \
+ } \
+ } while (0)
+#endif
+
+/*
* !CONFIG_SMP and spin_lock_init not previously defined
* (e.g. by including include/asm/spinlock.h)
*/
@@ -87,6 +129,7 @@
*/
#define spin_lock(lock) \
do { \
+ check_spinlock_irqs_disabled(lock); \
preempt_disable(); \
_raw_spin_lock(lock); \
} while(0)
@@ -102,6 +145,7 @@

#define read_lock(lock) \
do { \
+ check_spinlock_irqs_disabled(lock); \
preempt_disable(); \
_raw_read_lock(lock); \
} while(0)
@@ -114,6 +158,7 @@

#define write_lock(lock) \
do { \
+ check_spinlock_irqs_disabled(lock); \
preempt_disable(); \
_raw_write_lock(lock); \
} while(0)
@@ -136,6 +181,7 @@

#define spin_lock_irq(lock) \
do { \
+ check_spinlock_irq(lock); \
local_irq_disable(); \
preempt_disable(); \
_raw_spin_lock(lock); \
@@ -157,6 +203,7 @@

#define read_lock_irq(lock) \
do { \
+ check_spinlock_irq(lock); \
local_irq_disable(); \
preempt_disable(); \
_raw_read_lock(lock); \
@@ -178,6 +225,7 @@

#define write_lock_irq(lock) \
do { \
+ check_spinlock_irq(lock); \
local_irq_disable(); \
preempt_disable(); \
_raw_write_lock(lock); \
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/