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

[Markus Probst <markus.probst@posteo.de>]

[-- Attachment #1: Type: text/plain, Size: 2775 bytes --]

On Tue, 2026-01-27 at 10:32 +0100, Niklas Cassel wrote:
> On Mon, Jan 26, 2026 at 10:06:02PM +0000, Markus Probst wrote:
> > On Mon, 2026-01-26 at 10:00 +0100, Niklas Cassel wrote:
> > > 
> > > Why do we want to have this in kernel space?
> > Because there are more than enough devices that could make use of it.
> > 
> > Just search the term "NAS device" and you see rarely any devices for
> > which this wouldn't be useful.
> > 
> > The only reason the leds work on those devices currently, is because
> > they get shipped with a custom modified kernel by the manufacturer.
> > This shouldn't be a requirement for running Linux properly on a NAS
> > device with disk leds.
> 
> I understand why you want the feature. I just don't understand why we
> should add this feature to the kernel, rather than implement it in
> user space.
> 
> 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.
> 
> > > If we want something more complex than what is already there, then it
> > > is probably much better handled in user space, considering the amount
> > > of possible configuration options.
> > A userspace daemon by itself is possible, but I don't think it is the
> > best solution. Having an indicator for disk activity on a per-disk
> > basis seems like basic led functionality that should be present in the
> > kernel.
> 
> 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.

> > > Basically the same argument as used in:
> > > https://lore.kernel.org/linux-nvme/20220227234258.24619-1-ematsumiya@suse.de/T/#u
> > If I understood it corretly, the argument there is that led code
> > shouldn't be present in a fast path.
> > 
> > This does not apply to this scenario.
> 
> 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

@Pavel,@Lee: I would like to know your opinion on this.

Thanks
- Markus Probst

> 
> 
> Kind regards,
> Niklas

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 870 bytes --]