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

[Niklas Cassel <cassel@kernel.org>]

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.


> > 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


> > 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.


Kind regards,
Niklas