Re: [RFC PATCH 1/8] docs: Add block device LED trigger documentation

From: "Marek Behún" <kabel@kernel.org>
To: Ian Pilcher <arequipeno@gmail.com>
Cc: axboe@kernel.dk, kernelnewbies@kernelnewbies.org,
	linux-kernel@vger.kernel.org, linux-block@vger.kernel.org,
	pavel@ucw.cz, linux-leds@vger.kernel.org
Subject: Re: [RFC PATCH 1/8] docs: Add block device LED trigger documentation
Date: Thu, 29 Jul 2021 13:59:55 +0200	[thread overview]
Message-ID: <20210729135955.3e3f591c@thinkpad> (raw)
In-Reply-To: <20210729015344.3366750-2-arequipeno@gmail.com>

Dear Ian,

On Wed, 28 Jul 2021 20:53:37 -0500
Ian Pilcher <arequipeno@gmail.com> wrote:

> * Document the sysfs attributes (/sys/class/block/led_trigger_*
>   and /sys/class/block/${DEV}/led_trigger) that can be used to
>   create, list, and delete block device LED triggers and to
>   set and clear device/trigger associations.
> 
> * Pull API documentation from block/blk-ledtrig.c (once it
>   exists).
> 
> Signed-off-by: Ian Pilcher <arequipeno@gmail.com>

thank you for this proposal.

I don't really see the purpose for having multiple different block
device LED triggers. Moreover we really do not want userspace to be
able to add LED triggers with arbitrary names, and as many as the
userspace wants. There is no sense in making userspace be able to
create 10000 triggers. Also if userspace can create triggers with
arbitrary names, it could "steal" a name for a real trigger. For
example if netdev trigger is compiled as a module, and before loading
someone creates blockdev trigger with name "netdev", the loading of
netdev trigger will fail.

I would like the blkdev trigger to work in a similar way the netdev
trigger works:
- only one trigger, with name "blkdev"
- when activated on a LED, new sysfs files will be created:
  * device_name, where user can write sda1, vdb, ...
  * read (binary value, 1 means blink on read)
  * write (binary value, 1 means blink on write)
  * interval (blink interval)
  Note that device_name could allow multiple names, in theory...
  Also some other disk states may be included, like error, or something
- also the blinking itself can be done as is done netdev trigger: every
  50ms the work function would look at blkdev stats, and if current
  stat (number of bytes read/written) is different from previous, then
  blink the LED

Marek

_______________________________________________
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies