[PATCH] leds: core: Fix race condition for software blink

[PATCH] leds: core: Fix race condition for software blink

[Craig McQueen]

led_set_brightness() function: Change handling of software blink to
avoid race conditions when stopping blink and setting brightness.

Triggers may call led_set_brightness(LED_OFF),
led_set_brightness(LED_FULL) in quick succession to disable blinking and
turn the LED on. If the delayed work task has not yet disabled blinking
by the time the second call occurs, then the brightness also needs to be
changed in the delayed work task.

Signed-off-by: Craig McQueen <craig@mcqueen.au>
---
 drivers/leds/led-core.c | 33 ++++++++++++++++++++-------------
 1 file changed, 20 insertions(+), 13 deletions(-)

diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
index 385e78af1dac..073c547068cc 100644
--- a/drivers/leds/led-core.c
+++ b/drivers/leds/led-core.c
@@ -303,24 +303,31 @@ EXPORT_SYMBOL_GPL(led_stop_software_blink);
 
 void led_set_brightness(struct led_classdev *led_cdev, unsigned int brightness)
 {
-	/*
-	 * If software blink is active, delay brightness setting
-	 * until the next timer tick.
-	 */
-	if (test_bit(LED_BLINK_SW, &led_cdev->work_flags)) {
+	if (brightness) {
 		/*
-		 * If we need to disable soft blinking delegate this to the
-		 * work queue task to avoid problems in case we are called
-		 * from hard irq context.
+		 * If software blink disable is pending, also queue brightness setting.
+		 * If software blink is active, delay brightness setting
+		 * until the next timer tick.
 		 */
-		if (!brightness) {
-			set_bit(LED_BLINK_DISABLE, &led_cdev->work_flags);
+		if (test_bit(LED_SET_BRIGHTNESS, &led_cdev->work_flags) ||
+		    test_bit(LED_BLINK_DISABLE, &led_cdev->work_flags)) {
+			led_cdev->delayed_set_value = brightness;
+			set_bit(LED_SET_BRIGHTNESS, &led_cdev->work_flags);
 			queue_work(led_cdev->wq, &led_cdev->set_brightness_work);
-		} else {
-			set_bit(LED_BLINK_BRIGHTNESS_CHANGE,
-				&led_cdev->work_flags);
+			return;
+		} else if (test_bit(LED_BLINK_SW, &led_cdev->work_flags)) {
 			led_cdev->new_blink_brightness = brightness;
+			set_bit(LED_BLINK_BRIGHTNESS_CHANGE, &led_cdev->work_flags);
+			return;
 		}
+	} else if (test_bit(LED_BLINK_SW, &led_cdev->work_flags)) {
+		/*
+		 * If we need to disable soft blinking delegate this to the
+		 * work queue task to avoid problems in case we are called
+		 * from hard irq context.
+		 */
+		set_bit(LED_BLINK_DISABLE, &led_cdev->work_flags);
+		queue_work(led_cdev->wq, &led_cdev->set_brightness_work);
 		return;
 	}
 
-- 
2.52.0

Re: [PATCH] leds: core: Fix race condition for software blink

[Lee Jones]

On Thu, 23 Apr 2026, Craig McQueen wrote:

> led_set_brightness() function: Change handling of software blink to
> avoid race conditions when stopping blink and setting brightness.
> 
> Triggers may call led_set_brightness(LED_OFF),
> led_set_brightness(LED_FULL) in quick succession to disable blinking and
> turn the LED on. If the delayed work task has not yet disabled blinking
> by the time the second call occurs, then the brightness also needs to be
> changed in the delayed work task.
> 
> Signed-off-by: Craig McQueen <craig@mcqueen.au>
> ---
>  drivers/leds/led-core.c | 33 ++++++++++++++++++++-------------
>  1 file changed, 20 insertions(+), 13 deletions(-)

I think this needs more eyes on it and to be thoroughly tested by others.

> diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
> index 385e78af1dac..073c547068cc 100644
> --- a/drivers/leds/led-core.c
> +++ b/drivers/leds/led-core.c
> @@ -303,24 +303,31 @@ EXPORT_SYMBOL_GPL(led_stop_software_blink);
>  
>  void led_set_brightness(struct led_classdev *led_cdev, unsigned int brightness)
>  {
> -	/*
> -	 * If software blink is active, delay brightness setting
> -	 * until the next timer tick.
> -	 */
> -	if (test_bit(LED_BLINK_SW, &led_cdev->work_flags)) {
> +	if (brightness) {
>  		/*
> -		 * If we need to disable soft blinking delegate this to the
> -		 * work queue task to avoid problems in case we are called
> -		 * from hard irq context.
> +		 * If software blink disable is pending, also queue brightness setting.
> +		 * If software blink is active, delay brightness setting
> +		 * until the next timer tick.
>  		 */
> -		if (!brightness) {
> -			set_bit(LED_BLINK_DISABLE, &led_cdev->work_flags);
> +		if (test_bit(LED_SET_BRIGHTNESS, &led_cdev->work_flags) ||
> +		    test_bit(LED_BLINK_DISABLE, &led_cdev->work_flags)) {
> +			led_cdev->delayed_set_value = brightness;
> +			set_bit(LED_SET_BRIGHTNESS, &led_cdev->work_flags);
>  			queue_work(led_cdev->wq, &led_cdev->set_brightness_work);
> -		} else {
> -			set_bit(LED_BLINK_BRIGHTNESS_CHANGE,
> -				&led_cdev->work_flags);
> +			return;
> +		} else if (test_bit(LED_BLINK_SW, &led_cdev->work_flags)) {
>  			led_cdev->new_blink_brightness = brightness;
> +			set_bit(LED_BLINK_BRIGHTNESS_CHANGE, &led_cdev->work_flags);
> +			return;
>  		}
> +	} else if (test_bit(LED_BLINK_SW, &led_cdev->work_flags)) {
> +		/*
> +		 * If we need to disable soft blinking delegate this to the
> +		 * work queue task to avoid problems in case we are called
> +		 * from hard irq context.
> +		 */
> +		set_bit(LED_BLINK_DISABLE, &led_cdev->work_flags);
> +		queue_work(led_cdev->wq, &led_cdev->set_brightness_work);
>  		return;
>  	}
>  
> -- 
> 2.52.0
> 
> 

-- 
Lee Jones

Re: [PATCH] leds: core: Fix race condition for software blink

[Craig McQueen]

Lee Jones wrote
 > On Thu, 23 Apr 2026, Craig McQueen wrote: 
 >  
 > > led_set_brightness() function: Change handling of software blink to 
 > > avoid race conditions when stopping blink and setting brightness. 
 > > 
 > > Triggers may call led_set_brightness(LED_OFF), 
 > > led_set_brightness(LED_FULL) in quick succession to disable blinking and 
 > > turn the LED on. If the delayed work task has not yet disabled blinking 
 > > by the time the second call occurs, then the brightness also needs to be 
 > > changed in the delayed work task. 
 > > 
 > > Signed-off-by: Craig McQueen <craig@mcqueen.au> 
 > > --- 
 > >  drivers/leds/led-core.c | 33 ++++++++++++++++++++------------- 
 > >  1 file changed, 20 insertions(+), 13 deletions(-) 
 >  
 > I think this needs more eyes on it and to be thoroughly tested by others. 

The problem I have seen is when a Linux driver controls a trigger to change it from flashing to steady-on. I've seen it specifically with LED core's software blinking.

/* First stop the blinking */
led_trigger_event(&udev->led_trigger, LED_OFF);
/* Next turn the LED steady-on */
led_trigger_event(&udev->led_trigger, LED_FULL);

Without this patch, the LED usually ends up in the off state, rather than on.

I have tested this patch on a Rockchip RK3328 based system (ARM 64).

 > > diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c 
 > > index 385e78af1dac..073c547068cc 100644 
 > > --- a/drivers/leds/led-core.c 
 > > +++ b/drivers/leds/led-core.c 
 > > @@ -303,24 +303,31 @@ EXPORT_SYMBOL_GPL(led_stop_software_blink); 
 > > 
 > >  void led_set_brightness(struct led_classdev *led_cdev, unsigned int brightness) 
 > >  { 
 > > -    /* 
 > > -     * If software blink is active, delay brightness setting 
 > > -     * until the next timer tick. 
 > > -     */ 
 > > -    if (test_bit(LED_BLINK_SW, &led_cdev->work_flags)) { 
 > > +    if (brightness) { 
 > >          /* 
 > > -         * If we need to disable soft blinking delegate this to the 
 > > -         * work queue task to avoid problems in case we are called 
 > > -         * from hard irq context. 
 > > +         * If software blink disable is pending, also queue brightness setting. 
 > > +         * If software blink is active, delay brightness setting 
 > > +         * until the next timer tick. 
 > >           */ 
 > > -        if (!brightness) { 
 > > -            set_bit(LED_BLINK_DISABLE, &led_cdev->work_flags); 
 > > +        if (test_bit(LED_SET_BRIGHTNESS, &led_cdev->work_flags) || 
 > > +            test_bit(LED_BLINK_DISABLE, &led_cdev->work_flags)) { 
 > > +            led_cdev->delayed_set_value = brightness; 
 > > +            set_bit(LED_SET_BRIGHTNESS, &led_cdev->work_flags); 
 > >              queue_work(led_cdev->wq, &led_cdev->set_brightness_work); 
 > > -        } else { 
 > > -            set_bit(LED_BLINK_BRIGHTNESS_CHANGE, 
 > > -                &led_cdev->work_flags); 
 > > +            return; 
 > > +        } else if (test_bit(LED_BLINK_SW, &led_cdev->work_flags)) { 
 > >              led_cdev->new_blink_brightness = brightness; 
 > > +            set_bit(LED_BLINK_BRIGHTNESS_CHANGE, &led_cdev->work_flags); 
 > > +            return; 
 > >          } 
 > > +    } else if (test_bit(LED_BLINK_SW, &led_cdev->work_flags)) { 
 > > +        /* 
 > > +         * If we need to disable soft blinking delegate this to the 
 > > +         * work queue task to avoid problems in case we are called 
 > > +         * from hard irq context. 
 > > +         */ 
 > > +        set_bit(LED_BLINK_DISABLE, &led_cdev->work_flags); 
 > > +        queue_work(led_cdev->wq, &led_cdev->set_brightness_work); 
 > >          return; 
 > >      } 
 > > 
 > > -- 
 > > 2.52.0 
 > > 

Re: [PATCH] leds: core: Fix race condition for software blink

[Lee Jones]

On Fri, 08 May 2026, Craig McQueen wrote:

> Lee Jones wrote
>  > On Thu, 23 Apr 2026, Craig McQueen wrote: 
>  >  
>  > > led_set_brightness() function: Change handling of software blink to 
>  > > avoid race conditions when stopping blink and setting brightness. 
>  > > 
>  > > Triggers may call led_set_brightness(LED_OFF), 
>  > > led_set_brightness(LED_FULL) in quick succession to disable blinking and 
>  > > turn the LED on. If the delayed work task has not yet disabled blinking 
>  > > by the time the second call occurs, then the brightness also needs to be 
>  > > changed in the delayed work task. 
>  > > 
>  > > Signed-off-by: Craig McQueen <craig@mcqueen.au> 
>  > > --- 
>  > >  drivers/leds/led-core.c | 33 ++++++++++++++++++++------------- 
>  > >  1 file changed, 20 insertions(+), 13 deletions(-) 
>  >  
>  > I think this needs more eyes on it and to be thoroughly tested by others. 
> 
> The problem I have seen is when a Linux driver controls a trigger to change it from flashing to steady-on. I've seen it specifically with LED core's software blinking.
> 
> /* First stop the blinking */
> led_trigger_event(&udev->led_trigger, LED_OFF);
> /* Next turn the LED steady-on */
> led_trigger_event(&udev->led_trigger, LED_FULL);
> 
> Without this patch, the LED usually ends up in the off state, rather than on.
> 
> I have tested this patch on a Rockchip RK3328 based system (ARM 64).

I'm sure that this patch does work for you.  Just as the last iteration
of this function has been working fine for everyone else for the last 10
years.  However, since this is core change, I shall be exercising
caution.

If anyone would be kind enough to test it, or provide some extra eyes,
I'd be appreciative.

-- 
Lee Jones

Re: [PATCH] leds: core: Fix race condition for software blink

[Craig McQueen]

Lee Jones wrote:
 > On Fri, 08 May 2026, Craig McQueen wrote: 
 > > Lee Jones wrote 
 > >  > On Thu, 23 Apr 2026, Craig McQueen wrote: 
 > >  > 
 > >  > > led_set_brightness() function: Change handling of software blink to 
 > >  > > avoid race conditions when stopping blink and setting brightness. 
 > >  > > 
 > >  > > Triggers may call led_set_brightness(LED_OFF), 
 > >  > > led_set_brightness(LED_FULL) in quick succession to disable blinking and 
 > >  > > turn the LED on. If the delayed work task has not yet disabled blinking 
 > >  > > by the time the second call occurs, then the brightness also needs to be 
 > >  > > changed in the delayed work task. 
 > >  > > 
 > >  > > Signed-off-by: Craig McQueen <craig@mcqueen.au> 
 > >  > > --- 
 > >  > >  drivers/leds/led-core.c | 33 ++++++++++++++++++++------------- 
 > >  > >  1 file changed, 20 insertions(+), 13 deletions(-) 
 > >  > 
 > >  > I think this needs more eyes on it and to be thoroughly tested by others. 
 > > 
 > > The problem I have seen is when a Linux driver controls a trigger to change it from flashing to steady-on. I've seen it specifically with LED core's software blinking. 
 > > 
 > > /* First stop the blinking */ 
 > > led_trigger_event(&udev->led_trigger, LED_OFF); 
 > > /* Next turn the LED steady-on */ 
 > > led_trigger_event(&udev->led_trigger, LED_FULL); 
 > > 
 > > Without this patch, the LED usually ends up in the off state, rather than on. 
 > > 
 > > I have tested this patch on a Rockchip RK3328 based system (ARM 64). 
 >  
 > I'm sure that this patch does work for you.  Just as the last iteration 
 > of this function has been working fine for everyone else for the last 10 
 > years.  However, since this is core change, I shall be exercising 
 > caution. 

I can readily reproduce this bug on multiple platforms:

* Texas Instruments AM335x 32-bit ARM (BeagleBone Black)
* Rockchip RK3328 64-bit ARM
* Ubuntu 25.10 on a laptop
* Ubuntu 22.04 on a VMware Workstation 16 Player VM

If others haven't seen it, maybe it's because this bug appears only with the intersection of two conditions:

1. A kernel driver that changes an LED trigger from blinking to steady-on (uncommon)
2. An LED that uses the kernel's LED software blinking, rather than hardware blinking

Note also that this issue doesn't seem to occur when controlling an LED from the sysfs interface:

echo timer > /sys/class/leds/test0/trigger
sleep 3
# First stop the blinking
echo 0 > /sys/class/leds/test0/brightness
# Next turn the LED steady-on
echo 1 > /sys/class/leds/test0/brightness

I guess because the sysfs interface adds enough extra time delays.

 > If anyone would be kind enough to test it, or provide some extra eyes, 
 > I'd be appreciative. 

I can offer the following method to test it.
I created a userspace LED trigger driver last year (submitted to linux-leds but not accepted).
https://github.com/cmcqueen/linux-kernel-uledtriggers

1. Make the Linux kernel driver out-of-tree.
    cd driver
    make
2. Load the driver.
    insmod ./ledtrig-user.ko
3. Make the test program.
    cd ../test/uledtriggers_c/
    make
4. Run the test program. It will create a new LED trigger named "blink-steady".
    ./uledtriggers_blink_steady
5. Connect an LED to the "blink-steady" trigger. It needs to be an LED that doesn't have hardware blinking. For example, using the Linux kernel uledmon.c:
    gcc uledmon.c -o uledmon
    modprobe uleds
    ./uledmon "test::test"
    echo "blink-steady" > "/sys/class/leds/test::test/trigger"

The LED trigger should make the LED blink for 5 seconds, and then steady-on for 5 seconds, then repeat.

With the kernel bug I've described, the LED is usually left OFF for 5 seconds rather than ON for 5 seconds as intended.

With the patch I submitted, this should be fixed.

-- 
Craig McQueen

Re: (subset) [PATCH] leds: core: Fix race condition for software blink

[Lee Jones]

On Thu, 23 Apr 2026 21:36:38 +1000, Craig McQueen wrote:
> led_set_brightness() function: Change handling of software blink to
> avoid race conditions when stopping blink and setting brightness.
> 
> Triggers may call led_set_brightness(LED_OFF),
> led_set_brightness(LED_FULL) in quick succession to disable blinking and
> turn the LED on. If the delayed work task has not yet disabled blinking
> by the time the second call occurs, then the brightness also needs to be
> changed in the delayed work task.
> 
> [...]

Applied, thanks!

[1/1] leds: core: Fix race condition for software blink
      commit: db01b61ed4602a081d35c9458a522b58fca44b7a

--
Lee Jones [李琼斯]