Messages in this thread Patch in this message | | | Date | Tue, 6 Jun 2006 09:54:34 +0200 | From | Ingo Molnar <> | Subject | [patch, -rc5-mm3] lock validator: add CONFIG_DEBUG_NON_NESTED_UNLOCKS |
| |
Subject: lock validator: add CONFIG_DEBUG_NON_NESTED_UNLOCKS From: Ingo Molnar <mingo@elte.hu>
add CONFIG_DEBUG_NON_NESTED_UNLOCKS: if enabled then a non-alarming message about out of order unlocks is printed. If disabled then we fall back to the non-nested unlock method automatically, without printing a message and stopping the validator.
defaults to disabled. Tested with the option both on and off.
Signed-off-by: Ingo Molnar <mingo@elte.hu> --- kernel/lockdep.c | 90 ++++++++++++++++++++++++++++--------------------- lib/Kconfig.debug | 13 +++++++ lib/locking-selftest.c | 14 +++++++ 3 files changed, 79 insertions(+), 38 deletions(-)
Index: linux/kernel/lockdep.c =================================================================== --- linux.orig/kernel/lockdep.c +++ linux/kernel/lockdep.c @@ -2199,6 +2199,8 @@ static int __lockdep_acquire(struct lock return 1; } +#ifdef CONFIG_DEBUG_NON_NESTED_UNLOCKS + static int print_unlock_order_bug(struct task_struct *curr, struct lockdep_map *lock, struct held_lock *hlock, unsigned long ip) @@ -2207,9 +2209,10 @@ print_unlock_order_bug(struct task_struc if (debug_locks_silent) return 0; - printk("\n======================================\n"); - printk( "[ BUG: bad unlock ordering detected! ]\n"); - printk( "--------------------------------------\n"); + printk("\n=======================================\n"); + printk( "[ INFO: bad unlock ordering detected. ]\n"); + printk( "---------------------------------------\n"); + printk("The code is fine but needs lock validator annotation.\n"); printk("%s/%d is trying to release lock (", curr->comm, curr->pid); print_lockdep_cache(lock); @@ -2226,6 +2229,8 @@ print_unlock_order_bug(struct task_struc return 0; } +#endif /* CONFIG_DEBUG_NON_NESTED_UNLOCKS */ + static int print_unlock_inbalance_bug(struct task_struct *curr, struct lockdep_map *lock, unsigned long ip) @@ -2270,41 +2275,6 @@ static int check_unlock(struct task_stru } /* - * Remove the lock to the list of currently held locks - this gets - * called on mutex_unlock()/spin_unlock*() (or on a failed - * mutex_lock_interruptible()). This is done for unlocks that nest - * perfectly. (i.e. the current top of the lock-stack is unlocked) - */ -static int lockdep_release_nested(struct task_struct *curr, - struct lockdep_map *lock, unsigned long ip) -{ - struct held_lock *hlock; - unsigned int depth; - - /* - * Pop off the top of the lock stack: - */ - depth = --curr->lockdep_depth; - hlock = curr->held_locks + depth; - - if (hlock->instance != lock) - return print_unlock_order_bug(curr, lock, hlock, ip); - - if (DEBUG_WARN_ON(!depth && (hlock->prev_chain_key != 0))) - return 0; - - curr->curr_chain_key = hlock->prev_chain_key; - -#ifdef CONFIG_DEBUG_LOCKDEP - hlock->prev_chain_key = 0; - hlock->type = NULL; - hlock->acquire_ip = 0; - hlock->irq_context = 0; -#endif - return 1; -} - -/* * Remove the lock to the list of currently held locks in a * potentially non-nested (out of order) manner. This is a * relatively rare operation, as all the unlock APIs default @@ -2369,6 +2339,50 @@ found_it: * mutex_lock_interruptible()). This is done for unlocks that nest * perfectly. (i.e. the current top of the lock-stack is unlocked) */ +static int lockdep_release_nested(struct task_struct *curr, + struct lockdep_map *lock, unsigned long ip) +{ + struct held_lock *hlock; + unsigned int depth; + + /* + * Pop off the top of the lock stack: + */ + depth = curr->lockdep_depth - 1; + hlock = curr->held_locks + depth; + + /* + * Is the unlock non-nested: + */ + if (hlock->instance != lock) { +#ifdef CONFIG_DEBUG_NON_NESTED_UNLOCKS + return print_unlock_order_bug(curr, lock, hlock, ip); +#else + return lockdep_release_non_nested(curr, lock, ip); +#endif + } + curr->lockdep_depth--; + + if (DEBUG_WARN_ON(!depth && (hlock->prev_chain_key != 0))) + return 0; + + curr->curr_chain_key = hlock->prev_chain_key; + +#ifdef CONFIG_DEBUG_LOCKDEP + hlock->prev_chain_key = 0; + hlock->type = NULL; + hlock->acquire_ip = 0; + hlock->irq_context = 0; +#endif + return 1; +} + +/* + * Remove the lock to the list of currently held locks - this gets + * called on mutex_unlock()/spin_unlock*() (or on a failed + * mutex_lock_interruptible()). This is done for unlocks that nest + * perfectly. (i.e. the current top of the lock-stack is unlocked) + */ static void __lockdep_release(struct lockdep_map *lock, int nested, unsigned long ip) { Index: linux/lib/Kconfig.debug =================================================================== --- linux.orig/lib/Kconfig.debug +++ linux/lib/Kconfig.debug @@ -361,6 +361,19 @@ config LOCKDEP select KALLSYMS_ALL depends on PROVE_SPIN_LOCKING || PROVE_RW_LOCKING || PROVE_MUTEX_LOCKING || PROVE_RWSEM_LOCKING +config DEBUG_NON_NESTED_UNLOCKS + bool "Detect non-nested unlocks" + depends on LOCKDEP + help + If you say Y here, the lock dependency engine will do + additional runtime checks to detect and print non-nested + unlocks. + Non-nested unlocks are valid uses of the kernel's locking + APIs, but they cause more overhead for the validator, and + they can also be a sign for locking bugs or suboptimal + locking, so it's not a bad idea to annotate and thus + document those places. + config DEBUG_LOCKDEP bool "Lock dependency engine debugging" depends on LOCKDEP Index: linux/lib/locking-selftest.c =================================================================== --- linux.orig/lib/locking-selftest.c +++ linux/lib/locking-selftest.c @@ -1027,6 +1027,16 @@ static inline void print_testname(const dotest(name##_rsem, FAILURE, LOCKTYPE_RWSEM); \ printk("\n"); +#define DO_TESTCASE_6_SUCCESS(desc, name) \ + print_testname(desc); \ + dotest(name##_spin, SUCCESS, LOCKTYPE_SPIN); \ + dotest(name##_wlock, SUCCESS, LOCKTYPE_RWLOCK); \ + dotest(name##_rlock, SUCCESS, LOCKTYPE_RWLOCK); \ + dotest(name##_mutex, SUCCESS, LOCKTYPE_MUTEX); \ + dotest(name##_wsem, SUCCESS, LOCKTYPE_RWSEM); \ + dotest(name##_rsem, SUCCESS, LOCKTYPE_RWSEM); \ + printk("\n"); + /* * 'read' variant: rlocks must not trigger. */ @@ -1129,7 +1139,11 @@ void locking_selftest(void) DO_TESTCASE_6R("A-B-C-D-B-D-D-A deadlock", ABCDBDDA); DO_TESTCASE_6R("A-B-C-D-B-C-D-A deadlock", ABCDBCDA); DO_TESTCASE_6("double unlock", double_unlock); +#ifdef CONFIG_DEBUG_NON_NESTED_UNLOCKS DO_TESTCASE_6("bad unlock order", bad_unlock_order); +#else + DO_TESTCASE_6_SUCCESS("bad unlock order", bad_unlock_order); +#endif printk(" --------------------------------------------------------------------------\n"); print_testname("recursive read-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/
| |