Re: [PATCH v6 0/5] leds: trigger: netdev: fix sysfs_update_group() races

[Lee Jones <lee@kernel.org>]

On Tue, 27 Oct 2026, A. Sverdlin wrote:

> From: Alexander Sverdlin <alexander.sverdlin@siemens.com>
> 
> The netdev LED trigger refreshes the link_speed attribute group with
> sysfs_update_group() from several contexts (the NETDEV_CHANGE notifier,
> device_name writes and, indirectly, trigger (de)activation) that share no
> common lock. On a board that emits PHY link events while the trigger is
> being (re)armed during boot this is observed as a hard sysfs failure:
> 
>   sysfs: cannot create duplicate filename '...green:lan/link_10'
>   ...
>   led_trigger_set
>   led_trigger_write
> 
> The window between activate() and device_add_groups() in led_trigger_set()
> is narrow, so to reproduce it reliably I artificially widened it with the
> debug patch below:
> 
> --- a/drivers/leds/led-triggers.c
> +++ b/drivers/leds/led-triggers.c
> @@ -12,6 +12,7 @@
>  #include <linux/list.h>
>  #include <linux/spinlock.h>
>  #include <linux/device.h>
> +#include <linux/delay.h>
>  #include <linux/timer.h>
>  #include <linux/rwsem.h>
>  #include <linux/leds.h>
> @@ -223,6 +224,8 @@ int led_trigger_set(struct led_classdev *led_cdev, struct led_trigger *trig)
>  		if (ret)
>  			goto err_activate;
> 
> +		msleep(5000);
> +
>  		ret = device_add_groups(led_cdev->dev, trig->groups);
>  		if (ret) {
>  			dev_err(led_cdev->dev, "Failed to add trigger attributes\n");
> 
> With that msleep() in place, writing the trigger while a NETDEV_CHANGE is
> delivered from the linkwatch workqueue reproduces the duplicate-filename
> splat every time.
> 
> The whole series is LOCKDEP-verified.
> 
> Changelog:
> v6:
> - report invalid interval to user space
> - fixed unlocked hw_control check in interval_store()
> - added missing carrier_link_up check in netdev_trig_sw_blink()

This set is ready to apply, but needs a rebase.

Please add my RB on each patch when resending.

Acked-by: Lee Jones <lee@kernel.org>

> v5:
> - removed unset_device_name()
> - https://lore.kernel.org/all/20261020124651.1987272-1-alexander.sverdlin@siemens.com/
> v4:
> - introduced netdev_trig_sw_blink() and used it in netdev_trig_work()
> - https://lore.kernel.org/all/20261001131206.2513248-1-alexander.sverdlin@siemens.com/
> v3:
> - introduced unset_device_name() as a counterpart of set_device_name()
> - guard() has been converted to either explicit mutex_lock()/mutex_unlock()
>   or scoped_guard()
> - rebased onto next-20260925
> - https://lore.kernel.org/all/20260929120902.2793138-1-alexander.sverdlin@siemens.com/
> v2:
> - addressed pre-existing issues found by Sashiko (patches 1&2)
> - reduced verbosity both in comments and in commit messages
> - patch 3 is a combined rework of patches 1&2 from v1
> - dropped sync from cancel_delayed_work() (worker now takes the lock)
> - https://lore.kernel.org/all/20260921093249.3481413-1-alexander.sverdlin@siemens.com/
> v1:
> - https://lore.kernel.org/all/20260914132356.807004-1-alexander.sverdlin@siemens.com/
> 
> 
> Alexander Sverdlin (5):
>   leds: trigger: netdev: Put netdev on activate error path
>   leds: trigger: netdev: Access net_dev under trigger_data->lock in the
>     worker
>   leds: trigger: netdev: Fix sysfs_update_group() races
>   leds: trigger: netdev: Report invalid interval to user space
>   leds: trigger: netdev: Serialize mode/interval stores with trigger
>     lock
> 
>  drivers/leds/trigger/ledtrig-netdev.c | 140 ++++++++++++++++++--------
>  1 file changed, 96 insertions(+), 44 deletions(-)
> 
> -- 
> 2.56.0
> 
> 

-- 
Lee Jones