Re: [PATCH v5 02/10] locking/mutex: introduce devm_mutex_init
From: George Stark <hidden>
Date: 2024-03-12 11:39:22
Also in:
linux-leds, lkml
Subsystem:
locking primitives, the rest · Maintainers:
Peter Zijlstra, Ingo Molnar, Will Deacon, Boqun Feng, Linus Torvalds
Hello Christophe Thanks for the review You were right about typecheck - it was meant to check errors even if CONFIG_DEBUG_MUTEXES was off. Here's new version based on the comments:
diff --git a/include/linux/mutex.h b/include/linux/mutex.h
index 67edc4ca2bee..9193b163038f 100644
--- a/include/linux/mutex.h
+++ b/include/linux/mutex.h@@ -22,6 +22,8 @@ #include <linux/cleanup.h> #include <linux/mutex_types.h> +struct device; + #ifdef CONFIG_DEBUG_LOCK_ALLOC # define __DEP_MAP_MUTEX_INITIALIZER(lockname) \ , .dep_map = { \
@@ -117,6 +119,34 @@ do { \ } while (0) #endif /* CONFIG_PREEMPT_RT */ +#ifdef CONFIG_DEBUG_MUTEXES + +int debug_devm_mutex_init(struct device *dev, struct mutex *lock); + +static inline int __devm_mutex_init(struct device *dev, struct mutex *lock) +{ + return debug_devm_mutex_init(dev, lock); +} + +#else + +static inline int __devm_mutex_init(struct device *dev, struct mutex *lock) +{ + /* + * When CONFIG_DEBUG_MUTEXES is off mutex_destroy is just a nop so + * no really need to register it in devm subsystem. + */ + return 0; +} + +#endif + +#define devm_mutex_init(dev, mutex) \ +({ \ + mutex_init(mutex); \ + __devm_mutex_init(dev, mutex); \ +}) + /* * See kernel/locking/mutex.c for detailed documentation of these APIs. * Also see Documentation/locking/mutex-design.rst.
diff --git a/kernel/locking/mutex-debug.c b/kernel/locking/mutex-debug.c
index bc8abb8549d2..967a5367c79a 100644
--- a/kernel/locking/mutex-debug.c
+++ b/kernel/locking/mutex-debug.c@@ -19,6 +19,7 @@ #include <linux/kallsyms.h> #include <linux/interrupt.h> #include <linux/debug_locks.h> +#include <linux/device.h> #include "mutex.h"
@@ -89,6 +90,16 @@ void debug_mutex_init(struct mutex *lock, const char *name,
lock->magic = lock;
}
+static void devm_mutex_release(void *res)
+{
+ mutex_destroy(res);
+}
+
+int debug_devm_mutex_init(struct device *dev, struct mutex *lock)
+{
+ return devm_add_action_or_reset(dev, devm_mutex_release, lock);
+}
+
/***
* mutex_destroy - mark a mutex unusable
* @lock: the mutex to be destroyed
--
2.25.1
On 3/12/24 09:04, Christophe Leroy wrote:
...
I think it would be preferable to minimise the number of macros.
If I were you I would keep your devm_mutex_init() as is but rename it
__devm_mutex_init() and just remove the mutex_init() from it, then add
only one macro that works independant of CONFIG_DEBUG_MUTEXES:
#define devm_mutex_init(dev, mutex) \
({ \
mutex_init(mutex); \
__devm_mutex_init(dev, mutex); \
})
With that, no need of a second version of the macro and no need for the
typecheck either.
Note the __ which is a clear indication that allthough that function is
declared in public mutex.h, it is not meant to be used outside of it.
-- Best regards George