Re: [PATCH v6 1/9] locking/mutex: introduce devm_mutex_init
On Thu, 14 Mar 2024 11:45:23 +0300 George Stark 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() > > Signed-off-by: George Stark > Suggested by-by: Christophe Leroy Reviewed-by: Marek Behún
Re: [PATCH v5 02/10] locking/mutex: introduce devm_mutex_init
On Thu, 7 Mar 2024 08:39:46 -0500 Waiman Long wrote: > On 3/7/24 04:56, Marek Behún wrote: > > On Thu, Mar 07, 2024 at 05:40:26AM +0300, George Stark 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() > >> > >> Signed-off-by: George Stark > >> Signed-off-by: Christophe Leroy > >> --- > >> Hello Christophe. Hope you don't mind I put you SoB tag because you > >> helped alot > >> to make this patch happen. > >> > >> include/linux/mutex.h| 13 + > >> kernel/locking/mutex-debug.c | 22 ++ > >> 2 files changed, 35 insertions(+) > >> > >> diff --git a/include/linux/mutex.h b/include/linux/mutex.h > >> index f7611c092db7..9bcf72cb941a 100644 > >> --- a/include/linux/mutex.h > >> +++ b/include/linux/mutex.h > >> @@ -22,6 +22,8 @@ > >> #include > >> #include > >> > >> +struct device; > >> + > >> #ifdef CONFIG_DEBUG_LOCK_ALLOC > >> # define __DEP_MAP_MUTEX_INITIALIZER(lockname) \ > >>, .dep_map = { \ > >> @@ -115,10 +117,21 @@ do { > >> \ > >> > >> #ifdef CONFIG_DEBUG_MUTEXES > >> > >> +int devm_mutex_init(struct device *dev, struct mutex *lock); > >> void mutex_destroy(struct mutex *lock); > >> > >> #else > >> > >> +static inline int devm_mutex_init(struct device *dev, struct mutex *lock) > >> +{ > >> + /* > >> + * since mutex_destroy is nop actually there's no need to register it > >> + * in devm subsystem. > >> + */ > >> + mutex_init(lock); > >> + return 0; > >> +} > >> + > >> static inline void mutex_destroy(struct mutex *lock) {} > >> > >> #endif > >> diff --git a/kernel/locking/mutex-debug.c b/kernel/locking/mutex-debug.c > >> index bc8abb8549d2..c9efab1a8026 100644 > >> --- a/kernel/locking/mutex-debug.c > >> +++ b/kernel/locking/mutex-debug.c > >> @@ -19,6 +19,7 @@ > >> #include > >> #include > >> #include > >> +#include > >> > >> #include "mutex.h" > >> > >> @@ -104,3 +105,24 @@ 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); > > Hi George, > > > > look at > > https://lore.kernel.org/lkml/7013bf9e-2663-4613-ae61-61872e813...@redhat.com/ > > where Matthew and Hans explain that devm_mutex_init needs to be a macro > > because of the static lockdep key. > > > > so this should be something like: > > > > static inline int __devm_mutex_init(struct device *dev, struct mutex *mutex, > > const char *name, > > struct lock_class_key *key) > > { > > __mutex_init(mutex, name, key); > > return devm_add_action_or_reset(dev, devm_mutex_release, mutex); > > } > > > > #define devm_mutex_init(dev, mutex) \ > > do {
Re: [PATCH v5 02/10] locking/mutex: introduce devm_mutex_init
On Thu, Mar 07, 2024 at 05:40:26AM +0300, George Stark 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() > > Signed-off-by: George Stark > Signed-off-by: Christophe Leroy > --- > Hello Christophe. Hope you don't mind I put you SoB tag because you helped > alot > to make this patch happen. > > include/linux/mutex.h| 13 + > kernel/locking/mutex-debug.c | 22 ++ > 2 files changed, 35 insertions(+) > > diff --git a/include/linux/mutex.h b/include/linux/mutex.h > index f7611c092db7..9bcf72cb941a 100644 > --- a/include/linux/mutex.h > +++ b/include/linux/mutex.h > @@ -22,6 +22,8 @@ > #include > #include > > +struct device; > + > #ifdef CONFIG_DEBUG_LOCK_ALLOC > # define __DEP_MAP_MUTEX_INITIALIZER(lockname) \ > , .dep_map = { \ > @@ -115,10 +117,21 @@ do { > \ > > #ifdef CONFIG_DEBUG_MUTEXES > > +int devm_mutex_init(struct device *dev, struct mutex *lock); > void mutex_destroy(struct mutex *lock); > > #else > > +static inline int devm_mutex_init(struct device *dev, struct mutex *lock) > +{ > + /* > + * since mutex_destroy is nop actually there's no need to register it > + * in devm subsystem. > + */ > + mutex_init(lock); > + return 0; > +} > + > static inline void mutex_destroy(struct mutex *lock) {} > > #endif > diff --git a/kernel/locking/mutex-debug.c b/kernel/locking/mutex-debug.c > index bc8abb8549d2..c9efab1a8026 100644 > --- a/kernel/locking/mutex-debug.c > +++ b/kernel/locking/mutex-debug.c > @@ -19,6 +19,7 @@ > #include > #include > #include > +#include > > #include "mutex.h" > > @@ -104,3 +105,24 @@ 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); Hi George, look at https://lore.kernel.org/lkml/7013bf9e-2663-4613-ae61-61872e813...@redhat.com/ where Matthew and Hans explain that devm_mutex_init needs to be a macro because of the static lockdep key. so this should be something like: static inline int __devm_mutex_init(struct device *dev, struct mutex *mutex, const char *name, struct lock_class_key *key) { __mutex_init(mutex, name, key); return devm_add_action_or_reset(dev, devm_mutex_release, mutex); } #define devm_mutex_init(dev, mutex) \ do {\ static struct lock_class_key __key; \ \ __devm_mutex_init(dev, (mutex), #mutex, &__key);\ } while (0); Marek
Re: [PATCH] powerpc: dts: turris1x.dts: Add channel labels for temperature sensor
On Fri, 30 Sep 2022 14:39:01 +0200 Pali Rohár wrote: > Channel 0 of SA56004ED chip refers to internal SA56004ED chip sensor (chip > itself is located on the board) and channel 1 of SA56004ED chip refers to > external sensor which is connected to temperature diode of the P2020 CPU. > > Fixes: 54c15ec3b738 ("powerpc: dts: Add DTS file for CZ.NIC Turris 1.x > routers") > Signed-off-by: Pali Rohár Reviewed-by: Marek Behún
Re: [PATCH 6/6] i2c: Make remove callback return void
g , Nicolas Ferre , Robert Foss , Krzysztof Kozlowski , Daniel Vetter , Alvin =?UTF-8?B?xaBpcHJhZ2E=?= , Luca Ceresoli , =?UTF-8?B?Sm9zw6kgRXhww7NzaXRv?= , Johannes Berg , Colin Ian King , Maximilian Luz , Helge Deller , Lucas Stach Errors-To: linuxppc-dev-bounces+archive=mail-archive@lists.ozlabs.org Sender: "Linuxppc-dev" On Tue, 28 Jun 2022 16:03:12 +0200 Uwe Kleine-König wrote: > From: Uwe Kleine-König > > The value returned by an i2c driver's remove function is mostly ignored. > (Only an error message is printed if the value is non-zero that the > error is ignored.) > > So change the prototype of the remove function to return no value. This > way driver authors are not tempted to assume that passing an error to > the upper layer is a good idea. All drivers are adapted accordingly. > There is no intended change of behaviour, all callbacks were prepared to > return 0 before. > > Signed-off-by: Uwe Kleine-König For > drivers/leds/leds-turris-omnia.c | 4 +--- Acked-by: Marek Behún