From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jacek Anaszewski Subject: Re: [PATCH] leds: avoid races with workqueue Date: Thu, 2 May 2019 20:29:18 +0200 Message-ID: <9337b5fb-0ff8-9925-29e6-a781884af861@gmail.com> References: <20190426123513.GA18172@amd> <20190426214246.GA24966@amd> <84fac57d-1121-a1da-fb45-16a2521bdef9@gmail.com> <20190427193411.GA9709@amd> <2578a614-beb9-1c9d-9f74-208a8a7ab64f@gmail.com> <20190427223207.GA3585@amd> <20190429152259.GB10501@amd> <36e1fdd7-a220-4b0d-d558-829f522b0841@gmail.com> <20190501183600.GA20452@amd> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20190501183600.GA20452@amd> Content-Language: en-US List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: ibm-acpi-devel-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org To: Pavel Machek Cc: ibm-acpi-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org, ibm-acpi-N3TV7GIv+o9fyO9Q7EP/yw@public.gmane.org, linux-leds-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, platform-driver-x86-u79uwXL29TY76Z2rM5mHXA@public.gmane.org List-Id: linux-leds@vger.kernel.org Hi Pavel, On 5/1/19 8:36 PM, Pavel Machek wrote: > Hi! > >>> There are races between "main" thread and workqueue. They manifest >>> themselves on Thinkpad X60: >>> This should result in LED blinking, but it turns it off instead: >>> root@amd:/data/pavel# cd /sys/class/leds/tpacpi\:\:power >>> root@amd:/sys/class/leds/tpacpi::power# echo timer > trigger >>> root@amd:/sys/class/leds/tpacpi::power# echo timer > trigger >>> root@amd:/sys/class/leds/tpacpi::power# I believe this line is redundant, so I removed it. >>> It should be possible to transition from blinking to solid on by echo >>> 0 > brightness; echo 1 > brightness... but that does not work, either, >>> if done too quickly. >>> Synchronization of the workqueue fixes both. >>> Signed-off-by: Pavel Machek >>> >>> diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c >>> index 68aa923..dcb59c8 100644 >>> --- a/drivers/leds/led-class.c >>> +++ b/drivers/leds/led-class.c >>> @@ -57,6 +57,7 @@ static ssize_t brightness_store(struct device *dev, >>> if (state == LED_OFF) >>> led_trigger_remove(led_cdev); >>> led_set_brightness(led_cdev, state); >>> + flush_work(&led_cdev->set_brightness_work); >> >> Is this really required here? It creates non-uniform brightness >> setting behavior depending on whether it is set from sysfs or >> by in-kernel call to led_set_brightness(). > > This fixes the echo 0 > brightness; echo 1 > brightness. It has to be > at a place where we can sleep. > > If you have better idea, it is welcome, but it would be good to fix > the bug. Currently not, so I applied the patch in this shape. -- Best regards, Jacek Anaszewski