| Message ID | 20210729015344.3366750-2-arequipeno@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add configurable block device LED triggers | expand |
On 7/28/21 10:09 PM, Valdis Klētnieks wrote: >> + # cat /sys/class/block/led_trigger_list >> + baz: 0 >> + bar: 0 >> + foo: 0 > > This looks like an abuse of the "one entry one value" rule for sysfs. > Perhaps this should be a directory /sys/class/block/defined_triggers/ > and separate files under that for foo, bar, and baz? That would probably > make reference counting a lot easier as well.... Indeed it is. Funny that you should mention using a subdirectory. I originally wanted to put all of the trigger-related stuff into /sys/class/block/led_triggers/, but I couldn't find any API to create a subdirectory for *class* attributes (only for device attributes), nor do I see any such subdirectories on my system. # find /sys/class -type d | egrep '^/sys/class/[^/]+/' (no output) Is is possible to create subdirectories for class attributes?
On 7/29/21 6:59 AM, Marek Behún wrote: > I don't really see the purpose for having multiple different block > device LED triggers. Is there a different/better way to control per-device LEDs? (I'm thinking of something like my NAS, which has 5 drive bays, each with its own activity LED.) > Moreover we really do not want userspace to be > able to add LED triggers with arbitrary names, and as many as the > userspace wants. To be slightly flippant, why not? "Userspace" in this case is the system/device administrator. They presumably know what LEDs they have and what they want to use them for, something which the kernel cannot know (assuming a "generic" disto kernel). > There is no sense in making userspace be able to > create 10000 triggers. It would certainly be possible to impose a limit on the number of triggers that could be created. But then someone has to decide what that limit should be. Personally, I lean very much toward giving the system administrator the freedom to configure their system as they see fit, even if that means that they can break it. (Where "break" basically means that they need to reboot.) > 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. Would adding a prefix to the LED trigger name address your concern about arbitrary names and potential conflicts? I.e. the system administrator creates a block device LED trigger named "foo", and it shows up as an LED trigger named "blkdev:foo" (or something like that). > 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 How would you support multiple, per-device LEDs (the NAS use case above) in this scheme? > - 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 Is there a reason that you prefer this approach to simply having the block layer "fire" the trigger? Thanks for the feedback!
On Thu, Jul 29, 2021 at 10:52:06AM -0500, Ian Pilcher wrote: > On 7/28/21 10:09 PM, Valdis Klētnieks wrote: > > > + # cat /sys/class/block/led_trigger_list > > > + baz: 0 > > > + bar: 0 > > > + foo: 0 > > > > This looks like an abuse of the "one entry one value" rule for sysfs. > > Perhaps this should be a directory /sys/class/block/defined_triggers/ > > and separate files under that for foo, bar, and baz? That would probably > > make reference counting a lot easier as well.... > > Indeed it is. > > Funny that you should mention using a subdirectory. I originally wanted > to put all of the trigger-related stuff into > /sys/class/block/led_triggers/, but I couldn't find any API to create a > subdirectory for *class* attributes (only for device attributes), nor do > I see any such subdirectories on my system. Add a name to your attribute group and sysfs creates the subdirectory automagically for you. thanks, greg k-h
diff --git a/Documentation/block/index.rst b/Documentation/block/index.rst index 86dcf7159f99..a125ecdb4c7b 100644 --- a/Documentation/block/index.rst +++ b/Documentation/block/index.rst @@ -25,3 +25,4 @@ Block stat switching-sched writeback_cache_control + led-triggers diff --git a/Documentation/block/led-triggers.rst b/Documentation/block/led-triggers.rst new file mode 100644 index 000000000000..a67e06c68073 --- /dev/null +++ b/Documentation/block/led-triggers.rst @@ -0,0 +1,124 @@ +.. SPDX-License-Identifier: GPL-2.0 + +============ +LED Triggers +============ + +Available when ``CONFIG_BLK_LED_TRIGGERS=y``. + +sysfs interface +=============== + +Create a new block device LED trigger:: + + # echo foo > /sys/class/block/led_trigger_new + +The name must be unique among all LED triggers (not just block device LED +triggers). + +Create two more:: + + # echo bar baz > /sys/class/block/led_trigger_new + +List the triggers:: + + # cat /sys/class/block/led_trigger_list + baz: 0 + bar: 0 + foo: 0 + +(The number after each trigger is its reference count.) + +Associate a trigger with a block device:: + + # cat /sys/class/block/sda/led_trigger + (none) + + # echo foo > /sys/class/block/sda/led_trigger + # cat /sys/class/block/sda/led_trigger + foo + +Note that ``foo``'s reference count has increased, and it cannot be deleted:: + + # cat /sys/class/block/led_trigger_list + baz: 0 + bar: 0 + foo: 1 + + # echo foo > /sys/class/block/led_trigger_del + -bash: echo: write error: Device or resource busy + + # dmesg | tail -n 1 + [23176.475424] blockdev LED trigger foo still in use + +Associate the ``foo`` trigger with an LED:: + + # cat /sys/class/leds/input1::scrolllock/trigger + none usb-gadget usb-host rc-feedback [kbd-scrolllock] kbd-numlock + kbd-capslock kbd-kanalock kbd-shiftlock kbd-altgrlock kbd-ctrllock + kbd-altlock kbd-shiftllock kbd-shiftrlock kbd-ctrlllock kbd-ctrlrlock + disk-activity disk-read disk-write ide-disk mtd nand-disk panic + audio-mute audio-micmute rfkill-any rfkill-none foo bar baz + + # echo foo > /sys/class/leds/input1::scrolllock/trigger + + # cat /sys/class/leds/input1::scrolllock/trigger + none usb-gadget usb-host rc-feedback [kbd-scrolllock] kbd-numlock + kbd-capslock kbd-kanalock kbd-shiftlock kbd-altgrlock kbd-ctrllock + kbd-altlock kbd-shiftllock kbd-shiftrlock kbd-ctrlllock kbd-ctrlrlock + disk-activity disk-read disk-write ide-disk mtd nand-disk panic + audio-mute audio-micmute rfkill-any rfkill-none [foo] bar baz + +Reads and writes to ``sda`` should now cause the scroll lock LED on your +keyboard to blink (assuming that it has one). + +Multiple devices can be associated with a trigger:: + + # echo foo > /sys/class/block/sdb/led_trigger + + # cat /sys/class/block/led_trigger_list + baz: 0 + bar: 0 + foo: 2 + +Activity on either ``sda`` or ``sdb`` should now be shown by your scroll lock +LED. + +Clear ``sda``'s LED trigger:: + + # echo > /sys/class/block/sda/led_trigger + + # cat /sys/class/block/sda/led_trigger + (none) + + # cat /sys/class/block/led_trigger_list + baz: 0 + bar: 0 + foo: 1 + +And ``sdb``'s trigger:: + + # echo > /sys/class/block/sdb/led_trigger + +Delete the triggers:: + + # echo foo bar baz > /sys/class/block/led_trigger_del + + # cat /sys/class/block/led_trigger_list + + # cat /sys/class/leds/input1::scrolllock/trigger + none usb-gadget usb-host rc-feedback [kbd-scrolllock] kbd-numlock + kbd-capslock kbd-kanalock kbd-shiftlock kbd-altgrlock kbd-ctrllock + kbd-altlock kbd-shiftllock kbd-shiftrlock kbd-ctrlllock kbd-ctrlrlock + disk-activity disk-read disk-write ide-disk mtd nand-disk panic + audio-mute audio-micmute rfkill-any rfkill-none + +Also see **Userspace LEDs** (``Documentation/leds/uleds.rst``). + +Kernel API +========== + +``#include <linux/blk-ledtrig.h>`` + +.. kernel-doc:: block/blk-ledtrig.c + :export:
* 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> --- Documentation/block/index.rst | 1 + Documentation/block/led-triggers.rst | 124 +++++++++++++++++++++++++++ 2 files changed, 125 insertions(+) create mode 100644 Documentation/block/led-triggers.rst