#
ab477b76 |
| 13-Jun-2024 |
Thomas Weißschuh <linux@weissschuh.net> |
leds: triggers: Flush pending brightness before activating trigger
The race fixed in timer_trig_activate() between a blocking set_brightness() call and trigger->activate() can affect any trigger. So
leds: triggers: Flush pending brightness before activating trigger
The race fixed in timer_trig_activate() between a blocking set_brightness() call and trigger->activate() can affect any trigger. So move the call to flush_work() into led_trigger_set() where it can avoid the race for all triggers.
Fixes: 0db37915d912 ("leds: avoid races with workqueue") Fixes: 8c0f693c6eff ("leds: avoid flush_work in atomic context") Cc: stable@vger.kernel.org Tested-by: Dustin L. Howett <dustin@howett.net> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net> Link: https://lore.kernel.org/r/20240613-led-trigger-flush-v2-1-f4f970799d77@weissschuh.net Signed-off-by: Lee Jones <lee@kernel.org>
show more ...
|
#
b1bbd20f |
| 31-May-2024 |
Hans de Goede <hdegoede@redhat.com> |
leds: trigger: Call synchronize_rcu() before calling trig->activate()
Some triggers call led_trigger_event() from their activate() callback to initialize the brightness of the LED for which the trig
leds: trigger: Call synchronize_rcu() before calling trig->activate()
Some triggers call led_trigger_event() from their activate() callback to initialize the brightness of the LED for which the trigger is being activated.
In order for the LED's initial state to be set correctly this requires that the led_trigger_event() call uses the new version of trigger->led_cdevs, which has the new LED.
AFAICT led_trigger_event() will always use the new version when it is running on the same CPU as where the list_add_tail_rcu() call was made, which is why the missing synchronize_rcu() has not lead to bug reports. But if activate() is pre-empted, sleeps or uses a worker then the led_trigger_event() call may run on another CPU which may still use the old trigger->led_cdevs list.
Add a synchronize_rcu() call to ensure that any led_trigger_event() calls done from activate() always use the new list.
Triggers using led_trigger_event() from their activate() callback are: net/bluetooth/leds.c, net/rfkill/core.c and drivers/tty/vt/keyboard.c.
Signed-off-by: Hans de Goede <hdegoede@redhat.com> Link: https://lore.kernel.org/r/20240531120124.75662-1-hdegoede@redhat.com Signed-off-by: Lee Jones <lee@kernel.org>
show more ...
|
#
c0dc9adf |
| 04-May-2024 |
Hans de Goede <hdegoede@redhat.com> |
leds: trigger: Unregister sysfs attributes before calling deactivate()
Triggers which have trigger specific sysfs attributes typically store related data in trigger-data allocated by the activate()
leds: trigger: Unregister sysfs attributes before calling deactivate()
Triggers which have trigger specific sysfs attributes typically store related data in trigger-data allocated by the activate() callback and freed by the deactivate() callback.
Calling device_remove_groups() after calling deactivate() leaves a window where the sysfs attributes show/store functions could be called after deactivation and then operate on the just freed trigger-data.
Move the device_remove_groups() call to before deactivate() to close this race window.
This also makes the deactivation path properly do things in reverse order of the activation path which calls the activate() callback before calling device_add_groups().
Fixes: a7e7a3156300 ("leds: triggers: add device attribute support") Cc: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> Signed-off-by: Hans de Goede <hdegoede@redhat.com> Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> Link: https://lore.kernel.org/r/20240504162533.76780-1-hdegoede@redhat.com Signed-off-by: Lee Jones <lee@kernel.org>
show more ...
|
#
0921a57c |
| 31-May-2024 |
Hans de Goede <hdegoede@redhat.com> |
leds: trigger: Add led_mc_trigger_event() function
Add a new led_mc_trigger_event() function for triggers which want to change the color of a multi-color LED based on their trigger conditions.
Sign
leds: trigger: Add led_mc_trigger_event() function
Add a new led_mc_trigger_event() function for triggers which want to change the color of a multi-color LED based on their trigger conditions.
Signed-off-by: Hans de Goede <hdegoede@redhat.com> Reviewed-by: Jacek Anaszewski <jacek.anaszewski@gmail.com> Reviewed-by: Andy Shevchenko <andy@kernel.org> Link: https://lore.kernel.org/r/20240531114124.45346-6-hdegoede@redhat.com Signed-off-by: Lee Jones <lee@kernel.org>
show more ...
|
#
822c91e7 |
| 04-Mar-2024 |
Heiner Kallweit <hkallweit1@gmail.com> |
leds: trigger: Store brightness set by led_trigger_event()
If a simple trigger is assigned to a LED, then the LED may be off until the next led_trigger_event() call. This may be an issue for simple
leds: trigger: Store brightness set by led_trigger_event()
If a simple trigger is assigned to a LED, then the LED may be off until the next led_trigger_event() call. This may be an issue for simple triggers with rare led_trigger_event() calls, e.g. power supply charging indicators (drivers/power/supply/power_supply_leds.c). Therefore persist the brightness value of the last led_trigger_event() call and use this value if the trigger is assigned to a LED. In addition add a getter for the trigger brightness value.
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> Reviewed-by: Takashi Iwai <tiwai@suse.de> Link: https://lore.kernel.org/r/b1358b25-3f30-458d-8240-5705ae007a8a@gmail.com Signed-off-by: Lee Jones <lee@kernel.org>
show more ...
|
#
9225333e |
| 31-Jan-2024 |
Heiner Kallweit <hkallweit1@gmail.com> |
leds: triggers: Add helper led_match_default_trigger
Avoid code duplication and factor out common functionality to new helper led_match_default_trigger().
Signed-off-by: Heiner Kallweit <hkallweit1
leds: triggers: Add helper led_match_default_trigger
Avoid code duplication and factor out common functionality to new helper led_match_default_trigger().
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> Link: https://lore.kernel.org/r/d78eef6f-c18c-4546-b83e-6d1890849154@gmail.com Signed-off-by: Lee Jones <lee@kernel.org>
show more ...
|
#
e838a5a1 |
| 31-Jan-2024 |
Heiner Kallweit <hkallweit1@gmail.com> |
leds: trigger: Stop exporting trigger_list
Commit 682e98564ffb ("leds: trigger: panic: Simplify led_trigger_set_panic") removed the last external user of variable trigger_list. So stop exporting it.
leds: trigger: Stop exporting trigger_list
Commit 682e98564ffb ("leds: trigger: panic: Simplify led_trigger_set_panic") removed the last external user of variable trigger_list. So stop exporting it. If in future a need should arise again to access this variable, I think we better add some accessor instead of exporting the variable directly.
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> Link: https://lore.kernel.org/r/ca185fb1-3a66-46b9-920e-bfecbe39c6bf@gmail.com Signed-off-by: Lee Jones <lee@kernel.org>
show more ...
|
#
e09c706b |
| 21-Dec-2023 |
Heiner Kallweit <hkallweit1@gmail.com> |
leds: trigger: Load trigger modules on-demand if used as default trigger
Even if a trigger is set as default trigger for a LED device, the respective trigger module (if built as module) isn't automa
leds: trigger: Load trigger modules on-demand if used as default trigger
Even if a trigger is set as default trigger for a LED device, the respective trigger module (if built as module) isn't automatically loaded by the kernel if the LED device is registered. I think we can do better. Try to load the module asynchronously by alias ledtrig:<trigger name>. This requires that such an alias is added to relevant triggers.
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> Link: https://lore.kernel.org/r/79adb260-06ad-443a-a68e-abe4498c3298@gmail.com Signed-off-by: Lee Jones <lee@kernel.org>
show more ...
|
#
c82a1662 |
| 08-Dec-2023 |
Heiner Kallweit <hkallweit1@gmail.com> |
leds: trigger: Remove unused function led_trigger_rename_static()
This function was added with a8df7b1ab70b ("leds: add led_trigger_rename function") 11 yrs ago, but it has no users. So remove it.
leds: trigger: Remove unused function led_trigger_rename_static()
This function was added with a8df7b1ab70b ("leds: add led_trigger_rename function") 11 yrs ago, but it has no users. So remove it.
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> Link: https://lore.kernel.org/r/d90f30be-f661-4db7-b0b5-d09d07a78a68@gmail.com Signed-off-by: Lee Jones <lee@kernel.org>
show more ...
|
#
82f80ef5 |
| 10-May-2023 |
Hans de Goede <hdegoede@redhat.com> |
leds: Clear LED_INIT_DEFAULT_TRIGGER when clearing current trigger
Not all triggers use LED_INIT_DEFAULT_TRIGGER, which means that it will not get cleared when the default trigger is a trigger which
leds: Clear LED_INIT_DEFAULT_TRIGGER when clearing current trigger
Not all triggers use LED_INIT_DEFAULT_TRIGGER, which means that it will not get cleared when the default trigger is a trigger which does not use it such as "default-on".
If the default trigger then later gets replaced by a trigger which does check LED_INIT_DEFAULT_TRIGGER, such as "timer" then that trigger will behave as if it is the default trigger which it should not do.
To fix this clear the LED_INIT_DEFAULT_TRIGGER flag when clearing the current trigger, so that it will not be set for any subsequently set (non default) triggers.
Signed-off-by: Hans de Goede <hdegoede@redhat.com> Reviewed-by: Jacek Anaszewski <jacek.anaszewski@gmail.com> Tested-by: Yauhen Kharuzhy <jekhor@gmail.com> Link: https://lore.kernel.org/r/20230510162234.291439-5-hdegoede@redhat.com Signed-off-by: Lee Jones <lee@kernel.org>
show more ...
|
#
22720a87 |
| 10-May-2023 |
Hans de Goede <hdegoede@redhat.com> |
leds: Fix oops about sleeping in led_trigger_blink()
led_trigger_blink() calls led_blink_set() from a RCU read-side critical section so led_blink_set() must not sleep. Note sleeping was not allowed
leds: Fix oops about sleeping in led_trigger_blink()
led_trigger_blink() calls led_blink_set() from a RCU read-side critical section so led_blink_set() must not sleep. Note sleeping was not allowed before the switch to RCU either because a spinlock was held before.
led_blink_set() does not sleep when sw-blinking is used, but many LED controller drivers with hw blink support have a blink_set function which may sleep, leading to an oops like this one:
[ 832.605062] ------------[ cut here ]------------ [ 832.605085] Voluntary context switch within RCU read-side critical section! [ 832.605119] WARNING: CPU: 2 PID: 370 at kernel/rcu/tree_plugin.h:318 rcu_note_context_switch+0x4ee/0x690 <snip> [ 832.606453] Call Trace: [ 832.606466] <TASK> [ 832.606487] __schedule+0x9f/0x1480 [ 832.606527] schedule+0x5d/0xe0 [ 832.606549] schedule_timeout+0x79/0x140 [ 832.606572] ? __pfx_process_timeout+0x10/0x10 [ 832.606599] wait_for_completion_timeout+0x6f/0x140 [ 832.606627] i2c_dw_xfer+0x101/0x460 [ 832.606659] ? psi_group_change+0x168/0x400 [ 832.606680] __i2c_transfer+0x172/0x6d0 [ 832.606709] i2c_smbus_xfer_emulated+0x27d/0x9c0 [ 832.606732] ? __schedule+0x430/0x1480 [ 832.606753] ? preempt_count_add+0x6a/0xa0 [ 832.606778] ? get_nohz_timer_target+0x18/0x190 [ 832.606796] ? lock_timer_base+0x61/0x80 [ 832.606817] ? preempt_count_add+0x6a/0xa0 [ 832.606842] __i2c_smbus_xfer+0xa2/0x3f0 [ 832.606862] i2c_smbus_xfer+0x66/0xf0 [ 832.606882] i2c_smbus_read_byte_data+0x41/0x70 [ 832.606901] ? _raw_spin_unlock_irqrestore+0x23/0x40 [ 832.606922] ? __pm_runtime_suspend+0x46/0xc0 [ 832.606946] cht_wc_byte_reg_read+0x2e/0x60 [ 832.606972] _regmap_read+0x5c/0x120 [ 832.606997] _regmap_update_bits+0x96/0xc0 [ 832.607023] regmap_update_bits_base+0x5b/0x90 [ 832.607053] cht_wc_leds_brightness_get+0x412/0x910 [leds_cht_wcove] [ 832.607094] led_blink_setup+0x28/0x100 [ 832.607119] led_trigger_blink+0x40/0x70 [ 832.607145] power_supply_update_leds+0x1b7/0x1c0 [ 832.607174] power_supply_changed_work+0x67/0xe0 [ 832.607198] process_one_work+0x1c8/0x3c0 [ 832.607222] worker_thread+0x4d/0x380 [ 832.607243] ? __pfx_worker_thread+0x10/0x10 [ 832.607258] kthread+0xe9/0x110 [ 832.607279] ? __pfx_kthread+0x10/0x10 [ 832.607300] ret_from_fork+0x2c/0x50 [ 832.607337] </TASK> [ 832.607344] ---[ end trace 0000000000000000 ]---
Add a new led_blink_set_nosleep() function which defers the actual led_blink_set() call to a workqueue when necessary to fix this.
This also fixes an existing race where a pending led_set_brightness() has been deferred to set_brightness_work and might then race with a later led_cdev->blink_set() call. Note this race is only an issue with triggers mixing led_trigger_event() and led_trigger_blink() calls, sysfs API calls and led_trigger_blink_oneshot() are not affected.
Note rather then adding a separate blink_set_blocking callback this uses the presence of the already existing brightness_set_blocking callback to detect if the blinking call should be deferred to set_brightness_work.
Signed-off-by: Hans de Goede <hdegoede@redhat.com> Reviewed-by: Jacek Anaszewski <jacek.anaszewski@gmail.com> Tested-by: Yauhen Kharuzhy <jekhor@gmail.com> Link: https://lore.kernel.org/r/20230510162234.291439-4-hdegoede@redhat.com Signed-off-by: Lee Jones <lee@kernel.org>
show more ...
|
#
e298d8a3 |
| 10-May-2023 |
Hans de Goede <hdegoede@redhat.com> |
leds: Change led_trigger_blink[_oneshot]() delay parameters to pass-by-value
led_blink_set[_oneshot]()'s delay_on and delay_off function parameters are pass by reference, so that hw-blink implementa
leds: Change led_trigger_blink[_oneshot]() delay parameters to pass-by-value
led_blink_set[_oneshot]()'s delay_on and delay_off function parameters are pass by reference, so that hw-blink implementations can report back the actual achieved delays when the values have been rounded to something the hw supports.
This is really only interesting for the sysfs API / the timer trigger. Other triggers don't really care about this and none of the callers of led_trigger_blink[_oneshot]() do anything with the returned delay values.
Change the led_trigger_blink[_oneshot]() delay parameters to pass-by-value, there are 2 reasons for this:
1. led_cdev->blink_set() may sleep, while led_trigger_blink() may not. So on hw where led_cdev->blink_set() sleeps the call needs to be deferred to a workqueue, in which case the actual achieved delays are unknown (this is a preparation patch for the deferring).
2. Since the callers don't care about the actual achieved delays, allowing callers to directly pass a value leads to simpler code for most callers.
Signed-off-by: Hans de Goede <hdegoede@redhat.com> Reviewed-by: Jacek Anaszewski <jacek.anaszewski@gmail.com> Tested-by: Yauhen Kharuzhy <jekhor@gmail.com> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Acked-by: Sebastian Reichel <sebastian.reichel@collabora.com> Acked-by: Florian Westphal <fw@strlen.de> Link: https://lore.kernel.org/r/20230510162234.291439-2-hdegoede@redhat.com Signed-off-by: Lee Jones <lee@kernel.org>
show more ...
|
#
2a5a8fa8 |
| 15-Sep-2021 |
Johannes Berg <johannes.berg@intel.com> |
leds: trigger: use RCU to protect the led_cdevs list
Even with the previous commit 27af8e2c90fb ("leds: trigger: fix potential deadlock with libata") to this file, we still get lockdep unhappy, and
leds: trigger: use RCU to protect the led_cdevs list
Even with the previous commit 27af8e2c90fb ("leds: trigger: fix potential deadlock with libata") to this file, we still get lockdep unhappy, and Boqun explained the report here: https://lore.kernel.org/r/YNA+d1X4UkoQ7g8a@boqun-archlinux
Effectively, this means that the read_lock_irqsave() isn't enough here because another CPU might be trying to do a write lock, and thus block the readers.
This is all pretty messy, but it doesn't seem right that the LEDs framework imposes some locking requirements on users, in particular we'd have to make the spinlock in the iwlwifi driver always disable IRQs, even if we don't need that for any other reason, just to avoid this deadlock.
Since writes to the led_cdevs list are rare (and are done by userspace), just switch the list to RCU. This costs a synchronize_rcu() at removal time so we can ensure things are correct, but that seems like a small price to pay for getting lock-free iterations and no deadlocks (nor any locking requirements imposed on users.)
Signed-off-by: Johannes Berg <johannes.berg@intel.com> Signed-off-by: Pavel Machek <pavel@ucw.cz>
show more ...
|
#
27af8e2c |
| 25-Nov-2020 |
Andrea Righi <andrea.righi@canonical.com> |
leds: trigger: fix potential deadlock with libata
We have the following potential deadlock condition:
======================================================== WARNING: possible irq lock inversion
leds: trigger: fix potential deadlock with libata
We have the following potential deadlock condition:
======================================================== WARNING: possible irq lock inversion dependency detected 5.10.0-rc2+ #25 Not tainted -------------------------------------------------------- swapper/3/0 just changed the state of lock: ffff8880063bd618 (&host->lock){-...}-{2:2}, at: ata_bmdma_interrupt+0x27/0x200 but this lock took another, HARDIRQ-READ-unsafe lock in the past: (&trig->leddev_list_lock){.+.?}-{2:2}
and interrupts could create inverse lock ordering between them.
other info that might help us debug this: Possible interrupt unsafe locking scenario:
CPU0 CPU1 ---- ---- lock(&trig->leddev_list_lock); local_irq_disable(); lock(&host->lock); lock(&trig->leddev_list_lock); <Interrupt> lock(&host->lock);
*** DEADLOCK ***
no locks held by swapper/3/0.
the shortest dependencies between 2nd lock and 1st lock: -> (&trig->leddev_list_lock){.+.?}-{2:2} ops: 46 { HARDIRQ-ON-R at: lock_acquire+0x15f/0x420 _raw_read_lock+0x42/0x90 led_trigger_event+0x2b/0x70 rfkill_global_led_trigger_worker+0x94/0xb0 process_one_work+0x240/0x560 worker_thread+0x58/0x3d0 kthread+0x151/0x170 ret_from_fork+0x1f/0x30 IN-SOFTIRQ-R at: lock_acquire+0x15f/0x420 _raw_read_lock+0x42/0x90 led_trigger_event+0x2b/0x70 kbd_bh+0x9e/0xc0 tasklet_action_common.constprop.0+0xe9/0x100 tasklet_action+0x22/0x30 __do_softirq+0xcc/0x46d run_ksoftirqd+0x3f/0x70 smpboot_thread_fn+0x116/0x1f0 kthread+0x151/0x170 ret_from_fork+0x1f/0x30 SOFTIRQ-ON-R at: lock_acquire+0x15f/0x420 _raw_read_lock+0x42/0x90 led_trigger_event+0x2b/0x70 rfkill_global_led_trigger_worker+0x94/0xb0 process_one_work+0x240/0x560 worker_thread+0x58/0x3d0 kthread+0x151/0x170 ret_from_fork+0x1f/0x30 INITIAL READ USE at: lock_acquire+0x15f/0x420 _raw_read_lock+0x42/0x90 led_trigger_event+0x2b/0x70 rfkill_global_led_trigger_worker+0x94/0xb0 process_one_work+0x240/0x560 worker_thread+0x58/0x3d0 kthread+0x151/0x170 ret_from_fork+0x1f/0x30 } ... key at: [<ffffffff83da4c00>] __key.0+0x0/0x10 ... acquired at: _raw_read_lock+0x42/0x90 led_trigger_blink_oneshot+0x3b/0x90 ledtrig_disk_activity+0x3c/0xa0 ata_qc_complete+0x26/0x450 ata_do_link_abort+0xa3/0xe0 ata_port_freeze+0x2e/0x40 ata_hsm_qc_complete+0x94/0xa0 ata_sff_hsm_move+0x177/0x7a0 ata_sff_pio_task+0xc7/0x1b0 process_one_work+0x240/0x560 worker_thread+0x58/0x3d0 kthread+0x151/0x170 ret_from_fork+0x1f/0x30
-> (&host->lock){-...}-{2:2} ops: 69 { IN-HARDIRQ-W at: lock_acquire+0x15f/0x420 _raw_spin_lock_irqsave+0x52/0xa0 ata_bmdma_interrupt+0x27/0x200 __handle_irq_event_percpu+0xd5/0x2b0 handle_irq_event+0x57/0xb0 handle_edge_irq+0x8c/0x230 asm_call_irq_on_stack+0xf/0x20 common_interrupt+0x100/0x1c0 asm_common_interrupt+0x1e/0x40 native_safe_halt+0xe/0x10 arch_cpu_idle+0x15/0x20 default_idle_call+0x59/0x1c0 do_idle+0x22c/0x2c0 cpu_startup_entry+0x20/0x30 start_secondary+0x11d/0x150 secondary_startup_64_no_verify+0xa6/0xab INITIAL USE at: lock_acquire+0x15f/0x420 _raw_spin_lock_irqsave+0x52/0xa0 ata_dev_init+0x54/0xe0 ata_link_init+0x8b/0xd0 ata_port_alloc+0x1f1/0x210 ata_host_alloc+0xf1/0x130 ata_host_alloc_pinfo+0x14/0xb0 ata_pci_sff_prepare_host+0x41/0xa0 ata_pci_bmdma_prepare_host+0x14/0x30 piix_init_one+0x21f/0x600 local_pci_probe+0x48/0x80 pci_device_probe+0x105/0x1c0 really_probe+0x221/0x490 driver_probe_device+0xe9/0x160 device_driver_attach+0xb2/0xc0 __driver_attach+0x91/0x150 bus_for_each_dev+0x81/0xc0 driver_attach+0x1e/0x20 bus_add_driver+0x138/0x1f0 driver_register+0x91/0xf0 __pci_register_driver+0x73/0x80 piix_init+0x1e/0x2e do_one_initcall+0x5f/0x2d0 kernel_init_freeable+0x26f/0x2cf kernel_init+0xe/0x113 ret_from_fork+0x1f/0x30 } ... key at: [<ffffffff83d9fdc0>] __key.6+0x0/0x10 ... acquired at: __lock_acquire+0x9da/0x2370 lock_acquire+0x15f/0x420 _raw_spin_lock_irqsave+0x52/0xa0 ata_bmdma_interrupt+0x27/0x200 __handle_irq_event_percpu+0xd5/0x2b0 handle_irq_event+0x57/0xb0 handle_edge_irq+0x8c/0x230 asm_call_irq_on_stack+0xf/0x20 common_interrupt+0x100/0x1c0 asm_common_interrupt+0x1e/0x40 native_safe_halt+0xe/0x10 arch_cpu_idle+0x15/0x20 default_idle_call+0x59/0x1c0 do_idle+0x22c/0x2c0 cpu_startup_entry+0x20/0x30 start_secondary+0x11d/0x150 secondary_startup_64_no_verify+0xa6/0xab
This lockdep splat is reported after: commit e918188611f0 ("locking: More accurate annotations for read_lock()")
To clarify: - read-locks are recursive only in interrupt context (when in_interrupt() returns true) - after acquiring host->lock in CPU1, another cpu (i.e. CPU2) may call write_lock(&trig->leddev_list_lock) that would be blocked by CPU0 that holds trig->leddev_list_lock in read-mode - when CPU1 (ata_ac_complete()) tries to read-lock trig->leddev_list_lock, it would be blocked by the write-lock waiter on CPU2 (because we are not in interrupt context, so the read-lock is not recursive) - at this point if an interrupt happens on CPU0 and ata_bmdma_interrupt() is executed it will try to acquire host->lock, that is held by CPU1, that is currently blocked by CPU2, so:
* CPU0 blocked by CPU1 * CPU1 blocked by CPU2 * CPU2 blocked by CPU0
*** DEADLOCK ***
The deadlock scenario is better represented by the following schema (thanks to Boqun Feng <boqun.feng@gmail.com> for the schema and the detailed explanation of the deadlock condition):
CPU 0: CPU 1: CPU 2: ----- ----- ----- led_trigger_event(): read_lock(&trig->leddev_list_lock); <workqueue> ata_hsm_qc_complete(): spin_lock_irqsave(&host->lock); write_lock(&trig->leddev_list_lock); ata_port_freeze(): ata_do_link_abort(): ata_qc_complete(): ledtrig_disk_activity(): led_trigger_blink_oneshot(): read_lock(&trig->leddev_list_lock); // ^ not in in_interrupt() context, so could get blocked by CPU 2 <interrupt> ata_bmdma_interrupt(): spin_lock_irqsave(&host->lock);
Fix by using read_lock_irqsave/irqrestore() in led_trigger_event(), so that no interrupt can happen in between, preventing the deadlock condition.
Apply the same change to led_trigger_blink_setup() as well, since the same deadlock scenario can also happen in power_supply_update_bat_leds() -> led_trigger_blink() -> led_trigger_blink_setup() (workqueue context), and potentially prevent other similar usages.
Link: https://lore.kernel.org/lkml/20201101092614.GB3989@xps-13-7390/ Fixes: eb25cb9956cc ("leds: convert IDE trigger to common disk trigger") Signed-off-by: Andrea Righi <andrea.righi@canonical.com> Signed-off-by: Pavel Machek <pavel@ucw.cz>
show more ...
|
#
93690cdf |
| 16-Jul-2020 |
Marek Behún <marek.behun@nic.cz> |
leds: trigger: add support for LED-private device triggers
Some LED controllers may come with an internal HW triggering mechanism for the LED and the ability to switch between SW control and the int
leds: trigger: add support for LED-private device triggers
Some LED controllers may come with an internal HW triggering mechanism for the LED and the ability to switch between SW control and the internal HW control. This includes most PHYs, various ethernet switches, the Turris Omnia LED controller or AXP20X PMIC.
This adds support for registering such triggers.
This code is based on work by Pavel Machek <pavel@ucw.cz> and Ondřej Jirman <megous@megous.com>.
Signed-off-by: Marek Behún <marek.behun@nic.cz> Acked-by: Jacek Anaszewski <jacek.anaszewski@gmail.com> Signed-off-by: Pavel Machek <pavel@ucw.cz>
show more ...
|
#
14d3e74f |
| 09-Jun-2020 |
Flavio Suligoi <f.suligoi@asem.it> |
leds: fix spelling mistake
Fix typo: "Tigger" --> "Trigger"
Signed-off-by: Flavio Suligoi <f.suligoi@asem.it> Reviewed-by: Alexander Dahl <ada@thorsis.com> Signed-off-by: Pavel Machek <pavel@ucw.cz>
|
#
11f70002 |
| 29-Sep-2019 |
Akinobu Mita <akinobu.mita@gmail.com> |
leds: remove PAGE_SIZE limit of /sys/class/leds/<led>/trigger
Reading /sys/class/leds/<led>/trigger returns all available LED triggers. However, the size of this file is limited to PAGE_SIZE because
leds: remove PAGE_SIZE limit of /sys/class/leds/<led>/trigger
Reading /sys/class/leds/<led>/trigger returns all available LED triggers. However, the size of this file is limited to PAGE_SIZE because of the limitation for sysfs attribute.
Enabling LED CPU trigger on systems with thousands of CPUs easily hits PAGE_SIZE limit, and makes it impossible to see all available LED triggers and which trigger is currently activated.
We work around it here by converting /sys/class/leds/<led>/trigger to binary attribute, which is not limited by length. This is _not_ good design, do not copy it.
Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com> Cc: "Rafael J. Wysocki" <rafael@kernel.org> Cc: Pavel Machek <pavel@ucw.cz> Cc: Dan Murphy <dmurphy@ti.com>A Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Signed-off-by: Pavel Machek <pavel@ucw.cz>
show more ...
|
#
4016ba85 |
| 03-Sep-2019 |
Oleh Kravchenko <oleg@kaa.org.ua> |
led: triggers: Fix dereferencing of null pointer
Error was detected by PVS-Studio: V522 Dereferencing of the null pointer 'led_cdev->trigger' might take place.
Fixes: 2282e125a406 ("leds: triggers:
led: triggers: Fix dereferencing of null pointer
Error was detected by PVS-Studio: V522 Dereferencing of the null pointer 'led_cdev->trigger' might take place.
Fixes: 2282e125a406 ("leds: triggers: let struct led_trigger::activate() return an error code") Signed-off-by: Oleh Kravchenko <oleg@kaa.org.ua> Reviewed-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> Signed-off-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
show more ...
|
#
60e2dde1 |
| 19-Aug-2019 |
Wenwen Wang <wenwen@cs.uga.edu> |
led: triggers: Fix a memory leak bug
In led_trigger_set(), 'event' is allocated in kasprintf(). However, it is not deallocated in the following execution if the label 'err_activate' or 'err_add_grou
led: triggers: Fix a memory leak bug
In led_trigger_set(), 'event' is allocated in kasprintf(). However, it is not deallocated in the following execution if the label 'err_activate' or 'err_add_groups' is entered, leading to memory leaks. To fix this issue, free 'event' before returning the error.
Fixes: 52c47742f79d ("leds: triggers: send uevent when changing triggers") Signed-off-by: Wenwen Wang <wenwen@cs.uga.edu> Acked-by: Pavel Machek <pavel@ucw.cz> Signed-off-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
show more ...
|
#
d2912cb1 |
| 04-Jun-2019 |
Thomas Gleixner <tglx@linutronix.de> |
treewide: Replace GPLv2 boilerplate/reference with SPDX - rule 500
Based on 2 normalized pattern(s):
this program is free software you can redistribute it and or modify it under the terms of th
treewide: Replace GPLv2 boilerplate/reference with SPDX - rule 500
Based on 2 normalized pattern(s):
this program is free software you can redistribute it and or modify it under the terms of the gnu general public license version 2 as published by the free software foundation
this program is free software you can redistribute it and or modify it under the terms of the gnu general public license version 2 as published by the free software foundation #
extracted by the scancode license scanner the SPDX license identifier
GPL-2.0-only
has been chosen to replace the boilerplate/reference in 4122 file(s).
Signed-off-by: Thomas Gleixner <tglx@linutronix.de> Reviewed-by: Enrico Weigelt <info@metux.net> Reviewed-by: Kate Stewart <kstewart@linuxfoundation.org> Reviewed-by: Allison Randal <allison@lohutok.net> Cc: linux-spdx@vger.kernel.org Link: https://lkml.kernel.org/r/20190604081206.933168790@linutronix.de Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
show more ...
|
#
8146aace |
| 10-Dec-2018 |
Krzysztof Kozlowski <krzk@kernel.org> |
led: triggers: Initialize LED_INIT_DEFAULT_TRIGGER if trigger is brought after class
Trigger driver can be initialized after the LED class device driver. In such case led_trigger_set_default() won'
led: triggers: Initialize LED_INIT_DEFAULT_TRIGGER if trigger is brought after class
Trigger driver can be initialized after the LED class device driver. In such case led_trigger_set_default() won't be called and flag LED_INIT_DEFAULT_TRIGGER should be set from led_trigger_register().
Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org> Signed-off-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
show more ...
|
#
02d31765 |
| 10-Dec-2018 |
Jacek Anaszewski <jacek.anaszewski@gmail.com> |
led: triggers: Add LED_INIT_DEFAULT_TRIGGER flag
Add the flag LED_INIT_DEFAULT_TRIGGER for indicating that trigger being set is a default trigger for the LED class device, and thus it should be init
led: triggers: Add LED_INIT_DEFAULT_TRIGGER flag
Add the flag LED_INIT_DEFAULT_TRIGGER for indicating that trigger being set is a default trigger for the LED class device, and thus it should be initialized with settings provided in the fwnode.
Set the flag in the led_trigger_set_default(). It is expected to be cleared in the activate() op of a trigger after trigger fwnode initialization data is parsed and applied. This should happen only once after LED class device registration, to allow leaving triggers in the idle state on re-apply and let the users apply their own settings without interference from the default ones.
Signed-off-by: Jacek Anaszewski <jacek.anaszewski@gmail.com> Acked-by: Pavel Machek <pavel@ucw.cz> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
show more ...
|
#
c4f7bd4a |
| 10-Dec-2018 |
Jacek Anaszewski <jacek.anaszewski@gmail.com> |
led: triggers: Break the for loop after default trigger is found
It is of no avail to continue iterating through registered triggers in the led_trigger_set_default() after the trigger to set has bee
led: triggers: Break the for loop after default trigger is found
It is of no avail to continue iterating through registered triggers in the led_trigger_set_default() after the trigger to set has been found. Add "break" statement to fix this omission.
Signed-off-by: Jacek Anaszewski <jacek.anaszewski@gmail.com> Acked-by: Pavel Machek <pavel@ucw.cz> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
show more ...
|
#
a7d5904a |
| 02-Jul-2018 |
Uwe Kleine-König <u.kleine-koenig@pengutronix.de> |
leds: triggers: handle .trigger_data and .activated() in the core
This helps keeping these two fields consistent and drivers don't need to care for this themselves any more.
Note that .activated is
leds: triggers: handle .trigger_data and .activated() in the core
This helps keeping these two fields consistent and drivers don't need to care for this themselves any more.
Note that .activated isn't set to true automatically because that might confuse some triggers when deactivating (e.g. ledtrig-gpio).
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> Acked-by: Pavel Machek <pavel@ucw.cz> Signed-off-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
show more ...
|
#
a7e7a315 |
| 02-Jul-2018 |
Uwe Kleine-König <u.kleine-koenig@pengutronix.de> |
leds: triggers: add device attribute support
As many triggers use device attributes, add support for these in led_trigger_set which allows simplifying the drivers accordingly.
Signed-off-by: Uwe Kl
leds: triggers: add device attribute support
As many triggers use device attributes, add support for these in led_trigger_set which allows simplifying the drivers accordingly.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> Acked-by: Pavel Machek <pavel@ucw.cz> Signed-off-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
show more ...
|