Re: [PATCH v3 03/11] devm-helpers: introduce devm_mutex_init
From: George Stark <hidden>
Date: 2023-12-14 12:48:28
Also in:
linux-leds, lkml
Hello Christophe On 12/14/23 13:06, Christophe Leroy wrote:
...
So you abandonned the idea of using mutex.h ?
I'm not the one who make a choice here. The patch [1] you're talking about was seen by everyone but it seems like no one had shown interest. For me personally approach with #define mutex_destroy is not very usual but if even slight mixing device with mutex.h is unacceptable what else can we do? Avoiding the need to allocate devm slot for empty mutex_destroy is more important. Should I make series #4 with the patch [1] to give it a last chance? [1] https://lore.kernel.org/lkml/377e4437-7051-4d88-ae68-1460bcd692e1@redhat.com/T/#m3f6df30ffccaccb1df4669a327f349164f572931 (local)
I can't see the point to spread mutex functions into devm-helpers.h Adding a mutex_destroy macro for this purpose looks odd. And if someone defines a new version of mutex_destroy() and forget the macro, it will go undetected. Usually macros of that type serve the purpose of defining a fallback when the macro is not defined. In that case, when someone adds a new version without defining the macro, it gets detected because if conflicts with the fallback. But in your case it works the other way round, so I will just go undetected. For me the best solution remains to use mutex.h and have devm_mutex_init() defined or declared at the same place as mutex_destroy().
quoted
Signed-off-by: George Stark <redacted> --- include/linux/devm-helpers.h | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+)diff --git a/include/linux/devm-helpers.h b/include/linux/devm-helpers.h index 74891802200d..4043c3481d2e 100644 --- a/include/linux/devm-helpers.h +++ b/include/linux/devm-helpers.h@@ -24,6 +24,7 @@ */ #include <linux/device.h> +#include <linux/mutex.h> #include <linux/workqueue.h> static inline void devm_delayed_work_drop(void *res)@@ -76,4 +77,30 @@ static inline int devm_work_autocancel(struct device *dev, return devm_add_action(dev, devm_work_drop, w); } +#ifdef mutex_destroy +static inline void devm_mutex_release(void *res) +{ + mutex_destroy(res); +} +#endif + +/** + * 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. + */ +static inline int devm_mutex_init(struct device *dev, struct mutex *lock) +{ + mutex_init(lock); +#ifdef mutex_destroy + return devm_add_action_or_reset(dev, devm_mutex_release, lock); +#else + return 0; +#endif +} + #endif --2.25.1
-- Best regards George