Pavel,
I see that you are active now - could you please prioritise this one.
On Tue, 27 Dec 2022, Ian Pilcher wrote:
> Summary
> =======
>
> These patches add a new "blkdev" LED trigger that blinks LEDs in
> response to disk (or other block device) activity. The first patch is
> purely documentation, and the second patch adds the trigger.
>
> It operates very much like the netdev trigger. Device activity
> counters are checked periodically, and LEDs are blinked if the correct
> type of activity has occurred since the last check. The trigger has no
> impact on the actual I/O path.
>
> The trigger is extremely configurable. An LED can be configured to
> blink in response to any type (or combination of types) of block device
> activity - reads, writes, discards, or cache flushes. The frequency
> with which device activity is checked and the duration of LED blinks
> can also be set.
>
> The trigger supports many-to-many "link" relationships between block
> devices and LEDs. An LED can be linked to multiple block devices, and
> a block device can be linked to multiple LEDs. To support these
> many-to-many links with a sysfs API, the trigger uses write-only
> attributes to create and remove link relationships:
>
> * link_dev_by_path
> * unlink_dev_by_path
> * unlink_dev_by_name
>
> Existing relationships are shown as symbolic links in subdirectories
> beneath the block device and LED sysfs directories:
>
> * /sys/class/block/<DEVICE>/linked_leds
> * /sys/class/leds/<LED>/linked_devices
>
> As their names indicate, link_dev_by_path and unlink_dev_by_path each
> take a device special file path (e.g. /dev/sda), rather than a kernel
> device name. A block device can be unlinked from an LED by writing its
> kernel name to the LED's unlink_dev_by_name attribute, but creating a
> link does require a path. This is a consequence of the fact that the
> block subsystem does not provide an API to get a block device by its
> kernel name; only device special file paths (or device major and minor
> numbers) are supported.
>
> (I hope that if this module is accepted, it might provide a case for
> adding a "by name" API to the block subsystem. A link_dev_by_name
> attribute could then be added to this trigger.)
>
> The trigger can be built as a module or built in to the kernel.
>
> Changes from v12:
> =================
>
> * Use sysfs_emit(), instead of sprintf() in sysfs attribute show
> functions.
>
> Changes from v11:
> =================
>
> * Add unlink_dev_by_name attribute, so a block device can be unlinked
> from an LED with its kernel name.
>
> * Fix interval/frequency confusion in documentation.
>
> Changes from v10:
> =================
>
> * Fix kfree() of wrong pointer in blkdev_trig_get_bdev().
> * Fix typo in ledtrig-blkdev.rst.
>
> Changes from v9:
> ================
>
> No changes to sysfs API or module functionality.
>
> Readability changes:
>
> * Added overview and data type comments to describe module structure.
>
> * Reordered module source; eliminated almost all forward declarations.
>
> * Consistently refer to blkdev_trig_led structs as "BTLs."
>
> * Refactored LED-block device unlink function into separate variants for
> releasing & not releasing cases; eliminate enum type used as flag.
>
> Changes from v8:
> ================
>
> * Change sysfs attribute names:
> - link_device ==> link_dev_by_path
> - unlink_device ==> unlink_dev_by_path
>
> * Update documentation for changed attribute names
>
> * Minor code & comment cleanups
>
> Changes from v7:
> ================
>
> Fix blkdev_trig_activate() - Lock 'blkdev_trig_mutex' before accessing
> 'blkdev_trig_next_index'.
>
> Changes from v6:
> ================
>
> Remove incorrect use of get_jiffies_64(). We use the helper functions in
> include/linux/jiffies.h for all time comparisons, so jiffies rolling over
> on 32-bit platforms isn't a problem.
>
> Changes from v5:
> ================
>
> sysfs API changes:
>
> * Frequency with which the block devices associated with an LED are
> checked for activity is now a per-LED setting ('check_interval' device
> attribute replaces 'interval' class attribute).
>
> * 'mode' device attribute (read/write/rw) is replaced by 4 separate
> attributes - 'blink_on_read', 'blink_on_write', 'blink_on_discard', and
> 'blink_on_flush'.
>
> Logic changes:
>
> * Use jiffies instead of static "generation" variable.
>
> * LED mode is now a bitmask - 1 bit per read, write, discard, and flush.
>
> * When updating block device I/O stats, save separate I/O counter ('ios')
> and timestamp ('last_activity') for each activity type, along with
> 'last_checked' timestamp.
>
> * When checking an LED, save 'last_checked' timestamp.
>
> * When checking LEDs (in delayed work), determine when the next check
> needs to be performed (based on each LED's 'last_checked' and
> 'check_jiffies' values) and schedule the next check accordingly. (See
> blkdev_trig_check() at ledtrig-blkdev.c:661.)
>
> * When linking a block device to an LED, modify the delayed work schedule
> if necessary. (See blkdev_trig_sched_led() at ledtrig-blkdev.c:416.)
>
> Style changes:
>
> * "Prefix" of data types, static variables, function names, etc. is
> changed to 'blkdev_trig' ('BLKDEV_TRIG' for constants).
>
> * Don't declare function parameters and local variables as const.
>
> * Don't explicitly compare return values to 0 - i.e. 'if (ret == 0)'.
> Change variable name to 'err' and use 'if (err)' idiom.
>
> * In error path, return directly when no cleanup is required (instead of
> jumping to a single exit point).
>
> * Use kzalloc(), rather than kmalloc(), to allocate per-LED structs.
>
> Changes from v4:
> ================
>
> * Use xarrays, rather than lists, to model "links" between LEDs and block
> devices. This allows many-to-many relationships without the need for a
> separate link object.
>
> * When resolving (getting) a block device by path, don't retry with
> "/dev/" prepended to the path in the ENOENT case.
>
> * Use an enum, rather than a boolean, to tell led_bdev_unlink() whether
> the block device is being released or not.
>
> * Use preprocessor constant, rather than magic number, for the mode passed
> to blkdev_get_by_path() and blkdev_put().
>
> * Split the data structure used by mode attribute show & store functions
> into 2 separate arrays and move them into the functions that use them.
>
> Changes from v3:
> ================
>
> * Use blkdev_get_by_path() to "resolve" block devices
> (struct block_device). With this change, there are now no changes
> required to the block subsystem, so there are only 2 patches in this
> series.
>
> * link_device and unlink_device attributes now take paths to block device
> special files (e.g. /dev/sda), rather than kernel names. Symbolic
> links also work.
>
> If the path written to the attribute doesn't exist (-ENOENT), we re-try
> with /dev/ prepended, so "simple" names like sda will still work as long
> as the corresponding special file exists in /dev.
>
> * Fixed a bug that could cause "phantom" blinks because of old device
> activity that was not recognized at the correct time.
>
> * (Slightly) more detailed commit message for the patch that adds the
> trigger code. As with v3, the real details are found in the comments
> in the source file.
>
> Changes from v2:
> ================
>
> * Allow LEDs to be "linked" to partitions, as well as whole devices.
> Internally, the trigger now works with block_device structs, rather
> than gendisk structs.
>
> (Investigating the lifecycle of block_device structs led me to
> discover the device resource API, so ...)
>
> * Use the device resource API to manage the trigger's per-block device
> data structure (struct led_bdev_bdi). The trigger now uses a release
> function to remove references to block devices that have been removed.
>
> Because the release function is automatically called by the driver core,
> there is no longer any need for the block layer to explictly call the
> trigger's cleanup function.
>
> * Since there is no need to provide a built-in "stub" cleanup function
> when the trigger is built as a module, I have removed the always
> built-in "core" portion of the trigger.
>
> * Without a built-in component, the module does need access to the
> block_class symbol. The second patch in this series exports the symbol
> to the LEDTRIG_BLKDEV namespace and explains the reason for doing so.
>
> * Changed the interval sysfs attribute from a device attribute to a class
> attribute. It's single value that applies to all LEDs, so it didn't
> make sense as a device atribute.
>
> * As requested, I am posting the trigger code (ledtrig-blkdev.c) as a
> single patch. This eliminates the commit messages that would otherwise
> describe sections of the code, so I have added fairly extensive comments
> to each function.
>
> Changes from v1:
> ================
>
> * Use correct address for LKML.
>
> * Renamed the sysfs attributes used to manage and view the set of block
> devices associated ("linked") with an LED.
>
> - /sys/class/leds/<LED>/link_device to create associations
>
> - /sys/class/leds/<LED>/unlink_device to remove associations
>
> - /sys/class/leds/<LED>/linked_devices/ contains symlinks to all block
> devices associated with the LED
>
> - /sys/block/<DEVICE>/linked_leds (which only exists when the device is
> associated with at least one LED) contains symlinks to all LEDs with
> which the device is associated
>
> link_device and unlink_device are write-only attributes, each of which
> represents a single action, rather than any state. (The current state
> is shown by the symbolic links in the <LED>/linked_devices/ and
> <DEVICE>/linked_leds/ directories.)
>
> * Simplified sysfs attribute store functions. link_device and
> unlink_device no longer accept multiple devices at once, but this was
> really just an artifact of the way that sysfs repeatedly calls the
> store function when it doesn't "consume" all of its input, and it
> seemed to be confusing and unpopular anyway.
>
> * Use DEVICE_ATTR_* macros (rather than __ATTR) for the sysfs attributes.
>
> * Removed all pr_info() "system administrator error" messages.
>
> * Different minimum values for LED blink time (10 ms) and activity check
> interval (25 ms).
>
> v1 summary:
> ===========
>
> This patch series adds a new "blkdev" LED trigger for disk (or other block
> device) activity LEDs.
>
> It has the following functionality.
>
> * Supports all types of block devices, including virtual devices
> (unlike the existing disk trigger which only works with ATA devices).
>
> * LEDs can be configured to show read activity, write activity, or both.
>
> * Supports multiple devices and multiple LEDs in arbitrary many-to-many
> configurations. For example, it is possible to configure multiple
> devices with device-specific read activity LEDs and a shared write
> activity LED. (See Documentation/leds/ledtrig-blkdev.rst in the first
> patch.)
>
> * Doesn't add any overhead in the I/O path. Like the netdev LED trigger,
> it periodically checks the configured devices for activity and blinks
> its LEDs as appropriate.
>
> * Blink duration (per LED) and interval between activity checks (global)
> are configurable.
>
> * Requires minimal changes to the block subsystem.
>
> - Adds 1 pointer to struct gendisk,
>
> - Adds (inline function) call in device_add_disk() to ensure that the
> pointer is initialized to NULL (as protection against any drivers
> that allocate a gendisk themselves and don't use kzalloc()), and
>
> - Adds call in del_gendisk() to remove a device from the trigger when
> that device is being removed.
>
> These changes are all in patch #4, "block: Add block device LED trigger
> integrations."
>
> * The trigger can be mostly built as a module.
>
> When the trigger is modular, a small portion is built in to provide a
> "stub" function which can be called from del_gendisk(). The stub calls
> into the modular code via a function pointer when needed. The trigger
> also needs the ability to find gendisk's by name, which requires access
> to the un-exported block_class and disk_type symbols.
>
> Ian Pilcher (2):
> docs: Add block device (blkdev) LED trigger documentation
> leds: trigger: Add block device LED trigger
>
> Documentation/ABI/stable/sysfs-block | 10 +
> .../testing/sysfs-class-led-trigger-blkdev | 78 ++
> Documentation/leds/index.rst | 1 +
> Documentation/leds/ledtrig-blkdev.rst | 158 +++
> drivers/leds/trigger/Kconfig | 9 +
> drivers/leds/trigger/Makefile | 1 +
> drivers/leds/trigger/ledtrig-blkdev.c | 1221 +++++++++++++++++
> 7 files changed, 1478 insertions(+)
> create mode 100644 Documentation/ABI/testing/sysfs-class-led-trigger-blkdev
> create mode 100644 Documentation/leds/ledtrig-blkdev.rst
> create mode 100644 drivers/leds/trigger/ledtrig-blkdev.c
>
>
> base-commit: 1b929c02afd37871d5afb9d498426f83432e71c2
> --
> 2.38.1
>
--
Lee Jones [李琼斯]
