Re: [PATCH RFC 0/4] leds: extend disk trigger

[Damien Le Moal <dlemoal@kernel.org>]

On 1/28/26 12:34 AM, Markus Probst wrote:
>> Having a user space implementation for your feature would also allow
>> an upstream kernel, without the need for any custom kernel patches.
> Only because it can be done in userspace, doesn't mean it should be.

Yes it should. Maintaining userspace is far easier than forcing kernel changes
onto users to get blinking LEDs. So unless you have a very strong argument for
doing it in the kernel, userspace is probably the right approach. That will
apply to any block device, and not just ATA devices. E.g. NAS with M.2 NVMe
storage can work with the same.

>> There seems to be existing user space applications that handles this,
>> I think both the daemon I linked to before, which uses /sys/block/<dev>/stat
>> which is thus per device and not per port, and e.g. this:
>> https://linux.die.net/man/8/ledmon
>> https://github.com/md-raid-utilities/ledmon
>> https://github.com/md-raid-utilities/ledmon/blob/main/src/lib/ahci.c
> As far as I can tell, this daemon doesn't actually use the LED
> Subsystem, but instead leds directly connected to the storage
> controller.
> But yes, I would be capable of coding such daemon.

Then let's try. That will allow checking if anything is missing in the kernel
interface to do that nicely.

>> I think my main concern is that I don't think we should bloat the kernel
>> for a complex feature that can just as well be implemented in user space.
> It is still unclear to me if you worry about the complexity in
> drivers/ata/libata-* or drivers/leds/trigger/ledtrig-disk.c

It is not so much about complexity but rather the fact that controlling
blinking LEDs in the hot IO path is not desirable. While SATA HDDs will be less
sensible to the delays caused by the calls to the LED control functions
compared to fast NVMe SSDs, we do also need to think about much faster SATA
SSDs. So instead of risking performance regressions, let's try to do this in
userspace first.


-- 
Damien Le Moal
Western Digital Research