| Message ID | 20240502211425.8678-4-hdegoede@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | KTD2026 indicator LED for X86 Xiaomi Pad2 | expand |
On Fri, May 3, 2024 at 12:14 AM Hans de Goede <hdegoede@redhat.com> wrote: > > The mutex must be initialized before the LED class device is registered > otherwise there is a race where it may get used before it is initialized: > > DEBUG_LOCKS_WARN_ON(lock->magic != lock) > WARNING: CPU: 2 PID: 2045 at kernel/locking/mutex.c:587 __mutex_lock > ... > RIP: 0010:__mutex_lock+0x7db/0xc10 > ... > set_brightness_delayed_set_brightness.part.0+0x17/0x60 > set_brightness_delayed+0xf1/0x100 > process_one_work+0x222/0x5a0 ... > + mutex_init(&chip->mutex); devm_mutex_init() ? ... There is an immutable branch (in case of this series going behind LED subsystem): https://git.kernel.org/pub/scm/linux/kernel/git/lee/leds.git/log/?h=ib-leds-locking-6.10
Hi, On 5/3/24 5:43 AM, Andy Shevchenko wrote: > On Fri, May 3, 2024 at 12:14 AM Hans de Goede <hdegoede@redhat.com> wrote: >> >> The mutex must be initialized before the LED class device is registered >> otherwise there is a race where it may get used before it is initialized: >> >> DEBUG_LOCKS_WARN_ON(lock->magic != lock) >> WARNING: CPU: 2 PID: 2045 at kernel/locking/mutex.c:587 __mutex_lock >> ... >> RIP: 0010:__mutex_lock+0x7db/0xc10 >> ... >> set_brightness_delayed_set_brightness.part.0+0x17/0x60 >> set_brightness_delayed+0xf1/0x100 >> process_one_work+0x222/0x5a0 > > ... > >> + mutex_init(&chip->mutex); > > devm_mutex_init() ? That is not in Torvald's tree yet. Regards, Hans
On Fri, 03 May 2024, Hans de Goede wrote: > Hi, > > On 5/3/24 5:43 AM, Andy Shevchenko wrote: > > On Fri, May 3, 2024 at 12:14 AM Hans de Goede <hdegoede@redhat.com> wrote: > >> > >> The mutex must be initialized before the LED class device is registered > >> otherwise there is a race where it may get used before it is initialized: > >> > >> DEBUG_LOCKS_WARN_ON(lock->magic != lock) > >> WARNING: CPU: 2 PID: 2045 at kernel/locking/mutex.c:587 __mutex_lock > >> ... > >> RIP: 0010:__mutex_lock+0x7db/0xc10 > >> ... > >> set_brightness_delayed_set_brightness.part.0+0x17/0x60 > >> set_brightness_delayed+0xf1/0x100 > >> process_one_work+0x222/0x5a0 > > > > ... > > > >> + mutex_init(&chip->mutex); > > > > devm_mutex_init() ? > > That is not in Torvald's tree yet. Neither is this. :) Since we're nearly at -rc7, I think it's safe to say you have time.
Hi, On 5/3/24 9:03 AM, Lee Jones wrote: > On Fri, 03 May 2024, Hans de Goede wrote: > >> Hi, >> >> On 5/3/24 5:43 AM, Andy Shevchenko wrote: >>> On Fri, May 3, 2024 at 12:14 AM Hans de Goede <hdegoede@redhat.com> wrote: >>>> >>>> The mutex must be initialized before the LED class device is registered >>>> otherwise there is a race where it may get used before it is initialized: >>>> >>>> DEBUG_LOCKS_WARN_ON(lock->magic != lock) >>>> WARNING: CPU: 2 PID: 2045 at kernel/locking/mutex.c:587 __mutex_lock >>>> ... >>>> RIP: 0010:__mutex_lock+0x7db/0xc10 >>>> ... >>>> set_brightness_delayed_set_brightness.part.0+0x17/0x60 >>>> set_brightness_delayed+0xf1/0x100 >>>> process_one_work+0x222/0x5a0 >>> >>> ... >>> >>>> + mutex_init(&chip->mutex); >>> >>> devm_mutex_init() ? >> >> That is not in Torvald's tree yet. > > Neither is this. :) > > Since we're nearly at -rc7, I think it's safe to say you have time. Ok I'll prepare a v9 with this addressed and Andy's Reviewed-by added. Regards, Hans
diff --git a/drivers/leds/rgb/leds-ktd202x.c b/drivers/leds/rgb/leds-ktd202x.c index 60ca6ec34336..77247a98fe66 100644 --- a/drivers/leds/rgb/leds-ktd202x.c +++ b/drivers/leds/rgb/leds-ktd202x.c @@ -572,21 +572,25 @@ static int ktd202x_probe(struct i2c_client *client) return ret; } + mutex_init(&chip->mutex); + ret = ktd202x_probe_fw(chip); if (ret < 0) { regulator_bulk_disable(ARRAY_SIZE(chip->regulators), chip->regulators); - return ret; + goto destroy_mutex; } ret = regulator_bulk_disable(ARRAY_SIZE(chip->regulators), chip->regulators); if (ret) { dev_err_probe(dev, ret, "Failed to disable regulators.\n"); - return ret; + goto destroy_mutex; } - mutex_init(&chip->mutex); - return 0; + +destroy_mutex: + mutex_destroy(&chip->mutex); + return ret; } static void ktd202x_remove(struct i2c_client *client)
The mutex must be initialized before the LED class device is registered otherwise there is a race where it may get used before it is initialized: DEBUG_LOCKS_WARN_ON(lock->magic != lock) WARNING: CPU: 2 PID: 2045 at kernel/locking/mutex.c:587 __mutex_lock ... RIP: 0010:__mutex_lock+0x7db/0xc10 ... set_brightness_delayed_set_brightness.part.0+0x17/0x60 set_brightness_delayed+0xf1/0x100 process_one_work+0x222/0x5a0 Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- drivers/leds/rgb/leds-ktd202x.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-)