Hi, On 12/14/23 11:06, Christophe Leroy wrote: > > > Le 13/12/2023 à 23:30, George Stark a écrit : >> [Vous ne recevez pas souvent de courriers de gnstark@xxxxxxxxxxxxxxxxx. Découvrez pourquoi ceci est important à https://aka.ms/LearnAboutSenderIdentification ] >> >> Using of devm API leads to a certain order of releasing resources. >> So all dependent resources which are not devm-wrapped should be deleted >> with respect to devm-release order. Mutex is one of such objects that >> often is bound to other resources and has no own devm wrapper. >> Since mutex_destroy() actually does nothing in non-debug builds >> frequently calling mutex_destroy() is just ignored which is safe for now >> but wrong formally and can lead to a problem if mutex_destroy() is >> extended so introduce devm_mutex_init(). > > So you abandonned the idea of using mutex.h ? > > 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(). FWIW defining devm_mutex_init() in mutex.h is fine with me and makes sense to me. I also agree that putting it there would be better if that is acceptable for the mutex maintainers. devm-helpers.h is there for helpers which don't fit in another place. Regards, Hans > > >> >> Signed-off-by: George Stark <gnstark@xxxxxxxxxxxxxxxxx> >> --- >> 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 >>
