Generic Mutex Subsystem, mutex-migration-helper-core.patch
Subsystems (2)
PATCH 10/15 Generic Mutex Subsystem, mutex-migration-helper-core.patch
introduce the mutex_debug type, and switch the semaphore APIs to it in a type-sensitive way. Plain semaphores will still use the proper arch-semaphore calls.
----
include/linux/mutex.h | 116 ++++++++++++++++++++++++++++++++++++++++++++++ include/linux/semaphore.h | 64 +++++++++++++++++++++++++ kernel/mutex.c | 73 ++++++++++++++++++++++++++++ lib/Kconfig.debug | 28 +++++++++++ lib/semaphore-sleepers.c | 16 ++++-- 5 files changed, 293 insertions(+), 4 deletions(-)
include/linux/mutex.h +116 -0
@@ -99,4 +99,120 @@ extern int FASTCALL(mutex_trylock(structextern void FASTCALL (mutex_unlock (struct mutex *lock));extern int FASTCALL (mutex_is_locked (struct mutex *lock)); +/* +* Debugging variant of mutexes. The only difference is that they accept +* the semaphore APIs too: +*/ +struct mutex_debug { +struct mutex lock; +}; + +#define DEFINE_MUTEX_DEBUG (mutexname) \ + struct mutex_debug mutexname = \ + { .lock = __MUTEX_INITIALIZER(mutexname.lock) } + +extern void FASTCALL (mutex_debug_down (struct mutex_debug *lock)); +extern int FASTCALL (mutex_debug_down_interruptible (struct mutex_debug *lock)); +extern int FASTCALL (mutex_debug_down_trylock (struct mutex_debug *lock)); +extern void FASTCALL (mutex_debug_up (struct mutex_debug *lock)); +extern int FASTCALL (mutex_debug_sem_is_locked (struct mutex_debug *lock)); + +/* +* The down()/up() type-safe wrappers, for debug mutexes: +*/ + +extern int __bad_mutex_debug_func_type (void ); + +#undef TYPE_EQUAL +#define TYPE_EQUAL (var, type) \ + __builtin_types_compatible_p(typeof(var), type *) + +#define PICK_FUNC_1ARG (type1, type2, func1, func2, arg) \ +do { \ + if (TYPE_EQUAL((arg), type1)) \ + func1((type1 *)(arg)); \ + else if (TYPE_EQUAL((arg), type2)) \ + func2((type2 *)(arg)); \ + else __bad_mutex_debug_func_type(); \ +} while (0) + +#define PICK_FUNC_1ARG_RET (type1, type2, func1, func2, arg) \ +({ \ + unsigned long __ret; \ + \ + if (TYPE_EQUAL((arg), type1)) \ + __ret = func1((type1 *)(arg)); \ + else if (TYPE_EQUAL((arg), type2)) \ + __ret = func2((type2 *)(arg)); \ + else __ret = __bad_mutex_debug_func_type(); \ + \ + __ret; \ +}) + +#define PICK_FUNC_2ARG (type1, type2, func1, func2, arg0, arg1) \ +do { \ + if (TYPE_EQUAL((arg0), type1)) \ + func1((type1 *)(arg0), arg1); \ + else if (TYPE_EQUAL((arg0), type2)) \ + func2((type2 *)(arg0), arg1); \ + else __bad_mutex_debug_func_type(); \ +} while (0) + +#define sema_init (sem, val) \ + PICK_FUNC_2ARG(struct arch_semaphore, struct mutex_debug, \ + arch_sema_init, mutex_debug_sema_init, sem, val) + +#define init_MUTEX (sem) \ + PICK_FUNC_1ARG(struct arch_semaphore, struct mutex_debug, \ + arch_init_MUTEX, mutex_debug_init_MUTEX, sem) + +#define init_MUTEX_LOCKED (sem) \ + PICK_FUNC_1ARG(struct arch_semaphore, struct mutex_debug, \ + arch_init_MUTEX_LOCKED, mutex_debug_init_MUTEX_LOCKED, sem) + +#define down (sem) \ + PICK_FUNC_1ARG(struct arch_semaphore, struct mutex_debug, \ + arch_down, mutex_debug_down, sem) + +#define down_interruptible (sem) \ + PICK_FUNC_1ARG_RET(struct arch_semaphore, struct mutex_debug, \ + arch_down_interruptible, mutex_debug_down_interruptible, sem) + +#define down_trylock (sem) \ + PICK_FUNC_1ARG_RET(struct arch_semaphore, struct mutex_debug, \ + arch_down_trylock, mutex_debug_down_trylock, sem) + +#define up (sem) \ + PICK_FUNC_1ARG(struct arch_semaphore, struct mutex_debug, \ + arch_up, mutex_debug_up, sem) + +#define sem_is_locked (sem) \ + PICK_FUNC_1ARG_RET(struct arch_semaphore, struct mutex_debug, \ + arch_sem_is_locked, mutex_debug_sem_is_locked, sem) + +#define sema_count (sem) \ + PICK_FUNC_1ARG_RET(struct arch_semaphore, struct mutex_debug, \ + arch_sema_count, mutex_debug_sema_count, sem) + +#ifdef CONFIG_DEBUG_MUTEXESS + extern FASTCALL(void __mutex_debug_sema_init(struct mutex_debug *lock, + int val, char *name, char *file, int line)); +# define mutex_debug_sema_init (sem, val) \ + __mutex_debug_sema_init(sem, val, #sem, __FILE__, __LINE__) +#else + extern FASTCALL(void __mutex_debug_sema_init(struct mutex_debug *lock, int val)); +# define mutex_debug_sema_init (sem, val) \ + __mutex_debug_sema_init(sem, val) +#endif + +#define mutex_debug_init_MUTEX (sem) mutex_debug_sema_init(sem, 1) + +/* +* No locked initialization for mutexes: +*/ +extern void there_is_no_init_MUTEX_LOCKED_for_mutexes (void ); + +#define mutex_debug_init_MUTEX_LOCKED (sem) \ + there_is_no_init_MUTEX_LOCKED_for_mutexes() +#endif
include/linux/semaphore.h new file +64 -0
64 lines not on this page · raw message
kernel/mutex.c +73 -0
76 lines not on this page · raw message
lib/Kconfig.debug +28 -0
@@ -95,6 +95,34 @@ config DEBUG_PREEMPT if kernel code uses it in a preemption-unsafe way. Also, the kernel will detect preemption count underflows. +config GENERIC_MUTEXES + bool + depends on ARCH_SEMAPHORES + default y + +choice + prompt "Mutex Debugging Mode" + default DEBUG_NONE + depends on ARCH_SEMAPHORES + +config DEBUG_MUTEX_NONE + bool "Off" + help + No runtime checking of semaphores. Fastest mode. + +config DEBUG_MUTEX_PARTIAL + bool "Partial" + help + Check mutexes that have been marked as mutexes explicitly. + +config DEBUG_MUTEX_FULL + bool "Full" + help + Check all mutexes and semaphores. (except the handful of + semaphores that have been explicitly marked as non-debug) + +endchoice + config DEBUG_MUTEXESS bool "Mutex debugging, deadlock detection" default y
lib/semaphore-sleepers.c +12 -4
@@ -16,6 +16,7 @@#include <linux/sched.h> #include <linux/err.h> #include <linux/init.h> +#include <linux/module.h> #include <asm/semaphore.h> /*
@@ -49,12 +50,12 @@* we cannot lose wakeup events. */ -fastcall void __up(struct semaphore *sem) +fastcall void __up(struct arch_semaphore *sem) {wake_up (&sem->wait); } -fastcall void __sched __down(struct semaphore * sem) +fastcall void __sched __down(struct arch_semaphore * sem) {struct task_struct *tsk = current;DECLARE_WAITQUEUE (wait, tsk);
@@ -91,7 +92,7 @@ fastcall void __sched __down(struct sema tsk->state =TASK_RUNNING ; } -fastcall int __sched __down_interruptible(struct semaphore * sem) +fastcall int __sched __down_interruptible(struct arch_semaphore * sem) {int retval =0 ;struct task_struct *tsk = current;
@@ -154,7 +155,7 @@ fastcall int __sched __down_interruptibl* single "cmpxchg" without failure cases, * but then it wouldn't work on a 386. */ -fastcall int __down_trylock(struct semaphore * sem) +fastcall int __down_trylock(struct arch_semaphore * sem) {int sleepers;unsigned long flags;
@@ -175,3 +176,10 @@ fastcall int __down_trylock(struct semapspin_unlock_irqrestore (&sem->wait.lock, flags);return 1 ; } + +int fastcall arch_sem_is_locked(struct arch_semaphore *sem) +{ +return (int )atomic_read (&sem->count) <0 ; +} + +EXPORT_SYMBOL (arch_sem_is_locked);
include/linux/semaphore.h line 16
Quoted 10 lines · Ingo Molnar
include/linux/semaphore.h new file +10
-0+#ifdef CONFIG_DEBUG_MUTEX_FULL +# define semaphore mutex_debug +# define DECLARE_MUTEX DEFINE_MUTEX_DEBUG +#else +# define DECLARE_MUTEX ARCH_DECLARE_MUTEX +#endif + +# define DECLARE_MUTEX_LOCKED ARCH_DECLARE_MUTEX_LOCKED + +#if 0
Probably not good to have #if 0 in release patches.
-- Steve
Quoted 7 lines
include/linux/semaphore.h new file +7
-0+#ifdef CONFIG_GENERIC_MUTEXES +# include <linux/mutex.h> +#else + +#define DECLARE_MUTEX ARCH_DECLARE_MUTEX +#define DECLARE_MUTEX_LOCKED ARCH_DECLARE_MUTEX_LOCKED +
Quoted 13 lines · Steven Rostedt
include/linux/semaphore.h line 16
Quoted 10 lines · Ingo Molnar
include/linux/semaphore.h new file +10
-0+#ifdef CONFIG_DEBUG_MUTEX_FULL +# define semaphore mutex_debug +# define DECLARE_MUTEX DEFINE_MUTEX_DEBUG +#else +# define DECLARE_MUTEX ARCH_DECLARE_MUTEX +#endif + +# define DECLARE_MUTEX_LOCKED ARCH_DECLARE_MUTEX_LOCKED + +#if 0 Probably not good to have #if 0 in release patches.
yeah - i'll zap it.
Ingo
Quoted 15 lines · Ingo Molnar
kernel/mutex.c +3
-0+#ifdef CONFIG_DEBUG_MUTEXESS +void fastcall +__mutex_debug_sema_init(struct mutex_debug *lock, int val, char*name,
kernel/mutex.c +10
-0+ char *file, int line) +{ +__mutex_init (&lock->lock, name, file, line); + +DEBUG_WARN_ON (val !=0 && val !=1 ); +if (!val) + __mutex_lock(&lock->lock __CALLER_IP__); +} +#else +void fastcall __mutex_debug_sema_init(struct mutex_debug *lock, intval)
kernel/mutex.c void fastcall __mutex_init(struct mutex · line 586
Quoted 4 lines
kernel/mutex.c +4
-0+{ +__mutex_init (&lock->lock); + +DEBUG_WARN_ON (val !=0 && val !=1 );
DEBUG_WARN_ON in this part of the #if is always a no-op.
-- Steve
Quoted 5 lines
kernel/mutex.c +5
-0+if (!val) + __mutex_lock(&lock->lock __CALLER_IP__); +} +#endif +
Quoted 3 lines · Steven Rostedt
Quoted 1 line
+
DEBUG_WARN_ON(val != 0 && val != 1);
DEBUG_WARN_ONin this part of the #if is always a no-op.
ok, zapped it.
Ingo
include/linux/mutex.h
Quoted 4 lines · Ingo Molnar
Index:
linux/include/linux/mutex.h
===================================================================include/linux/mutex.h +0
-0
Maybe this should be in its own mutex-debug.h file with a:
#ifndef __LIUNX_MUTEX_H
# error Do not include this file directly, use mutex.h #endif
Quoted 7 lines
include/linux/mutex.h +2
-0@@ -99,4 +99,120 @@ extern int FASTCALL(mutex_trylock(structextern void FASTCALL (mutex_unlock (struct mutex *lock));extern int FASTCALL (mutex_is_locked (struct mutex *lock)); +/* +* Debugging variant of mutexes. The only difference is that they accept
Also, add a comment here that mutex_debug should NOT be used directly. This may seem obvious, but new Linux kernel programmers may just be scanning the code for what they would like to use and add it. At least let them know (although it may seem obvious) that this is just a temporary structure that will go away soon, and if they want to use mutexes, then use mutex, and don't be tempted to have a mutex up/down.
OK, I'm watching out for the idiots, but since I'm one every so often, I hope someone watches out for me ;-)
-- Steve
Quoted 10 lines
+ * the semaphore APIs too:
+ */include/linux/mutex.h +8
-0+struct mutex_debug { +struct mutex lock; +}; + +#define DEFINE_MUTEX_DEBUG (mutexname) \ + struct mutex_debug mutexname = \ + { .lock = __MUTEX_INITIALIZER(mutexname.lock) } +
Quoted 11 lines · Steven Rostedt
include/linux/mutex.h
Quoted 4 lines · Ingo Molnar
Index:
linux/include/linux/mutex.h
===================================================================include/linux/mutex.h +0
-0Maybe this should be in its own
mutex-debug.hfile with a:#ifndef
__LIUNX_MUTEX_H# error Do not include this file directly, use mutex.h #endif
yeah.
Quoted 10 lines
Quoted 3 lines
+/*
+ * Debugging variant of mutexes. The only difference is that they acceptAlso, add a comment here that
mutex_debugshould NOT be used directly. This may seem obvious, but new Linux kernel programmers may just be scanning the code for what they would like to use and add it. At least let them know (although it may seem obvious) that this is just a temporary structure that will go away soon, and if they want to use mutexes, then use mutex, and don't be tempted to have a mutex up/down.
well, it could surface temporarily, as in 'could you try this test-release, it also turns on mutexes, lets see whether it breaks' - without the subsystem maintainer having to do a big patch changing all the down()/up() calls to mutex_lock()/mutex_unlock() ...
Ingo
Quoted 4 lines · Ingo Molnar
introduce the
mutex_debugtype, and switch the semaphore APIs to it in a type-sensitive way. Plain semaphores will still use the proper arch-semaphore calls.
I think we shouldn't introduce this one. It just encourages people to do really things. Everything else in the patchseries looks sensible to me.
Quoted 8 lines · Christoph Hellwig
Quoted 4 lines · Ingo Molnar
introduce the
mutex_debugtype, and switch the semaphore APIs to it in a type-sensitive way. Plain semaphores will still use the proper arch-semaphore calls.I think we shouldn't introduce this one. It just encourages people to do really things. Everything else in the patchseries looks sensible to me.
I believe that Ingo is adding the "arch_semaphore" temporarily mainly for debugging. From Ingo's 00/15 email:
once all semaphores have been safely categorized and converted to one group or another, and all out-of-tree codebases are fixed and a deprecation period has passed, we can rename arch_semaphores back to 'struct semaphore'.
The arch_semaphore is not for a global change or to be what we call semaphores from now on. It is used as a place marker for debugging, until we feel comfortable that we got all the places that need to be mutexes. So with this arch_semaphore, we can hit a debug option that turns all non arch_semaphores into mutexes and if it runs fine, we know we can change them from semaphores to mutexes.
So once the system has been audit enuough, we remove both the mutex_debug and the arch_semaphores, where all the remaining arch_semaphores become semaphores, and those semaphores that were not arch_semaphores should have become full mutexes.
-- Steve
Quoted 8 lines · Christoph Hellwig
Quoted 4 lines · Ingo Molnar
introduce the
mutex_debugtype, and switch the semaphore APIs to it in a type-sensitive way. Plain semaphores will still use the proper arch-semaphore calls.I think we shouldn't introduce this one. It just encourages people to do really things.
^---("weird" ?)ok. I can keep it a separate patch - it's needed for -rt, and it's also a good debugging tool to test-drive the mutex implementation.
historically, i found 90% of the mutex.c bugs by stress-testing it from userspace, but a crutial 10% did come from the DEBUG_MUTEX_FULL mode, where almost all of the 7000+ semaphore users are switched over to mutexes.
but in terms of keeping the upstream kernel code base clean, i agree with a clean and separate set of mutex APIs that are introduced into subsystems gradually.
Quoted 1 line
Everything else in the patchseries looks sensible to me.
great!
Ingo