All of lore.kernel.org
 help / color / mirror / Atom feed
From: Lee Jones <lee@kernel.org>
To: Hans de Goede <hdegoede@redhat.com>
Cc: Pavel Machek <pavel@ucw.cz>,
	Jacek Anaszewski <jacek.anaszewski@gmail.com>,
	Sebastian Reichel <sre@kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Michael Grzeschik <m.grzeschik@pengutronix.de>,
	Johannes Berg <johannes@sipsolutions.net>,
	Pablo Neira Ayuso <pablo@netfilter.org>,
	Jozsef Kadlecsik <kadlec@netfilter.org>,
	linux-leds@vger.kernel.org, linux-pm@vger.kernel.org,
	linux-usb@vger.kernel.org, netdev@vger.kernel.org,
	netfilter-devel@vger.kernel.org, coreteam@netfilter.org,
	Yauhen Kharuzhy <jekhor@gmail.com>
Subject: Re: [PATCH RESEND 2/4] leds: Fix set_brightness_delayed() race
Date: Thu, 18 May 2023 13:13:11 +0100	[thread overview]
Message-ID: <20230518121311.GH404509@google.com> (raw)
In-Reply-To: <20230510162234.291439-3-hdegoede@redhat.com>

On Wed, 10 May 2023, Hans de Goede wrote:

> When a trigger wants to switch from blinking to LED on it needs to call:
> 	led_set_brightness(LED_OFF);
> 	led_set_brightness(LED_FULL);
> 
> To first call disables blinking and the second then turns the LED on
> (the power-supply charging-blink-full-solid triggers do this).
> 
> These calls happen immediately after each other, so it is possible
> that set_brightness_delayed() from the first call has not run yet
> when the led_set_brightness(LED_FULL) call finishes.
> 
> If this race hits then this is causing problems for both
> sw- and hw-blinking:
> 
> For sw-blinking set_brightness_delayed() clears delayed_set_value
> when LED_BLINK_DISABLE is set causing the led_set_brightness(LED_FULL)
> call effects to get lost when hitting the race, resulting in the LED
> turning off instead of on.
> 
> For hw-blinking if the race hits delayed_set_value has been
> set to LED_FULL by the time set_brightness_delayed() runs.
> So led_cdev->brightness_set_blocking() is never called with
> LED_OFF as argument and the hw-blinking is never disabled leaving
> the LED blinking instead of on.
> 
> Fix both issues by adding LED_SET_BRIGHTNESS and LED_SET_BRIGHTNESS_OFF
> work_flags making this 2 separate actions to be run by
> set_brightness_delayed().
> 
> Reviewed-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
> Tested-by: Yauhen Kharuzhy <jekhor@gmail.com>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/leds/led-core.c | 57 +++++++++++++++++++++++++++++++----------
>  include/linux/leds.h    |  3 +++
>  2 files changed, 47 insertions(+), 13 deletions(-)

Applied, thanks

-- 
Lee Jones [李琼斯]

  reply	other threads:[~2023-05-18 12:13 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-10 16:22 [PATCH RESEND 0/4] Fix oops about sleeping in led_trigger_blink() Hans de Goede
2023-05-10 16:22 ` [PATCH RESEND 1/4] leds: Change led_trigger_blink[_oneshot]() delay parameters to pass-by-value Hans de Goede
2023-05-10 18:19   ` Sebastian Reichel
2023-05-10 21:43   ` Florian Westphal
2023-05-10 22:53   ` Greg Kroah-Hartman
2023-05-18 12:12   ` Lee Jones
2023-05-10 16:22 ` [PATCH RESEND 2/4] leds: Fix set_brightness_delayed() race Hans de Goede
2023-05-18 12:13   ` Lee Jones [this message]
2023-05-10 16:22 ` [PATCH RESEND 3/4] leds: Fix oops about sleeping in led_trigger_blink() Hans de Goede
2023-05-18 12:13   ` Lee Jones
2023-05-10 16:22 ` [PATCH RESEND 4/4] leds: Clear LED_INIT_DEFAULT_TRIGGER when clearing current trigger Hans de Goede
2023-05-18 12:14   ` Lee Jones

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=20230518121311.GH404509@google.com \
    --to=lee@kernel.org \
    --cc=coreteam@netfilter.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=hdegoede@redhat.com \
    --cc=jacek.anaszewski@gmail.com \
    --cc=jekhor@gmail.com \
    --cc=johannes@sipsolutions.net \
    --cc=kadlec@netfilter.org \
    --cc=linux-leds@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=m.grzeschik@pengutronix.de \
    --cc=netdev@vger.kernel.org \
    --cc=netfilter-devel@vger.kernel.org \
    --cc=pablo@netfilter.org \
    --cc=pavel@ucw.cz \
    --cc=sre@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.