| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <9d75b9bcfd18f2e2073f201375458af7bf5f1dc7.camel@posteo.de>
Date: Fri, 23 Jan 2026 19:18:03 +0000
From: Markus Probst <markus.probst@...teo.de>
To: Lee Jones <lee@...nel.org>, Pavel Machek <pavel@...nel.org>, Rob Herring
<robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
<conor+dt@...nel.org>, Jacek Anaszewski <jacek.anaszewski@...il.com>,
Damien Le Moal <dlemoal@...nel.org>, Niklas Cassel <cassel@...nel.org>,
John Garry <john.g.garry@...cle.com>, Jason Yan <yanaijie@...wei.com>,
"James E.J. Bottomley" <James.Bottomley@...senPartnership.com>, "Martin K.
Petersen" <martin.petersen@...cle.com>
Cc: Pavel Machek <pavel@....cz>, linux-leds@...r.kernel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-ide@...r.kernel.org, linux-scsi@...r.kernel.org
Subject: Re: [PATCH RFC 3/4] leds: add delay_on, delay_off and invert
attributes to disk trigger
On Fri, 2026-01-23 at 19:05 +0000, Markus Probst wrote:
> Add delay_on, delay_off and invert device attributes to leds using the
> disk trigger.
>
> Signed-off-by: Markus Probst <markus.probst@...teo.de>
> ---
> drivers/leds/trigger/ledtrig-disk.c | 194 +++++++++++++++++++++++++++++++++---
> 1 file changed, 182 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/leds/trigger/ledtrig-disk.c b/drivers/leds/trigger/ledtrig-disk.c
> index e9b87ee944f2..ed5ef83a5b35 100644
> --- a/drivers/leds/trigger/ledtrig-disk.c
> +++ b/drivers/leds/trigger/ledtrig-disk.c
> @@ -9,31 +9,201 @@
>
> #include <linux/kernel.h>
> #include <linux/init.h>
> +#include <linux/list.h>
> #include <linux/leds.h>
> +#include "../leds.h"
>
> -#define BLINK_DELAY 30
> +#define DEFAULT_BLINK_DELAY 30
>
> -DEFINE_LED_TRIGGER(ledtrig_disk);
> -DEFINE_LED_TRIGGER(ledtrig_disk_read);
> -DEFINE_LED_TRIGGER(ledtrig_disk_write);
> +struct ledtrig_disk_data {
> + unsigned long delay_on;
> + unsigned long delay_off;
> + unsigned int invert;
> +};
> +
> +static ssize_t led_delay_on_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct led_classdev *led_cdev = led_trigger_get_led(dev);
> + struct ledtrig_disk_data *disk_data = led_get_trigger_data(led_cdev);
> +
> + return sprintf(buf, "%lu\n", disk_data->delay_on);
> +}
> +
> +static ssize_t led_delay_on_store(struct device *dev,
> + struct device_attribute *attr, const char *buf, size_t size)
> +{
> + struct led_classdev *led_cdev = led_trigger_get_led(dev);
> + struct ledtrig_disk_data *disk_data = led_get_trigger_data(led_cdev);
> + unsigned long state;
> + ssize_t ret;
> +
> + ret = kstrtoul(buf, 10, &state);
> + if (ret)
> + return ret;
> +
> + disk_data->delay_on = state;
> +
> + return size;
> +}
> +
> +static ssize_t led_delay_off_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct led_classdev *led_cdev = led_trigger_get_led(dev);
> + struct ledtrig_disk_data *disk_data = led_get_trigger_data(led_cdev);
> +
> + return sprintf(buf, "%lu\n", disk_data->delay_off);
> +}
> +
> +static ssize_t led_delay_off_store(struct device *dev,
> + struct device_attribute *attr, const char *buf, size_t size)
> +{
> + struct led_classdev *led_cdev = led_trigger_get_led(dev);
> + struct ledtrig_disk_data *disk_data = led_get_trigger_data(led_cdev);
> + unsigned long state;
> + ssize_t ret;
> +
> + ret = kstrtoul(buf, 10, &state);
> + if (ret)
> + return ret;
> +
> + disk_data->delay_off = state;
> +
> + return size;
> +}
> +
> +static ssize_t led_invert_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct ledtrig_disk_data *disk_data =
> + led_trigger_get_drvdata(dev);
> +
> + return sprintf(buf, "%u\n", disk_data->invert);
> +}
> +
> +static ssize_t led_invert_store(struct device *dev,
> + struct device_attribute *attr, const char *buf, size_t size)
> +{
> + struct led_classdev *led_cdev = led_trigger_get_led(dev);
> + struct ledtrig_disk_data *disk_data = led_get_trigger_data(led_cdev);
> + unsigned long state;
> + int ret;
> +
> + ret = kstrtoul(buf, 0, &state);
> + if (ret)
> + return ret;
> +
> + led_set_brightness_nosleep(led_cdev, state ? LED_FULL : LED_OFF);
> + disk_data->invert = !!state;
> +
> + return size;
> +}
> +
> +static DEVICE_ATTR(delay_on, 0644, led_delay_on_show, led_delay_on_store);
> +static DEVICE_ATTR(delay_off, 0644, led_delay_off_show, led_delay_off_store);
> +static DEVICE_ATTR(invert, 0644, led_invert_show, led_invert_store);
> +
> +static struct attribute *ledtrig_disk_attrs[] = {
> + &dev_attr_delay_on.attr,
> + &dev_attr_delay_off.attr,
> + &dev_attr_invert.attr,
> + NULL
> +};
> +ATTRIBUTE_GROUPS(ledtrig_disk);
> +
> +static void pattern_init(struct led_classdev *led_cdev, struct ledtrig_disk_data *disk_data)
> +{
> + unsigned int size = 0;
> +
> + u32 *pattern __free(kfree) = led_get_default_pattern(led_cdev, &size);
> + if (!pattern)
> + return;
> +
> + if (size != 3) {
> + dev_warn(led_cdev->dev,
> + "Expected 3 but got %u values for delays + invert pattern\n",
> + size);
> + return;
> + }
> +
> + disk_data->delay_on = pattern[0];
> + disk_data->delay_off = pattern[1];
> + disk_data->invert = !!pattern[2];
> +}
> +
> +static int ledtrig_disk_activate(struct led_classdev *led_cdev)
> +{
> + struct ledtrig_disk_data *disk_data;
> +
> + disk_data = kzalloc(sizeof(*disk_data), GFP_KERNEL);
> + if (!disk_data)
> + return -ENOMEM;
> +
> + disk_data->delay_on = DEFAULT_BLINK_DELAY;
> + disk_data->delay_off = DEFAULT_BLINK_DELAY;
> +
> + led_set_trigger_data(led_cdev, disk_data);
> +
> + if (led_cdev->flags & LED_INIT_DEFAULT_TRIGGER) {
> + pattern_init(led_cdev, disk_data);
> + /*
> + * Mark as initialized even on pattern_init() error because
> + * any consecutive call to it would produce the same error.
> + */
> + led_cdev->flags &= ~LED_INIT_DEFAULT_TRIGGER;
> + }
> +
> + led_set_brightness_nosleep(led_cdev, disk_data->invert ? LED_FULL : LED_OFF);
> +
> + return 0;
> +}
> +
> +static struct led_trigger ledtrig_disk = {
> + .name = "disk-activity",
> + .activate = ledtrig_disk_activate,
> + .groups = ledtrig_disk_groups,
> +};
> +static struct led_trigger ledtrig_disk_read = {
> + .name = "disk-read",
> + .activate = ledtrig_disk_activate,
> + .groups = ledtrig_disk_groups,
> +};
> +static struct led_trigger ledtrig_disk_write = {
> + .name = "disk-write",
> + .activate = ledtrig_disk_activate,
> + .groups = ledtrig_disk_groups,
> +};
> +
> +static void ledtrig_disk_blink_oneshot(struct led_trigger *trig)
> +{
> + struct led_classdev *led_cdev;
> + struct ledtrig_disk_data *disk_data;
> +
> + rcu_read_lock();
> + list_for_each_entry_rcu(led_cdev, &trig->led_cdevs, trig_list) {
> + disk_data = led_get_trigger_data(led_cdev);
Also there is likely a race condition here, as there is a time gap
between the led being added to trig->led_cdevs and
ledtrig_disk_activate being run.
Is there a better way to avoid the race condition, than defining an own
list for each trigger?
Thanks
- Markus Probst
> + led_blink_set_oneshot(led_cdev, &disk_data->delay_on, &disk_data->delay_off,
> + disk_data->invert);
> + }
> + rcu_read_unlock();
> +}
>
> void ledtrig_disk_activity(bool write)
> {
> - led_trigger_blink_oneshot(ledtrig_disk, BLINK_DELAY, BLINK_DELAY, 0);
> + ledtrig_disk_blink_oneshot(&ledtrig_disk);
> if (write)
> - led_trigger_blink_oneshot(ledtrig_disk_write,
> - BLINK_DELAY, BLINK_DELAY, 0);
> + ledtrig_disk_blink_oneshot(&ledtrig_disk_write);
> else
> - led_trigger_blink_oneshot(ledtrig_disk_read,
> - BLINK_DELAY, BLINK_DELAY, 0);
> + ledtrig_disk_blink_oneshot(&ledtrig_disk_read);
> }
> EXPORT_SYMBOL(ledtrig_disk_activity);
>
> static int __init ledtrig_disk_init(void)
> {
> - led_trigger_register_simple("disk-activity", &ledtrig_disk);
> - led_trigger_register_simple("disk-read", &ledtrig_disk_read);
> - led_trigger_register_simple("disk-write", &ledtrig_disk_write);
> + led_trigger_register(&ledtrig_disk);
> + led_trigger_register(&ledtrig_disk_read);
> + led_trigger_register(&ledtrig_disk_write);
>
> return 0;
> }
Download attachment "signature.asc" of type "application/pgp-signature" (871 bytes)
Powered by blists - more mailing lists