| Message ID | 20221227225226.546489-1-arequipeno@gmail.com |
|---|---|
| Headers | show |
| Series | Introduce block device LED trigger | expand |
On Tue, 27 Dec 2022, Ian Pilcher wrote: > Summary > ======= FYI: Leaving this one for Pavel. > 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 >
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 >
On 3/1/23 08:27, Lee Jones wrote: > Pavel, > > I see that you are active now - could you please prioritise this one. > Lee - Just FYI, Pavel did respond. Unfortunately, he doesn't feel that this can go in with its current sysfs interface, and making the change that he wants would require changes to the block subsystem (adding an in- kernel API to look up a block device by its kernel name, rather than its major & minor numbers or a special file path). Similar changes have been rejected in the past by the block subsystem maintainers. The position I have seen is that major & minor numbers, or device special files from which they can be determined, is *the* interface to block devices. I've also had some pretty negative experiences when interacting with the block subsystem community - unnecessary profanity, etc. Given that history, I don't see much prospect that I (an unknown newb) would succeed in convincing the block subsystem maintainers to add the API required to implement the interface that Pavel wants. So I'm pretty much done trying to push this thing unless something changes that leads me to think that there's actually a decent chance of success. Hopefully that makes sense.
On Wed, 01 Mar 2023, Ian Pilcher wrote: > On 3/1/23 08:27, Lee Jones wrote: > > Pavel, > > > > I see that you are active now - could you please prioritise this one. > > > > Lee - > > Just FYI, Pavel did respond. Unfortunately, he doesn't feel that this > can go in with its current sysfs interface, and making the change that > he wants would require changes to the block subsystem (adding an in- > kernel API to look up a block device by its kernel name, rather than its > major & minor numbers or a special file path). > > Similar changes have been rejected in the past by the block subsystem > maintainers. The position I have seen is that major & minor numbers, > or device special files from which they can be determined, is *the* > interface to block devices. > > I've also had some pretty negative experiences when interacting with the > block subsystem community - unnecessary profanity, etc. > > Given that history, I don't see much prospect that I (an unknown newb) > would succeed in convincing the block subsystem maintainers to add the > API required to implement the interface that Pavel wants. So I'm pretty > much done trying to push this thing unless something changes that leads > me to think that there's actually a decent chance of success. > > Hopefully that makes sense. I understand and empathise with the situation. Pavel is the boss here, so I cannot over-rule the decisions he makes. Hopefully the two of you can find some middle ground and work something out between you. Best of luck.
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