Re: [PATCH v6 1/9] locking/mutex: introduce devm_mutex_init

[Andy Shevchenko]

On Thu, Mar 14, 2024 at 10:46 AM George Stark <gnstark@xxxxxxxxxxxxxxxxx> wrote:
>
> 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 wrapping.
> 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() will be
> extended so introduce devm_mutex_init()

Missing period at the end.

...

> Suggested by-by: Christophe Leroy <christophe.leroy@xxxxxxxxxx>

Needs properly spelled tag.

..

> +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

mutex_destroy()

> +        * no really need to register it in devm subsystem.

in the devm

> +        */
> +       return 0;
> +}

..

> @@ -19,6 +19,7 @@
>  #include <linux/kallsyms.h>
>  #include <linux/interrupt.h>
>  #include <linux/debug_locks.h>
> +#include <linux/device.h>

Without seeing much context can't say if there is a better (more
ordered) place to squeeze a new header to. Please, check.

..

After addressing the above comments
Reviewed-by: Andy Shevchenko <andy.shevchenko@xxxxxxxxx>

-- 
With Best Regards,
Andy Shevchenko