linux-leds.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
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>
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 v4 00/10] devm_led_classdev_register() usage problem
Date: Thu, 14 Dec 2023 20:36:04 +0300	[thread overview]
Message-ID: <20231214173614.2820929-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>

[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/#m63126025f5d1bdcef69bcad50f2e58274d42e2d7

George Stark (10):
  leds: aw2013: unlock mutex before destroying it
  locking: 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 | 15 +++++----------
 drivers/leds/leds-aw200xx.c  | 33 ++++++++++++++++++++++-----------
 drivers/leds/leds-aw2013.c   | 27 +++++++++++++++------------
 drivers/leds/leds-lm3532.c   | 30 ++++++++++++++++++------------
 drivers/leds/leds-lp3952.c   | 21 +++++++++++----------
 drivers/leds/leds-mlxreg.c   | 17 ++++++-----------
 drivers/leds/leds-nic78bx.c  | 25 +++++++++++++------------
 drivers/leds/leds-powernv.c  | 23 ++++++++---------------
 include/linux/mutex.h        | 23 +++++++++++++++++++++++
 kernel/locking/mutex-debug.c | 22 ++++++++++++++++++++++
 10 files changed, 143 insertions(+), 93 deletions(-)

-- 
2.25.1


             reply	other threads:[~2023-12-14 17:36 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-14 17:36 George Stark [this message]
2023-12-14 17:36 ` [PATCH v4 01/10] leds: aw2013: unlock mutex before destroying it George Stark
2024-02-23 15:07   ` (subset) " Lee Jones
2023-12-14 17:36 ` [PATCH v4 02/10] locking: introduce devm_mutex_init George Stark
2023-12-14 18:48   ` Waiman Long
2023-12-14 19:53     ` Christophe Leroy
2023-12-14 21:48       ` Waiman Long
2023-12-15  5:46         ` Christophe Leroy
2023-12-17  1:05           ` George Stark
2023-12-17  9:31             ` Christophe Leroy
2023-12-18 13:26               ` George Stark
2023-12-14 19:47   ` Christophe Leroy
2023-12-15  6:22   ` [PATCH RFC v4-bis] " Christophe Leroy
2023-12-15 15:58     ` Andy Shevchenko
2023-12-15 17:51       ` Christophe Leroy
2023-12-16  1:30       ` Waiman Long
2023-12-14 17:36 ` [PATCH v4 03/10] leds: aw2013: use devm API to cleanup module's resources George Stark
2023-12-14 17:36 ` [PATCH v4 04/10] leds: aw200xx: " George Stark
2023-12-14 17:36 ` [PATCH v4 05/10] leds: lp3952: " George Stark
2023-12-14 17:36 ` [PATCH v4 06/10] leds: lm3532: " George Stark
2023-12-14 17:36 ` [PATCH v4 07/10] leds: nic78bx: " George Stark
2023-12-14 17:36 ` [PATCH v4 08/10] leds: mlxreg: use devm_mutex_init for mutex initializtion George Stark
2023-12-14 17:36 ` [PATCH v4 09/10] leds: an30259a: use devm_mutext_init for mutext initialization George Stark
2023-12-14 17:36 ` [PATCH v4 10/10] leds: powernv: use LED_RETAIN_AT_SHUTDOWN flag for leds George Stark
2023-12-21 15:11 ` [PATCH v4 00/10] devm_led_classdev_register() usage problem Lee Jones
2024-02-09 17:11   ` Andy Shevchenko
2024-02-11 23:52     ` [DMARC error][SPF error] " George Stark
2024-02-12  9:53       ` Andy Shevchenko
2024-02-13  0:14         ` George Stark
2024-02-13 10:55           ` Andy Shevchenko
2024-02-13 23:56             ` George Stark
2024-02-09 17:17 ` Andy Shevchenko

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=20231214173614.2820929-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=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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).