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 (link_dev_by_path and unlink_dev_by_path) to create and
remove link relationships. Existing links are shown as symbolic links
in subdirectories beneath the block device and LED sysfs directories
(/sys/class/block/<DEVICE>/linked_leds and
/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. This is required, because 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. link_dev_by_name and
unlink_dev_by_name could then be added to this trigger.)
The trigger can be built as a module or built in to the kernel.
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 | 68 +
Documentation/leds/index.rst | 1 +
Documentation/leds/ledtrig-blkdev.rst | 155 +++
drivers/leds/trigger/Kconfig | 9 +
drivers/leds/trigger/Makefile | 1 +
drivers/leds/trigger/ledtrig-blkdev.c | 1177 +++++++++++++++++
7 files changed, 1421 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: 42e66b1cc3a070671001f8a1e933a80818a192bf
--
2.37.2
Hi Ian,
with a heavily patched Linux 6.0-rc4 with kfence, kmemleak and slub_debug I get the
following splat at boot:
Sep 07 11:33:11 kernel: =============================================================================
Sep 07 11:33:11 kernel: BUG kmalloc-16 (Not tainted): Object already free
Sep 07 11:33:11 kernel: -----------------------------------------------------------------------------
Sep 07 11:33:11 kernel: Allocated in kernfs_fop_write_iter+0x178/0x200 age=1 cpu=0 pid=453
Sep 07 11:33:11 kernel: __slab_alloc.constprop.0+0x42/0x80
Sep 07 11:33:11 kernel: __kmalloc+0x334/0x3a0
Sep 07 11:33:11 kernel: kernfs_fop_write_iter+0x178/0x200
Sep 07 11:33:11 kernel: vfs_write+0x268/0x430
Sep 07 11:33:11 kernel: ksys_write+0x6f/0xf0
Sep 07 11:33:11 kernel: do_syscall_64+0x5c/0x90
Sep 07 11:33:11 kernel: entry_SYSCALL_64_after_hwframe+0x63/0xcd
Sep 07 11:33:11 kernel: Freed in blkdev_trig_get_bdev+0x47/0x60 [ledtrig_blkdev] age=1 cpu=0 pid=453
Sep 07 11:33:11 kernel: kfree+0x374/0x3b0
Sep 07 11:33:11 kernel: blkdev_trig_get_bdev+0x47/0x60 [ledtrig_blkdev]
Sep 07 11:33:11 kernel: link_dev_by_path_store+0x5c/0x3f0 [ledtrig_blkdev]
Sep 07 11:33:11 kernel: kernfs_fop_write_iter+0x11f/0x200
Sep 07 11:33:11 kernel: vfs_write+0x268/0x430
Sep 07 11:33:11 kernel: ksys_write+0x6f/0xf0
Sep 07 11:33:11 kernel: do_syscall_64+0x5c/0x90
Sep 07 11:33:11 kernel: entry_SYSCALL_64_after_hwframe+0x63/0xcd
Sep 07 11:33:11 kernel: Slab 0xffffeb91446ad1c0 objects=32 used=31 fp=0xffff912c1ab47b10 flags=0x4000000000000201(locked|slab|zone=2)
Sep 07 11:33:11 kernel: Object 0xffff912c1ab47b10 @offset=2832 fp=0x0000000000000000
Sep 07 11:33:11 kernel: Redzone ffff912c1ab47b00: bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb ................
Sep 07 11:33:11 kernel: Object ffff912c1ab47b10: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b a5 kkkkkkkkkkkkkkk.
Sep 07 11:33:11 kernel: Redzone ffff912c1ab47b20: bb bb bb bb bb bb bb bb ........
Sep 07 11:33:11 kernel: Padding ffff912c1ab47b70: 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a ZZZZZZZZZZZZZZZZ
[...]
Sep 07 11:33:11 kernel: Call Trace:
Sep 07 11:33:11 kernel: <TASK>
Sep 07 11:33:11 kernel: dump_stack_lvl+0x37/0x4a
Sep 07 11:33:11 kernel: object_err+0x2f/0x42
Sep 07 11:33:11 kernel: free_debug_processing.cold+0x9c/0x126
Sep 07 11:33:11 kernel: ? kernfs_fop_write_iter+0xa0/0x200
Sep 07 11:33:11 kernel: __slab_free+0x265/0x450
Sep 07 11:33:11 kernel: ? _raw_spin_lock_irqsave+0x1b/0x50
Sep 07 11:33:11 kernel: ? _raw_spin_unlock_irqrestore+0x22/0x40
Sep 07 11:33:11 kernel: ? kernfs_fop_write_iter+0xa0/0x200
Sep 07 11:33:11 kernel: kfree+0x374/0x3b0
Sep 07 11:33:11 kernel: kernfs_fop_write_iter+0xa0/0x200
Sep 07 11:33:11 kernel: vfs_write+0x268/0x430
Sep 07 11:33:11 kernel: ksys_write+0x6f/0xf0
Sep 07 11:33:11 kernel: do_syscall_64+0x5c/0x90
Sep 07 11:33:11 kernel: ? do_syscall_64+0x6b/0x90
Sep 07 11:33:11 kernel: ? do_syscall_64+0x6b/0x90
Sep 07 11:33:11 kernel: entry_SYSCALL_64_after_hwframe+0x63/0xcd
Sep 07 11:33:11 kernel: RIP: 0033:0x74dc50050e94
Sep 07 11:33:11 kernel: Code: 15 f9 0e 0e 00 f7 d8 64 89 02 48 c7 c0 ff ff ff ff eb b7 0f 1f 00 f3 0f 1e fa 80 3d 8d 96 0e 00 00 74 13 b8 01 00 00 00 0f 05 <48> 3>
Sep 07 11:33:11 kernel: RSP: 002b:00007fff526d4058 EFLAGS: 00000202 ORIG_RAX: 0000000000000001
Sep 07 11:33:11 kernel: RAX: ffffffffffffffda RBX: 000000000000000c RCX: 000074dc50050e94
Sep 07 11:33:11 kernel: RDX: 000000000000000c RSI: 0000620ac0072430 RDI: 0000000000000005
Sep 07 11:33:11 kernel: RBP: 0000620ac0072430 R08: 0000620ac00852a0 R09: 007265776f703a3a
Sep 07 11:33:11 kernel: R10: 0000000000000000 R11: 0000000000000202 R12: 0000000000000005
Sep 07 11:33:11 kernel: R13: 0000000000000000 R14: 0000000000000005 R15: 0000620ac00852a0
Sep 07 11:33:11 kernel: </TASK>
Sep 07 11:33:11 kernel: Disabling lock debugging due to kernel taint
Sep 07 11:33:11 kernel: FIX kmalloc-16: Object at 0xffff912c1ab47b10 not freed
I'm not 100% sure if this is an issue with ledtrig_blkdev or something else,
but I thought I'll let you know about it.
I have not been able to test this on a vanilla kernel yet.
Other than that, I hope this patchset gets included in upstream.
I have been using it for a long time now and found it very useful.
Cheers,
Tor
On 9/7/22 07:16, [email protected] wrote:
> Hi Ian,
>
> with a heavily patched Linux 6.0-rc4 with kfence, kmemleak and slub_debug I get the
> following splat at boot:
Sorry about that! I'm not sure how that slipped throgh, as I was sure
that I tested the new version before I sent it off.
Basically, I messed up while cleaning up the function parameter names,
so you need to apply this:
--- drivers/leds/trigger/ledtrig-blkdev.c.old 2022-09-07
10:00:26.194484681 -0500
+++ drivers/leds/trigger/ledtrig-blkdev.c 2022-09-04
11:36:16.107690614 -0500
@@ -540,7 +540,7 @@
return ERR_PTR(-ENOMEM);
bdev = blkdev_get_by_path(strim(buf), BLKDEV_TRIG_FMODE,
THIS_MODULE);
- kfree(path);
+ kfree(buf);
return bdev;
}
> I'm not 100% sure if this is an issue with ledtrig_blkdev or something else,
> but I thought I'll let you know about it.
> I have not been able to test this on a vanilla kernel yet.
Defnintely my fault.
> Other than that, I hope this patchset gets included in upstream.
> I have been using it for a long time now and found it very useful.
It's really hard to know if anyone is interested in/using this, so
that's great to hear.
--
========================================================================
Google Where SkyNet meets Idiocracy
========================================================================
> Ian Pilcher <[email protected]> hat am 07.09.2022 15:11 GMT geschrieben:
>
>
> On 9/7/22 07:16, [email protected] wrote:
> > Hi Ian,
> >
> > with a heavily patched Linux 6.0-rc4 with kfence, kmemleak and slub_debug I get the
> > following splat at boot:
>
> Sorry about that! I'm not sure how that slipped throgh, as I was sure
> that I tested the new version before I sent it off.
>
> Basically, I messed up while cleaning up the function parameter names,
> so you need to apply this:
>
> --- drivers/leds/trigger/ledtrig-blkdev.c.old 2022-09-07
> 10:00:26.194484681 -0500
> +++ drivers/leds/trigger/ledtrig-blkdev.c 2022-09-04
> 11:36:16.107690614 -0500
> @@ -540,7 +540,7 @@
> return ERR_PTR(-ENOMEM);
>
> bdev = blkdev_get_by_path(strim(buf), BLKDEV_TRIG_FMODE,
> THIS_MODULE);
> - kfree(path);
> + kfree(buf);
> return bdev;
> }
>
This fixes the issue, no more errors.
Thank you!
> > I'm not 100% sure if this is an issue with ledtrig_blkdev or something else,
> > but I thought I'll let you know about it.
> > I have not been able to test this on a vanilla kernel yet.
>
> Defnintely my fault.
>
> > Other than that, I hope this patchset gets included in upstream.
> > I have been using it for a long time now and found it very useful.
>
> It's really hard to know if anyone is interested in/using this, so
> that's great to hear.
When and if you send a v11, you can add my
Tested-by: Tor Vic <[email protected]>
>
> --
> ========================================================================
> Google Where SkyNet meets Idiocracy
> ========================================================================