On 12/7/23 16:01, Christophe Leroy wrote:
Le 07/12/2023 à 13:51, George Stark a écrit :
On 12/7/23 15:28, Christophe Leroy wrote:
Le 07/12/2023 à 13:02, Andy Shevchenko a écrit :
On Thu, Dec 7, 2023 at 1:23 AM George Stark
<gnstark@xxxxxxxxxxxxxxxxx> wrote:
On 12/7/23 01:37, Christophe Leroy wrote:
Le 06/12/2023 à 23:14, Christophe Leroy a écrit :
...
Looking at it closer, I have the feeling that you want to do
similar to
devm_gpio_request() in linux/gpio.h :
In linux/mutex.h, add a prototype for devm_mutex_init() when
CONFIG_DEBUG_MUTEXES is defined and an empty static inline otherwise.
Then define devm_mutex_init() in kernel/locking/mutex-debug.c
Yes, this would be almost perfect decision. BTW just as in linux/gpio.h
we wouldn't have to include whole "linux/device.h" into mutex.h, only
add forward declaration of struct device;
In case you place it into a C-file. Otherwise you need a header for
the API and that is not acceptable for mutex.h.
Right, that's the reason why I'm suggesting to define devm_mutex_init()
in kernel/locking/mutex-debug.c.
In linux/mutex.h, you define a stub for when CONFIG_DEBUG_MUTEXES is not
set, and the prototype of devm_mutex_init() when CONFIG_DEBUG_MUTEXES is
set.
Something like this:
diff --git a/include/linux/mutex.h b/include/linux/mutex.h
index a33aa9eb9fc3..4a6041a7fd44 100644
--- a/include/linux/mutex.h
+++ b/include/linux/mutex.h
@@ -21,6 +21,8 @@
#include <linux/debug_locks.h>
#include <linux/cleanup.h>
+struct device;
+
#ifdef CONFIG_DEBUG_LOCK_ALLOC
# define __DEP_MAP_MUTEX_INITIALIZER(lockname) \
, .dep_map = { \
@@ -127,6 +129,20 @@ extern void __mutex_init(struct mutex *lock, const
char *name,
*/
extern bool mutex_is_locked(struct mutex *lock);
+#ifdef CONFIG_DEBUG_MUTEXES
There is already a CONFIG_DEBUG_MUTEXES block, can you re-use it ?
those CONFIG_DEBUG_MUTEXES blockd are declared before mutex_init macro :(
+
+extern int devm_mutex_init(struct device *dev, struct mutex *lock);
'extern' is pointless and deprecated for function prototypes.
I know the kernel is full of them, but it is not a good reason to add
new ones.
Ok
Sure I will send this patch in the right way and then we could have
proper review but firstly I'd like to hear from Andy and mutex.h's
maintainers is it acceptable at all?
+
+#else
+
+static inline int devm_mutex_init(struct device *dev, struct mutex *lock)
+{
+ mutex_init(lock);
+ return 0;
+}
+
+#endif
+
#else /* !CONFIG_PREEMPT_RT */
/*
* Preempt-RT variant based on rtmutexes.
@@ -169,6 +185,13 @@ do { \
\
__mutex_init((mutex), #mutex, &__key); \
} while (0)
+
+static inline int devm_mutex_init(struct device *dev, struct mutex *lock)
+{
+ mutex_init(lock);
+ return 0;
+}
+
#endif /* CONFIG_PREEMPT_RT */
/*
diff --git a/kernel/locking/mutex-debug.c b/kernel/locking/mutex-debug.c
index bc8abb8549d2..d50dfa06e82c 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"
@@ -104,3 +105,25 @@ void mutex_destroy(struct mutex *lock)
}
EXPORT_SYMBOL_GPL(mutex_destroy);
+
+static void devm_mutex_release(void *res)
+{
+ mutex_destroy(res);
+}
+
+/**
+ * devm_mutex_init - Resource-managed mutex initialization
+ * @dev: Device which lifetime mutex is bound to
+ * @lock: Pointer to a mutex
+ *
+ * Initialize mutex which is automatically destroyed when the driver is
detached.
+ *
+ * Returns: 0 on success or a negative error code on failure.
+ */
+int devm_mutex_init(struct device *dev, struct mutex *lock)
+{
+ mutex_init(lock);
+ return devm_add_action_or_reset(dev, devm_mutex_release, lock);
+}
+
+EXPORT_SYMBOL_GPL(devm_mutex_init);
\ No newline at end of file
--
Best regards
George
