From: George Stark <gnstark@salutedevices.com>
To: <andy.shevchenko@gmail.com>, <pavel@ucw.cz>, <lee@kernel.org>,
<vadimp@nvidia.com>, <mpe@ellerman.id.au>, <npiggin@gmail.com>,
<christophe.leroy@csgroup.eu>, <hdegoede@redhat.com>,
<mazziesaccount@gmail.com>, <peterz@infradead.org>,
<mingo@redhat.com>, <will@kernel.org>, <longman@redhat.com>,
<boqun.feng@gmail.com>, <nikitos.tr@gmail.com>,
<kabel@kernel.org>
Cc: <linux-leds@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
<linuxppc-dev@lists.ozlabs.org>, <kernel@salutedevices.com>,
George Stark <gnstark@salutedevices.com>
Subject: [PATCH v5 00/10] devm_led_classdev_register() usage problem
Date: Thu, 7 Mar 2024 05:40:24 +0300 [thread overview]
Message-ID: <20240307024034.1548605-1-gnstark@salutedevices.com> (raw)
This patch series fixes the problem of devm_led_classdev_register misusing.
The basic problem is described in [1]. Shortly when devm_led_classdev_register()
is used then led_classdev_unregister() called after driver's remove() callback.
led_classdev_unregister() calls driver's brightness_set callback and that callback
may use resources which were destroyed already in driver's remove().
After discussion with maintainers [2] [3] we decided:
1) don't touch led subsytem core code and don't remove led_set_brightness() from it
but fix drivers
2) don't use devm_led_classdev_unregister
So the solution is to use devm wrappers for all resources
driver's brightness_set() depends on. And introduce dedicated devm wrapper
for mutex as it's often used resource.
[1] https://lore.kernel.org/lkml/8704539b-ed3b-44e6-aa82-586e2f895e2b@salutedevices.com/T/
[2] https://lore.kernel.org/lkml/8704539b-ed3b-44e6-aa82-586e2f895e2b@salutedevices.com/T/#mc132b9b350fa51931b4fcfe14705d9f06e91421f
[3] https://lore.kernel.org/lkml/8704539b-ed3b-44e6-aa82-586e2f895e2b@salutedevices.com/T/#mdbf572a85c33f869a553caf986b6228bb65c8383
Changelog:
v1->v2:
revise patch series completely
v2->v3:
locking: add define if mutex_destroy() is not an empty function
new patch, discussed here [8]
devm-helpers: introduce devm_mutex_init
previous version [4]
- revise code based on mutex_destroy define
- update commit message
- update devm_mutex_init()'s description
leds: aw2013: unlock mutex before destroying it
previous version [5]
- make this patch first in the series
- add tags Fixes and RvB by Andy
leds: aw2013: use devm API to cleanup module's resources
previous version [6]
- make aw2013_chip_disable_action()'s body oneline
- don't shadow devm_mutex_init() return code
leds: aw200xx: use devm API to cleanup module's resources
previous version [7]
- make aw200xx_*_action()'s bodies oneline
- don't shadow devm_mutex_init() return code
leds: lm3532: use devm API to cleanup module's resources
leds: nic78bx: use devm API to cleanup module's resources
leds: mlxreg: use devm_mutex_init for mutex initializtion
leds: an30259a: use devm_mutext_init for mutext initialization
leds: powernv: add LED_RETAIN_AT_SHUTDOWN flag for leds
- those pathes were planned but not sent in the series #2 due to mail server
problem on my side. I revised them according to the comments.
v3->v4:
locking: introduce devm_mutex_init
new patch
- move devm_mutex_init implementation completely from devm-helpers.h to mutex.h
locking: add define if mutex_destroy() is not an empty function
drop the patch [9]
devm-helpers: introduce devm_mutex_init
drop the patch [10]
leds: aw2013: use devm API to cleanup module's resources
- add tag Tested-by: Nikita Travkin <nikita@trvn.ru>
v4->v5:
leds: aw2013: unlock mutex before destroying it
merged separately and removed from the series
locking/mutex: move mutex_destroy() definition lower
introduce optional refactoring patch
locking/mutex: introduce devm_mutex_init
leave only one devm_mutex_init definition
add tag Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
leds* patches
remove #include <linux/devm-helpers.h> due to devm_mutext_init in mutex.h now
[4] https://lore.kernel.org/lkml/20231204180603.470421-1-gnstark@salutedevices.com/T/#mf500af0eda2a9ffc95594607dbe4cb64f2e3c9a8
[5] https://lore.kernel.org/lkml/20231204180603.470421-1-gnstark@salutedevices.com/T/#mc92df4fb4f7d4187fb01cc1144acfa5fb5230dd2
[6] https://lore.kernel.org/lkml/20231204180603.470421-1-gnstark@salutedevices.com/T/#m300df89710c43cc2ab598baa16c68dd0a0d7d681
[7] https://lore.kernel.org/lkml/20231204180603.470421-1-gnstark@salutedevices.com/T/#m8e5c65e0c6b137c91fa00bb9320ad581164d1d0b
[8] https://lore.kernel.org/lkml/377e4437-7051-4d88-ae68-1460bcd692e1@redhat.com/T/#m5f84a4a2f387d49678783e652b9e658e02c27450
[9] https://lore.kernel.org/lkml/20231213223020.2713164-1-gnstark@salutedevices.com/T/#m19ad1fc04c560012c1e27418e3156d0c9306dd84
[10] https://lore.kernel.org/lkml/20231213223020.2713164-1-gnstark@salutedevices.com/T/#m63126025f5d1bdcef69bcad50f2e58274d42e2d
George Stark (10):
locking/mutex: move mutex_destroy() definition lower
locking/mutex: introduce devm_mutex_init
leds: aw2013: use devm API to cleanup module's resources
leds: aw200xx: use devm API to cleanup module's resources
leds: lp3952: use devm API to cleanup module's resources
leds: lm3532: use devm API to cleanup module's resources
leds: nic78bx: use devm API to cleanup module's resources
leds: mlxreg: use devm_mutex_init for mutex initializtion
leds: an30259a: use devm_mutext_init for mutext initialization
leds: powernv: use LED_RETAIN_AT_SHUTDOWN flag for leds
drivers/leds/leds-an30259a.c | 14 ++++----------
drivers/leds/leds-aw200xx.c | 32 +++++++++++++++++++++-----------
drivers/leds/leds-aw2013.c | 25 +++++++++++++------------
drivers/leds/leds-lm3532.c | 29 +++++++++++++++++------------
drivers/leds/leds-lp3952.c | 21 +++++++++++----------
drivers/leds/leds-mlxreg.c | 14 +++++---------
drivers/leds/leds-nic78bx.c | 23 +++++++++++++----------
drivers/leds/leds-powernv.c | 23 ++++++++---------------
include/linux/mutex.h | 27 +++++++++++++++++++++++----
kernel/locking/mutex-debug.c | 22 ++++++++++++++++++++++
10 files changed, 137 insertions(+), 93 deletions(-)
--
2.25.1
WARNING: multiple messages have this Message-ID (diff)
From: George Stark <gnstark@salutedevices.com>
To: <andy.shevchenko@gmail.com>, <pavel@ucw.cz>, <lee@kernel.org>,
<vadimp@nvidia.com>, <mpe@ellerman.id.au>, <npiggin@gmail.com>,
<christophe.leroy@csgroup.eu>, <hdegoede@redhat.com>,
<mazziesaccount@gmail.com>, <peterz@infradead.org>,
<mingo@redhat.com>, <will@kernel.org>, <longman@redhat.com>,
<boqun.feng@gmail.com>, <nikitos.tr@gmail.com>,
<kabel@kernel.org>
Cc: kernel@salutedevices.com, linuxppc-dev@lists.ozlabs.org,
linux-kernel@vger.kernel.org, linux-leds@vger.kernel.org,
George Stark <gnstark@salutedevices.com>
Subject: [PATCH v5 00/10] devm_led_classdev_register() usage problem
Date: Thu, 7 Mar 2024 05:40:24 +0300 [thread overview]
Message-ID: <20240307024034.1548605-1-gnstark@salutedevices.com> (raw)
This patch series fixes the problem of devm_led_classdev_register misusing.
The basic problem is described in [1]. Shortly when devm_led_classdev_register()
is used then led_classdev_unregister() called after driver's remove() callback.
led_classdev_unregister() calls driver's brightness_set callback and that callback
may use resources which were destroyed already in driver's remove().
After discussion with maintainers [2] [3] we decided:
1) don't touch led subsytem core code and don't remove led_set_brightness() from it
but fix drivers
2) don't use devm_led_classdev_unregister
So the solution is to use devm wrappers for all resources
driver's brightness_set() depends on. And introduce dedicated devm wrapper
for mutex as it's often used resource.
[1] https://lore.kernel.org/lkml/8704539b-ed3b-44e6-aa82-586e2f895e2b@salutedevices.com/T/
[2] https://lore.kernel.org/lkml/8704539b-ed3b-44e6-aa82-586e2f895e2b@salutedevices.com/T/#mc132b9b350fa51931b4fcfe14705d9f06e91421f
[3] https://lore.kernel.org/lkml/8704539b-ed3b-44e6-aa82-586e2f895e2b@salutedevices.com/T/#mdbf572a85c33f869a553caf986b6228bb65c8383
Changelog:
v1->v2:
revise patch series completely
v2->v3:
locking: add define if mutex_destroy() is not an empty function
new patch, discussed here [8]
devm-helpers: introduce devm_mutex_init
previous version [4]
- revise code based on mutex_destroy define
- update commit message
- update devm_mutex_init()'s description
leds: aw2013: unlock mutex before destroying it
previous version [5]
- make this patch first in the series
- add tags Fixes and RvB by Andy
leds: aw2013: use devm API to cleanup module's resources
previous version [6]
- make aw2013_chip_disable_action()'s body oneline
- don't shadow devm_mutex_init() return code
leds: aw200xx: use devm API to cleanup module's resources
previous version [7]
- make aw200xx_*_action()'s bodies oneline
- don't shadow devm_mutex_init() return code
leds: lm3532: use devm API to cleanup module's resources
leds: nic78bx: use devm API to cleanup module's resources
leds: mlxreg: use devm_mutex_init for mutex initializtion
leds: an30259a: use devm_mutext_init for mutext initialization
leds: powernv: add LED_RETAIN_AT_SHUTDOWN flag for leds
- those pathes were planned but not sent in the series #2 due to mail server
problem on my side. I revised them according to the comments.
v3->v4:
locking: introduce devm_mutex_init
new patch
- move devm_mutex_init implementation completely from devm-helpers.h to mutex.h
locking: add define if mutex_destroy() is not an empty function
drop the patch [9]
devm-helpers: introduce devm_mutex_init
drop the patch [10]
leds: aw2013: use devm API to cleanup module's resources
- add tag Tested-by: Nikita Travkin <nikita@trvn.ru>
v4->v5:
leds: aw2013: unlock mutex before destroying it
merged separately and removed from the series
locking/mutex: move mutex_destroy() definition lower
introduce optional refactoring patch
locking/mutex: introduce devm_mutex_init
leave only one devm_mutex_init definition
add tag Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
leds* patches
remove #include <linux/devm-helpers.h> due to devm_mutext_init in mutex.h now
[4] https://lore.kernel.org/lkml/20231204180603.470421-1-gnstark@salutedevices.com/T/#mf500af0eda2a9ffc95594607dbe4cb64f2e3c9a8
[5] https://lore.kernel.org/lkml/20231204180603.470421-1-gnstark@salutedevices.com/T/#mc92df4fb4f7d4187fb01cc1144acfa5fb5230dd2
[6] https://lore.kernel.org/lkml/20231204180603.470421-1-gnstark@salutedevices.com/T/#m300df89710c43cc2ab598baa16c68dd0a0d7d681
[7] https://lore.kernel.org/lkml/20231204180603.470421-1-gnstark@salutedevices.com/T/#m8e5c65e0c6b137c91fa00bb9320ad581164d1d0b
[8] https://lore.kernel.org/lkml/377e4437-7051-4d88-ae68-1460bcd692e1@redhat.com/T/#m5f84a4a2f387d49678783e652b9e658e02c27450
[9] https://lore.kernel.org/lkml/20231213223020.2713164-1-gnstark@salutedevices.com/T/#m19ad1fc04c560012c1e27418e3156d0c9306dd84
[10] https://lore.kernel.org/lkml/20231213223020.2713164-1-gnstark@salutedevices.com/T/#m63126025f5d1bdcef69bcad50f2e58274d42e2d
George Stark (10):
locking/mutex: move mutex_destroy() definition lower
locking/mutex: introduce devm_mutex_init
leds: aw2013: use devm API to cleanup module's resources
leds: aw200xx: use devm API to cleanup module's resources
leds: lp3952: use devm API to cleanup module's resources
leds: lm3532: use devm API to cleanup module's resources
leds: nic78bx: use devm API to cleanup module's resources
leds: mlxreg: use devm_mutex_init for mutex initializtion
leds: an30259a: use devm_mutext_init for mutext initialization
leds: powernv: use LED_RETAIN_AT_SHUTDOWN flag for leds
drivers/leds/leds-an30259a.c | 14 ++++----------
drivers/leds/leds-aw200xx.c | 32 +++++++++++++++++++++-----------
drivers/leds/leds-aw2013.c | 25 +++++++++++++------------
drivers/leds/leds-lm3532.c | 29 +++++++++++++++++------------
drivers/leds/leds-lp3952.c | 21 +++++++++++----------
drivers/leds/leds-mlxreg.c | 14 +++++---------
drivers/leds/leds-nic78bx.c | 23 +++++++++++++----------
drivers/leds/leds-powernv.c | 23 ++++++++---------------
include/linux/mutex.h | 27 +++++++++++++++++++++++----
kernel/locking/mutex-debug.c | 22 ++++++++++++++++++++++
10 files changed, 137 insertions(+), 93 deletions(-)
--
2.25.1
next reply other threads:[~2024-03-07 2:40 UTC|newest]
Thread overview: 56+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-07 2:40 George Stark [this message]
2024-03-07 2:40 ` [PATCH v5 00/10] devm_led_classdev_register() usage problem George Stark
2024-03-07 2:40 ` [PATCH v5 01/10] locking/mutex: move mutex_destroy() definition lower George Stark
2024-03-07 2:40 ` George Stark
2024-03-07 2:40 ` [PATCH v5 02/10] locking/mutex: introduce devm_mutex_init George Stark
2024-03-07 2:40 ` George Stark
2024-03-07 9:56 ` Marek Behún
2024-03-07 9:56 ` Marek Behún
2024-03-07 13:39 ` Waiman Long
2024-03-07 13:39 ` Waiman Long
2024-03-07 16:44 ` Marek Behún
2024-03-07 16:44 ` Marek Behún
2024-03-11 23:47 ` George Stark
2024-03-11 23:47 ` George Stark
2024-03-12 1:10 ` Waiman Long
2024-03-12 1:10 ` Waiman Long
2024-03-12 5:44 ` Christophe Leroy
2024-03-12 5:44 ` Christophe Leroy
2024-03-12 6:04 ` Christophe Leroy
2024-03-12 6:04 ` Christophe Leroy
2024-03-12 11:39 ` George Stark
2024-03-12 11:39 ` George Stark
2024-03-12 11:51 ` Christophe Leroy
2024-03-12 11:51 ` Christophe Leroy
2024-03-12 15:30 ` George Stark
2024-03-12 15:30 ` George Stark
2024-03-12 18:17 ` Christophe Leroy
2024-03-12 18:17 ` Christophe Leroy
2024-03-07 10:34 ` Andy Shevchenko
2024-03-07 10:34 ` Andy Shevchenko
2024-03-12 0:01 ` George Stark
2024-03-12 0:01 ` George Stark
2024-03-12 5:41 ` Christophe Leroy
2024-03-12 5:41 ` Christophe Leroy
2024-03-12 8:58 ` Andy Shevchenko
2024-03-12 8:58 ` Andy Shevchenko
2024-03-07 13:50 ` Christophe Leroy
2024-03-07 13:50 ` Christophe Leroy
2024-03-11 23:31 ` George Stark
2024-03-11 23:31 ` George Stark
2024-03-07 2:40 ` [PATCH v5 03/10] leds: aw2013: use devm API to cleanup module's resources George Stark
2024-03-07 2:40 ` George Stark
2024-03-07 2:40 ` [PATCH v5 04/10] leds: aw200xx: " George Stark
2024-03-07 2:40 ` George Stark
2024-03-07 2:40 ` [PATCH v5 05/10] leds: lp3952: " George Stark
2024-03-07 2:40 ` George Stark
2024-03-07 2:40 ` [PATCH v5 06/10] leds: lm3532: " George Stark
2024-03-07 2:40 ` George Stark
2024-03-07 2:40 ` [PATCH v5 07/10] leds: nic78bx: " George Stark
2024-03-07 2:40 ` George Stark
2024-03-07 2:40 ` [PATCH v5 08/10] leds: mlxreg: use devm_mutex_init for mutex initializtion George Stark
2024-03-07 2:40 ` George Stark
2024-03-07 2:40 ` [PATCH v5 09/10] leds: an30259a: use devm_mutext_init for mutext initialization George Stark
2024-03-07 2:40 ` George Stark
2024-03-07 2:40 ` [PATCH v5 10/10] leds: powernv: use LED_RETAIN_AT_SHUTDOWN flag for leds George Stark
2024-03-07 2:40 ` George Stark
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240307024034.1548605-1-gnstark@salutedevices.com \
--to=gnstark@salutedevices.com \
--cc=andy.shevchenko@gmail.com \
--cc=boqun.feng@gmail.com \
--cc=christophe.leroy@csgroup.eu \
--cc=hdegoede@redhat.com \
--cc=kabel@kernel.org \
--cc=kernel@salutedevices.com \
--cc=lee@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-leds@vger.kernel.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=longman@redhat.com \
--cc=mazziesaccount@gmail.com \
--cc=mingo@redhat.com \
--cc=mpe@ellerman.id.au \
--cc=nikitos.tr@gmail.com \
--cc=npiggin@gmail.com \
--cc=pavel@ucw.cz \
--cc=peterz@infradead.org \
--cc=vadimp@nvidia.com \
--cc=will@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.