Re: [PATCH] lock assertion macros for 2.5.31

[Jesse Barnes <jbarnes@sgi.com>]

On Mon, Aug 12, 2002 at 04:12:41PM -0700, Andrew Morton wrote:
> ...
> #define might_sleep()	BUG_ON(preempt_count())
> 
> _this_ would catch numerous bugs, including code which is not buggy
> in 2.4, but became buggy when wild-eyed loonies changed core kernel
> rules without even looking at what drivers were doing (rant).
> 
> I expect something like this will fall out of the wash soon, at
> least for preemptible kernels.

Is it really that simple?  Maybe it should go into sched.h sometime
soon?  I guess the real work is sprinkling it in all the places where
it needs to go.

Anyway, here's an updated version of the lock assertion patch.  Should
it be split into two patches, one that implements the macros and
another that puts checks everywhere?  Should I add a small doc to
Documentation/ (maybe the might_sleep() could be documented there
too)?

Thanks,
Jesse


diff -Naur -X /home/jbarnes/dontdiff linux-2.5.31/fs/inode.c linux-2.5.31-lockassert/fs/inode.c
--- linux-2.5.31/fs/inode.c	Sat Aug 10 18:42:05 2002
+++ linux-2.5.31-lockassert/fs/inode.c	Wed Aug 21 11:14:49 2002
@@ -193,6 +193,8 @@
  */
 void __iget(struct inode * inode)
 {
+	assert_locked(&inode_lock);
+
 	if (atomic_read(&inode->i_count)) {
 		atomic_inc(&inode->i_count);
 		return;
diff -Naur -X /home/jbarnes/dontdiff linux-2.5.31/include/asm-i386/semaphore.h linux-2.5.31-lockassert/include/asm-i386/semaphore.h
--- linux-2.5.31/include/asm-i386/semaphore.h	Sat Aug 10 18:41:24 2002
+++ linux-2.5.31-lockassert/include/asm-i386/semaphore.h	Wed Aug 21 11:13:11 2002
@@ -40,6 +40,7 @@
 #include <asm/atomic.h>
 #include <linux/wait.h>
 #include <linux/rwsem.h>
+#include <linux/config.h>
 
 struct semaphore {
 	atomic_t count;
@@ -55,6 +56,12 @@
 		, (int)&(name).__magic
 #else
 # define __SEM_DEBUG_INIT(name)
+#endif
+
+#ifdef CONFIG_DEBUG_SPINLOCK
+# define assert_sem_held(sem)		BUG_ON(!down_trylock(sem))
+#else
+# define assert_sem_held(sem)		do { } while(0)
 #endif
 
 #define __SEMAPHORE_INITIALIZER(name,count) \
diff -Naur -X /home/jbarnes/dontdiff linux-2.5.31/include/asm-i386/spinlock.h linux-2.5.31-lockassert/include/asm-i386/spinlock.h
--- linux-2.5.31/include/asm-i386/spinlock.h	Sat Aug 10 18:41:23 2002
+++ linux-2.5.31-lockassert/include/asm-i386/spinlock.h	Wed Aug 21 11:05:41 2002
@@ -157,6 +157,7 @@
 #define RW_LOCK_UNLOCKED (rwlock_t) { RW_LOCK_BIAS RWLOCK_MAGIC_INIT }
 
 #define rwlock_init(x)	do { *(x) = RW_LOCK_UNLOCKED; } while(0)
+#define rwlock_is_locked(x) ((x)->lock != RW_LOCK_BIAS)
 
 /*
  * On x86, we implement read-write locks as a 32-bit counter
diff -Naur -X /home/jbarnes/dontdiff linux-2.5.31/include/asm-ia64/semaphore.h linux-2.5.31-lockassert/include/asm-ia64/semaphore.h
--- linux-2.5.31/include/asm-ia64/semaphore.h	Sat Aug 10 18:41:28 2002
+++ linux-2.5.31-lockassert/include/asm-ia64/semaphore.h	Wed Aug 21 11:13:28 2002
@@ -6,6 +6,7 @@
  * Copyright (C) 1998-2000 David Mosberger-Tang <davidm@hpl.hp.com>
  */
 
+#include <linux/config.h>
 #include <linux/wait.h>
 #include <linux/rwsem.h>
 
@@ -24,6 +25,12 @@
 # define __SEM_DEBUG_INIT(name)		, (long) &(name).__magic
 #else
 # define __SEM_DEBUG_INIT(name)
+#endif
+
+#ifdef CONFIG_DEBUG_SPINLOCK
+# define assert_sem_held(sem)		BUG_ON(!down_trylock(sem))
+#else
+# define assert_sem_held(sem)		do { } while(0)
 #endif
 
 #define __SEMAPHORE_INITIALIZER(name,count)					\
diff -Naur -X /home/jbarnes/dontdiff linux-2.5.31/include/asm-ia64/spinlock.h linux-2.5.31-lockassert/include/asm-ia64/spinlock.h
--- linux-2.5.31/include/asm-ia64/spinlock.h	Sat Aug 10 18:41:19 2002
+++ linux-2.5.31-lockassert/include/asm-ia64/spinlock.h	Wed Aug 21 11:05:41 2002
@@ -109,6 +109,7 @@
 #define RW_LOCK_UNLOCKED (rwlock_t) { 0, 0 }
 
 #define rwlock_init(x) do { *(x) = RW_LOCK_UNLOCKED; } while(0)
+#define rwlock_is_locked(x) ((x)->read_counter != 0 || (x)->write_lock != 0)
 
 #define _raw_read_lock(rw)							\
 do {										\
diff -Naur -X /home/jbarnes/dontdiff linux-2.5.31/include/linux/rwsem-spinlock.h linux-2.5.31-lockassert/include/linux/rwsem-spinlock.h
--- linux-2.5.31/include/linux/rwsem-spinlock.h	Sat Aug 10 18:41:38 2002
+++ linux-2.5.31-lockassert/include/linux/rwsem-spinlock.h	Wed Aug 21 11:12:43 2002
@@ -46,6 +46,14 @@
 #define __RWSEM_DEBUG_INIT	/* */
 #endif
 
+#ifdef CONFIG_DEBUG_SPINLOCK
+#define assert_rwsem_held_for_write(rwsem)	BUG_ON(__down_read_trylock(sem))
+#define assert_rwsem_held_for_read(rwsem)	BUG_ON(__down_write_trylock(rwsem))
+#else
+#define assert_rwsem_held_for_write(rwsem)	do { } while(0)
+#define assert_rwsem_held_for_read(rwsem)	do { } while(0)
+#endif
+
 #define __RWSEM_INITIALIZER(name) \
 { 0, SPIN_LOCK_UNLOCKED, LIST_HEAD_INIT((name).wait_list) __RWSEM_DEBUG_INIT }
 
diff -Naur -X /home/jbarnes/dontdiff linux-2.5.31/include/linux/rwsem.h linux-2.5.31-lockassert/include/linux/rwsem.h
--- linux-2.5.31/include/linux/rwsem.h	Sat Aug 10 18:41:25 2002
+++ linux-2.5.31-lockassert/include/linux/rwsem.h	Wed Aug 21 11:05:41 2002
@@ -7,6 +7,7 @@
 #ifndef _LINUX_RWSEM_H
 #define _LINUX_RWSEM_H
 
+#include <linux/config.h>
 #include <linux/linkage.h>
 
 #define RWSEM_DEBUG 0
diff -Naur -X /home/jbarnes/dontdiff linux-2.5.31/include/linux/spinlock.h linux-2.5.31-lockassert/include/linux/spinlock.h
--- linux-2.5.31/include/linux/spinlock.h	Sat Aug 10 18:41:29 2002
+++ linux-2.5.31-lockassert/include/linux/spinlock.h	Wed Aug 21 11:19:35 2002
@@ -117,7 +117,19 @@
 #define _raw_write_lock(lock)	(void)(lock) /* Not "unused variable". */
 #define _raw_write_unlock(lock)	do { } while(0)
 
-#endif /* !SMP */
+#endif /* !CONFIG_SMP */
+
+/*
+ * Simple lock assertions for debugging and documenting where locks need
+ * to be held.
+ */
+#if defined(CONFIG_DEBUG_SPINLOCK) && defined(CONFIG_SMP)
+#define assert_locked(lock)		BUG_ON(!spin_is_locked(lock))
+#define assert_rw_locked(lock)		BUG_ON(!rwlock_is_locked(lock))
+#else
+#define assert_locked(lock)		do { } while(0)
+#define assert_rw_locked(lock)		do { } while(0)
+#endif /* CONFIG_DEBUG_SPINLOCK && CONFIG_SMP */
 
 #ifdef CONFIG_PREEMPT
 
diff -Naur -X /home/jbarnes/dontdiff linux-2.5.31/kernel/acct.c linux-2.5.31-lockassert/kernel/acct.c
--- linux-2.5.31/kernel/acct.c	Sat Aug 10 18:41:36 2002
+++ linux-2.5.31-lockassert/kernel/acct.c	Wed Aug 21 11:15:03 2002
@@ -160,6 +160,8 @@
 {
 	struct file *old_acct = NULL;
 
+	assert_locked(&acct_globals.lock);
+
 	if (acct_globals.file) {
 		old_acct = acct_globals.file;
 		del_timer(&acct_globals.timer);
diff -Naur -X /home/jbarnes/dontdiff linux-2.5.31/kernel/printk.c linux-2.5.31-lockassert/kernel/printk.c
--- linux-2.5.31/kernel/printk.c	Sat Aug 10 18:41:29 2002
+++ linux-2.5.31-lockassert/kernel/printk.c	Wed Aug 21 11:15:18 2002
@@ -337,6 +337,8 @@
 	unsigned long cur_index, start_print;
 	static int msg_level = -1;
 
+	assert_sem_held(&console_sem);
+
 	if (((long)(start - end)) > 0)
 		BUG();