From: Andy Shevchenko <andy.shevchenko@gmail.com>
To: Lee Jones <lee@kernel.org>
Cc: George Stark <gnstark@salutedevices.com>,
pavel@ucw.cz, 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,
linux-leds@vger.kernel.org, linux-kernel@vger.kernel.org,
linuxppc-dev@lists.ozlabs.org, kernel@salutedevices.com
Subject: Re: [PATCH v4 00/10] devm_led_classdev_register() usage problem
Date: Fri, 9 Feb 2024 19:11:01 +0200 [thread overview]
Message-ID: <ZcZcpUHygltD2ETa@smile.fi.intel.com> (raw)
In-Reply-To: <20231221151111.GJ10102@google.com>
On Thu, Dec 21, 2023 at 03:11:11PM +0000, Lee Jones wrote:
> On Thu, 14 Dec 2023, George Stark wrote:
>
> > 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
...
> FYI: I'll conduct my review once the locking side is settled.
To reduce burden can you apply the first one? It's a fix.
--
With Best Regards,
Andy Shevchenko
WARNING: multiple messages have this Message-ID (diff)
From: Andy Shevchenko <andy.shevchenko@gmail.com>
To: Lee Jones <lee@kernel.org>
Cc: kernel@salutedevices.com, vadimp@nvidia.com,
mazziesaccount@gmail.com, peterz@infradead.org,
boqun.feng@gmail.com, linuxppc-dev@lists.ozlabs.org,
linux-kernel@vger.kernel.org, hdegoede@redhat.com,
mingo@redhat.com, npiggin@gmail.com, pavel@ucw.cz,
George Stark <gnstark@salutedevices.com>,
longman@redhat.com, nikitos.tr@gmail.com, will@kernel.org,
linux-leds@vger.kernel.org
Subject: Re: [PATCH v4 00/10] devm_led_classdev_register() usage problem
Date: Fri, 9 Feb 2024 19:11:01 +0200 [thread overview]
Message-ID: <ZcZcpUHygltD2ETa@smile.fi.intel.com> (raw)
In-Reply-To: <20231221151111.GJ10102@google.com>
On Thu, Dec 21, 2023 at 03:11:11PM +0000, Lee Jones wrote:
> On Thu, 14 Dec 2023, George Stark wrote:
>
> > 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
...
> FYI: I'll conduct my review once the locking side is settled.
To reduce burden can you apply the first one? It's a fix.
--
With Best Regards,
Andy Shevchenko
next prev parent reply other threads:[~2024-02-09 17:12 UTC|newest]
Thread overview: 63+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-12-14 17:36 [PATCH v4 00/10] devm_led_classdev_register() usage problem George Stark
2023-12-14 17:36 ` George Stark
2023-12-14 17:36 ` [PATCH v4 01/10] leds: aw2013: unlock mutex before destroying it George Stark
2023-12-14 17:36 ` George Stark
2024-02-23 15:07 ` (subset) " Lee Jones
2024-02-23 15:07 ` Lee Jones
2023-12-14 17:36 ` [PATCH v4 02/10] locking: introduce devm_mutex_init George Stark
2023-12-14 17:36 ` George Stark
2023-12-14 18:48 ` Waiman Long
2023-12-14 18:48 ` Waiman Long
2023-12-14 19:53 ` Christophe Leroy
2023-12-14 19:53 ` Christophe Leroy
2023-12-14 21:48 ` Waiman Long
2023-12-14 21:48 ` Waiman Long
2023-12-15 5:46 ` Christophe Leroy
2023-12-15 5:46 ` Christophe Leroy
2023-12-17 1:05 ` George Stark
2023-12-17 1:05 ` George Stark
2023-12-17 9:31 ` Christophe Leroy
2023-12-17 9:31 ` Christophe Leroy
2023-12-18 13:26 ` George Stark
2023-12-18 13:26 ` George Stark
2023-12-14 19:47 ` Christophe Leroy
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 15:58 ` Andy Shevchenko
2023-12-15 17:51 ` Christophe Leroy
2023-12-15 17:51 ` Christophe Leroy
2023-12-16 1:30 ` Waiman Long
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 ` George Stark
2023-12-14 17:36 ` [PATCH v4 04/10] leds: aw200xx: " George Stark
2023-12-14 17:36 ` George Stark
2023-12-14 17:36 ` [PATCH v4 05/10] leds: lp3952: " George Stark
2023-12-14 17:36 ` George Stark
2023-12-14 17:36 ` [PATCH v4 06/10] leds: lm3532: " George Stark
2023-12-14 17:36 ` George Stark
2023-12-14 17:36 ` [PATCH v4 07/10] leds: nic78bx: " George Stark
2023-12-14 17:36 ` 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 ` 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 ` 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-14 17:36 ` George Stark
2023-12-21 15:11 ` [PATCH v4 00/10] devm_led_classdev_register() usage problem Lee Jones
2023-12-21 15:11 ` Lee Jones
2024-02-09 17:11 ` Andy Shevchenko [this message]
2024-02-09 17:11 ` Andy Shevchenko
2024-02-11 23:52 ` [DMARC error][SPF error] " George Stark
2024-02-11 23:52 ` George Stark
2024-02-12 9:53 ` Andy Shevchenko
2024-02-12 9:53 ` Andy Shevchenko
2024-02-13 0:14 ` George Stark
2024-02-13 0:14 ` George Stark
2024-02-13 10:55 ` Andy Shevchenko
2024-02-13 10:55 ` Andy Shevchenko
2024-02-13 23:56 ` George Stark
2024-02-13 23:56 ` George Stark
2024-02-09 17:17 ` Andy Shevchenko
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=ZcZcpUHygltD2ETa@smile.fi.intel.com \
--to=andy.shevchenko@gmail.com \
--cc=boqun.feng@gmail.com \
--cc=christophe.leroy@csgroup.eu \
--cc=gnstark@salutedevices.com \
--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 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.