2022-12-27 22:57:17

by Ian Pilcher

[permalink] [raw]
Subject: [PATCH v13 0/2] Introduce block device LED trigger

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


2022-12-27 23:05:15

by Ian Pilcher

[permalink] [raw]
Subject: [PATCH v13 2/2] leds: trigger: Add block device LED trigger

Add "blkdev" LED trigger to blink LEDs in response to block device
activity.

Add LEDS_TRIGGER_BLKDEV (tristate) config option to control building of
the trigger.

Signed-off-by: Ian Pilcher <[email protected]>
Tested-by: Tor Vic <[email protected]> (version 12)
---
drivers/leds/trigger/Kconfig | 9 +
drivers/leds/trigger/Makefile | 1 +
drivers/leds/trigger/ledtrig-blkdev.c | 1221 +++++++++++++++++++++++++
3 files changed, 1231 insertions(+)
create mode 100644 drivers/leds/trigger/ledtrig-blkdev.c

diff --git a/drivers/leds/trigger/Kconfig b/drivers/leds/trigger/Kconfig
index dc6816d36d06..bda249068182 100644
--- a/drivers/leds/trigger/Kconfig
+++ b/drivers/leds/trigger/Kconfig
@@ -154,4 +154,13 @@ config LEDS_TRIGGER_TTY

When build as a module this driver will be called ledtrig-tty.

+config LEDS_TRIGGER_BLKDEV
+ tristate "LED Trigger for block devices"
+ depends on BLOCK
+ help
+ The blkdev LED trigger allows LEDs to be controlled by block device
+ activity (reads and writes).
+
+ See Documentation/leds/ledtrig-blkdev.rst.
+
endif # LEDS_TRIGGERS
diff --git a/drivers/leds/trigger/Makefile b/drivers/leds/trigger/Makefile
index 25c4db97cdd4..d53bab5d93f1 100644
--- a/drivers/leds/trigger/Makefile
+++ b/drivers/leds/trigger/Makefile
@@ -16,3 +16,4 @@ obj-$(CONFIG_LEDS_TRIGGER_NETDEV) += ledtrig-netdev.o
obj-$(CONFIG_LEDS_TRIGGER_PATTERN) += ledtrig-pattern.o
obj-$(CONFIG_LEDS_TRIGGER_AUDIO) += ledtrig-audio.o
obj-$(CONFIG_LEDS_TRIGGER_TTY) += ledtrig-tty.o
+obj-$(CONFIG_LEDS_TRIGGER_BLKDEV) += ledtrig-blkdev.o
diff --git a/drivers/leds/trigger/ledtrig-blkdev.c b/drivers/leds/trigger/ledtrig-blkdev.c
new file mode 100644
index 000000000000..067eedb003b5
--- /dev/null
+++ b/drivers/leds/trigger/ledtrig-blkdev.c
@@ -0,0 +1,1221 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+/*
+ * Block device LED trigger
+ *
+ * Copyright 2021-2022 Ian Pilcher <[email protected]>
+ */
+
+#include <linux/blkdev.h>
+#include <linux/leds.h>
+#include <linux/module.h>
+#include <linux/part_stat.h>
+#include <linux/xarray.h>
+
+/**
+ * DOC: Overview
+ *
+ * The ``blkdev`` LED trigger works by periodically checking the activity
+ * counters of block devices that have been linked to one or more LEDs and
+ * blinking those LED(s) if the correct type of activity has occurred. The
+ * periodic check is scheduled with the Linux kernel's deferred work facility.
+ *
+ * Trigger-specific data about block devices and LEDs is stored in two data
+ * structures --- &struct blkdev_trig_bdev (a "BTB") and &struct blkdev_trig_led
+ * (a "BTL"). Each structure contains a &struct xarray that holds links to any
+ * linked devices of the other type. I.e. &blkdev_trig_bdev.linked_btls
+ * contains links to all BTLs whose LEDs have been linked to the BTB's block
+ * device, and &blkdev_trig_led.linked_btbs contains links to all BTBs whose
+ * block devices have been linked to the BTL's LED. Thus, a block device can
+ * be linked to more than one LED, and an LED can be linked to more than one
+ * block device.
+ */
+
+/* Default, minimum & maximum blink duration (milliseconds) */
+#define BLKDEV_TRIG_BLINK_DEF 75
+#define BLKDEV_TRIG_BLINK_MIN 10
+#define BLKDEV_TRIG_BLINK_MAX 86400000 /* 24 hours */
+
+/* Default, minimum & maximum activity check interval (milliseconds) */
+#define BLKDEV_TRIG_CHECK_DEF 100
+#define BLKDEV_TRIG_CHECK_MIN 25
+#define BLKDEV_TRIG_CHECK_MAX 86400000 /* 24 hours */
+
+/*
+ * If blkdev_trig_check() can't lock the mutex, how long to wait before trying
+ * again (milliseconds)
+ */
+#define BLKDEV_TRIG_CHECK_RETRY 5
+
+/* Mode argument for calls to blkdev_get_by_path() and blkdev_put() */
+#define BLKDEV_TRIG_FMODE 0
+
+/**
+ * struct blkdev_trig_bdev - Trigger-specific data about a block device.
+ * @last_checked: Time (in jiffies) at which the trigger last checked this
+ * block device for activity.
+ * @last_activity: Time (in jiffies) at which the trigger last detected
+ * activity of each type.
+ * @ios: Activity counter values for each type, corresponding to
+ * the timestamps in &last_activity.
+ * @index: &xarray index, so the BTB can be included in one or more
+ * &blkdev_trig_led.linked_btbs.
+ * @bdev: The block device.
+ * @linked_btls: The BTLs that represent the LEDs linked to the BTB's
+ * block device.
+ *
+ * Every block device linked to at least one LED gets a "BTB." A BTB is created
+ * when a block device that is not currently linked to any LEDs is linked to an
+ * LED.
+ *
+ * A BTB is freed when one of the following occurs:
+ *
+ * * The number of LEDs linked to the block device becomes zero, because it has
+ * been unlinked from its last LED using the trigger's &sysfs interface.
+ *
+ * * The number of LEDs linked to the block device becomes zero, because the
+ * last LED to which it was linked has been disassociated from the trigger
+ * (which happens automatically if the LED device is removed from the system).
+ *
+ * * The BTB's block device is removed from the system. To accomodate this
+ * scenario, BTB's are created as device resources, so that the release
+ * function will be called by the driver core when the device is removed.
+ */
+struct blkdev_trig_bdev {
+ unsigned long last_checked;
+ unsigned long last_activity[NR_STAT_GROUPS];
+ unsigned long ios[NR_STAT_GROUPS];
+ unsigned long index;
+ struct block_device *bdev;
+ struct xarray linked_btls;
+};
+
+/**
+ * struct blkdev_trig_led - Trigger-specific data about an LED.
+ * @last_checked: Time (in jiffies) at which the trigger last checked the
+ * the block devices linked to this LED for activity.
+ * @index: &xarray index, so the BTL can be included in one or more
+ * &blkdev_trig_bdev.linked_btls.
+ * @mode: Bitmask for types of block device activity that will
+ * cause this LED to blink --- reads, writes, discards,
+ * etc.
+ * @led: The LED device.
+ * @blink_msec: Duration of a blink (milliseconds).
+ * @check_jiffies: Frequency with which block devices linked to this LED
+ * should be checked for activity (jiffies).
+ * @linked_btbs: The BTBs that represent the block devices linked to the
+ * BTL's LED.
+ * @all_btls_node: The BTL's node in the module's list of all BTLs.
+ *
+ * Every LED associated with the block device trigger gets a "BTL." A BTL is
+ * created when the trigger is "activated" on an LED (usually by writing
+ * ``blkdev`` to the LED's &sysfs &trigger attribute). A BTL is freed wnen its
+ * LED is disassociated from the trigger, either through the trigger's &sysfs
+ * interface or because the LED device is removed from the system.
+ */
+struct blkdev_trig_led {
+ unsigned long last_checked;
+ unsigned long index;
+ unsigned long mode; /* must be ulong for atomic bit ops */
+ struct led_classdev *led;
+ unsigned int blink_msec;
+ unsigned int check_jiffies;
+ struct xarray linked_btbs;
+ struct hlist_node all_btls_node;
+};
+
+/* Protects everything except atomic LED attributes */
+static DEFINE_MUTEX(blkdev_trig_mutex);
+
+/* BTB device resource release function */
+static void blkdev_trig_btb_release(struct device *dev, void *res);
+
+/* Index for next BTB or BTL */
+static unsigned long blkdev_trig_next_index;
+
+/* All LEDs associated with the trigger */
+static HLIST_HEAD(blkdev_trig_all_btls);
+
+/* Delayed work to periodically check for activity & blink LEDs */
+static void blkdev_trig_check(struct work_struct *work);
+static DECLARE_DELAYED_WORK(blkdev_trig_work, blkdev_trig_check);
+
+/* When is the delayed work scheduled to run next (jiffies) */
+static unsigned long blkdev_trig_next_check;
+
+/* Total number of BTB-to-BTL links */
+static unsigned int blkdev_trig_link_count;
+
+/* Empty sysfs attribute list for next 2 declarations */
+static struct attribute *blkdev_trig_attrs_empty[] = { NULL };
+
+/* linked_leds sysfs directory for block devs linked to 1 or more LEDs */
+static const struct attribute_group blkdev_trig_linked_leds = {
+ .name = "linked_leds",
+ .attrs = blkdev_trig_attrs_empty,
+};
+
+/* linked_devices sysfs directory for each LED associated with the trigger */
+static const struct attribute_group blkdev_trig_linked_devs = {
+ .name = "linked_devices",
+ .attrs = blkdev_trig_attrs_empty,
+};
+
+
+/*
+ *
+ * Delayed work to check for activity & blink LEDs
+ *
+ */
+
+/**
+ * blkdev_trig_blink() - Blink an LED, if the correct type of activity has
+ * occurred on the block device.
+ * @btl: The BTL that represents the LED
+ * @btb: The BTB that represents the block device
+ *
+ * Context: Process context. Caller must hold &blkdev_trig_mutex.
+ * Return: &true if the LED is blinked, &false if not.
+ */
+static bool blkdev_trig_blink(const struct blkdev_trig_led *btl,
+ const struct blkdev_trig_bdev *btb)
+{
+ unsigned long mode, mask, delay_on, delay_off;
+ enum stat_group i;
+
+ mode = READ_ONCE(btl->mode);
+
+ for (i = STAT_READ, mask = 1; i <= STAT_FLUSH; ++i, mask <<= 1) {
+
+ if (!(mode & mask))
+ continue;
+
+ if (time_before_eq(btb->last_activity[i], btl->last_checked))
+ continue;
+
+ delay_on = READ_ONCE(btl->blink_msec);
+ delay_off = 1; /* 0 leaves LED turned on */
+
+ led_blink_set_oneshot(btl->led, &delay_on, &delay_off, 0);
+ return true;
+ }
+
+ return false;
+}
+
+/**
+ * blkdev_trig_update_btb() - Update a BTB's activity counters and timestamps.
+ * @btb: The BTB
+ * @now: Timestamp (in jiffies)
+ *
+ * Context: Process context. Caller must hold &blkdev_trig_mutex.
+ */
+static void blkdev_trig_update_btb(struct blkdev_trig_bdev *btb,
+ unsigned long now)
+{
+ unsigned long new_ios;
+ enum stat_group i;
+
+ for (i = STAT_READ; i <= STAT_FLUSH; ++i) {
+
+ new_ios = part_stat_read(btb->bdev, ios[i]);
+
+ if (new_ios != btb->ios[i]) {
+ btb->ios[i] = new_ios;
+ btb->last_activity[i] = now;
+ }
+ }
+
+ btb->last_checked = now;
+}
+
+/**
+ * blkdev_trig_check() - Check linked devices for activity and blink LEDs.
+ * @work: Delayed work (&blkdev_trig_work)
+ *
+ * Context: Process context. Takes and releases &blkdev_trig_mutex.
+ */
+static void blkdev_trig_check(struct work_struct *work)
+{
+ struct blkdev_trig_led *btl;
+ struct blkdev_trig_bdev *btb;
+ unsigned long index, delay, now, led_check, led_delay;
+ bool blinked;
+
+ if (!mutex_trylock(&blkdev_trig_mutex)) {
+ delay = msecs_to_jiffies(BLKDEV_TRIG_CHECK_RETRY);
+ goto exit_reschedule;
+ }
+
+ now = jiffies;
+ delay = ULONG_MAX;
+
+ hlist_for_each_entry (btl, &blkdev_trig_all_btls, all_btls_node) {
+
+ led_check = btl->last_checked + btl->check_jiffies;
+
+ if (time_before_eq(led_check, now)) {
+
+ blinked = false;
+
+ xa_for_each (&btl->linked_btbs, index, btb) {
+
+ if (btb->last_checked != now)
+ blkdev_trig_update_btb(btb, now);
+ if (!blinked)
+ blinked = blkdev_trig_blink(btl, btb);
+ }
+
+ btl->last_checked = now;
+ led_delay = btl->check_jiffies;
+
+ } else {
+ led_delay = led_check - now;
+ }
+
+ if (led_delay < delay)
+ delay = led_delay;
+ }
+
+ mutex_unlock(&blkdev_trig_mutex);
+
+exit_reschedule:
+ WARN_ON_ONCE(delay == ULONG_MAX);
+ WARN_ON_ONCE(!schedule_delayed_work(&blkdev_trig_work, delay));
+}
+
+/**
+ * blkdev_trig_sched_led() - Set the schedule of the delayed work when a new
+ * LED is added to the schedule.
+ * @btl: The BTL that represents the LED
+ *
+ * Called when the number of block devices to which an LED is linked becomes
+ * non-zero.
+ *
+ * Context: Process context. Caller must hold &blkdev_trig_mutex.
+ */
+static void blkdev_trig_sched_led(const struct blkdev_trig_led *btl)
+{
+ unsigned long delay = READ_ONCE(btl->check_jiffies);
+ unsigned long check_by = jiffies + delay;
+
+ /*
+ * If no other LED-to-block device links exist, simply schedule the
+ * delayed work according to this LED's check_interval attribute
+ * (check_jiffies).
+ */
+ if (blkdev_trig_link_count == 0) {
+ WARN_ON(!schedule_delayed_work(&blkdev_trig_work, delay));
+ blkdev_trig_next_check = check_by;
+ return;
+ }
+
+ /*
+ * If the next check is already scheduled to occur soon enough to
+ * accomodate this LED's check_interval, the schedule doesn't need
+ * to be changed.
+ */
+ if (time_after_eq(check_by, blkdev_trig_next_check))
+ return;
+
+ /*
+ * Modify the schedule, so that the delayed work runs soon enough for
+ * this LED.
+ */
+ WARN_ON(!mod_delayed_work(system_wq, &blkdev_trig_work, delay));
+ blkdev_trig_next_check = check_by;
+}
+
+
+/*
+ *
+ * Linking and unlinking LEDs and block devices
+ *
+ */
+
+/**
+ * blkdev_trig_link() - Link a block device to an LED.
+ * @btl: The BTL that represents the LED
+ * @btb: The BTB that represents the block device
+ *
+ * Context: Process context. Caller must hold &blkdev_trig_mutex.
+ * Return: &0 on success, negative &errno on error.
+ */
+static int blkdev_trig_link(struct blkdev_trig_led *btl,
+ struct blkdev_trig_bdev *btb)
+{
+ bool led_first_link;
+ int err;
+
+ led_first_link = xa_empty(&btl->linked_btbs);
+
+ err = xa_insert(&btb->linked_btls, btl->index, btl, GFP_KERNEL);
+ if (err)
+ return err;
+
+ err = xa_insert(&btl->linked_btbs, btb->index, btb, GFP_KERNEL);
+ if (err)
+ goto error_erase_btl;
+
+ /* Create /sys/class/block/<bdev>/linked_leds/<led> symlink */
+ err = sysfs_add_link_to_group(bdev_kobj(btb->bdev),
+ blkdev_trig_linked_leds.name,
+ &btl->led->dev->kobj, btl->led->name);
+ if (err)
+ goto error_erase_btb;
+
+ /* Create /sys/class/leds/<led>/linked_devices/<bdev> symlink */
+ err = sysfs_add_link_to_group(&btl->led->dev->kobj,
+ blkdev_trig_linked_devs.name,
+ bdev_kobj(btb->bdev),
+ dev_name(&btb->bdev->bd_device));
+ if (err)
+ goto error_remove_symlink;
+
+ /*
+ * If this is the first block device linked to this LED, the delayed
+ * work schedule may need to be changed.
+ */
+ if (led_first_link)
+ blkdev_trig_sched_led(btl);
+
+ ++blkdev_trig_link_count;
+
+ return 0;
+
+error_remove_symlink:
+ sysfs_remove_link_from_group(bdev_kobj(btb->bdev),
+ blkdev_trig_linked_leds.name,
+ btl->led->name);
+error_erase_btb:
+ xa_erase(&btl->linked_btbs, btb->index);
+error_erase_btl:
+ xa_erase(&btb->linked_btls, btl->index);
+ return err;
+}
+
+/**
+ * blkdev_trig_put_btb() - Remove and free a BTB, if it is no longer needed.
+ * @btb: The BTB
+ *
+ * Does nothing if the BTB (block device) is still linked to at least one LED.
+ *
+ * Context: Process context. Caller must hold &blkdev_trig_mutex.
+ */
+static void blkdev_trig_put_btb(struct blkdev_trig_bdev *btb)
+{
+ struct block_device *bdev = btb->bdev;
+ int err;
+
+ if (xa_empty(&btb->linked_btls)) {
+
+ sysfs_remove_group(bdev_kobj(bdev), &blkdev_trig_linked_leds);
+ err = devres_destroy(&bdev->bd_device, blkdev_trig_btb_release,
+ NULL, NULL);
+ WARN_ON(err);
+ }
+}
+
+/**
+ * _blkdev_trig_unlink_always() - Perform the unconditionally required steps of
+ * unlinking a block device from an LED.
+ * @btl: The BTL that represents the LED
+ * @btb: The BTB that represents the block device
+ *
+ * When a block device is unlinked from an LED, certain steps must be performed
+ * only if the block device is **not** being released. This function performs
+ * those steps that are **always** required, whether or not the block device is
+ * being released.
+ *
+ * Context: Process context. Caller must hold &blkdev_trig_mutex.
+ */
+static void _blkdev_trig_unlink_always(struct blkdev_trig_led *btl,
+ struct blkdev_trig_bdev *btb)
+{
+ --blkdev_trig_link_count;
+
+ if (blkdev_trig_link_count == 0)
+ WARN_ON(!cancel_delayed_work_sync(&blkdev_trig_work));
+
+ xa_erase(&btb->linked_btls, btl->index);
+ xa_erase(&btl->linked_btbs, btb->index);
+
+ /* Remove /sys/class/leds/<led>/linked_devices/<bdev> symlink */
+ sysfs_remove_link_from_group(&btl->led->dev->kobj,
+ blkdev_trig_linked_devs.name,
+ dev_name(&btb->bdev->bd_device));
+}
+
+/**
+ * blkdev_trig_unlink_norelease() - Unlink an LED from a block device that is
+ * **not** being released.
+ * @btl: The BTL that represents the LED.
+ * @btb: The BTB that represents the block device.
+ *
+ * Context: Process context. Caller must hold &blkdev_trig_mutex.
+ */
+static void blkdev_trig_unlink_norelease(struct blkdev_trig_led *btl,
+ struct blkdev_trig_bdev *btb)
+{
+ _blkdev_trig_unlink_always(btl, btb);
+
+ /* Remove /sys/class/block/<bdev>/linked_leds/<led> symlink */
+ sysfs_remove_link_from_group(bdev_kobj(btb->bdev),
+ blkdev_trig_linked_leds.name,
+ btl->led->name);
+
+ blkdev_trig_put_btb(btb);
+}
+
+/**
+ * blkdev_trig_unlink_release() - Unlink an LED from a block device that is
+ * being released.
+ * @btl: The BTL that represents the LED
+ * @btb: The BTB that represents the block device
+ *
+ * Context: Process context. Caller must hold &blkdev_trig_mutex.
+ */
+static void blkdev_trig_unlink_release(struct blkdev_trig_led *btl,
+ struct blkdev_trig_bdev *btb)
+{
+ _blkdev_trig_unlink_always(btl, btb);
+
+ /*
+ * If the BTB is being released, the driver core has already removed the
+ * device's attribute groups, and the BTB will be freed automatically,
+ * so there's nothing else to do.
+ */
+}
+
+
+/*
+ *
+ * BTB creation
+ *
+ */
+
+/**
+ * blkdev_trig_btb_release() - BTB device resource release function.
+ * @dev: The block device
+ * @res: The BTB
+ *
+ * Called by the driver core when a block device with a BTB is removed.
+ *
+ * Context: Process context. Takes and releases &blkdev_trig_mutex.
+ */
+static void blkdev_trig_btb_release(struct device *dev, void *res)
+{
+ struct blkdev_trig_bdev *btb = res;
+ struct blkdev_trig_led *btl;
+ unsigned long index;
+
+ mutex_lock(&blkdev_trig_mutex);
+
+ xa_for_each (&btb->linked_btls, index, btl)
+ blkdev_trig_unlink_release(btl, btb);
+
+ mutex_unlock(&blkdev_trig_mutex);
+}
+
+/**
+ * blkdev_trig_get_bdev() - Get a block device by path.
+ * @path: The value written to an LED's &link_dev_by_path or
+ * &unlink_dev_by_path attribute, which should be the path to a
+ * special file that represents a block device
+ * @len: The number of characters in &path (not including its
+ * terminating null)
+ *
+ * The caller must call blkdev_put() when finished with the device.
+ *
+ * Context: Process context.
+ * Return: The block device, or an error pointer.
+ */
+static struct block_device *blkdev_trig_get_bdev(const char *path, size_t len)
+{
+ struct block_device *bdev;
+ char *buf;
+
+ buf = kmemdup(path, len + 1, GFP_KERNEL); /* +1 to include null */
+ if (buf == NULL)
+ return ERR_PTR(-ENOMEM);
+
+ bdev = blkdev_get_by_path(strim(buf), BLKDEV_TRIG_FMODE, THIS_MODULE);
+ kfree(buf);
+ return bdev;
+}
+
+/**
+ * blkdev_trig_get_btb() - Find or create the BTB for a block device.
+ * @path: The value written to an LED's &link_dev_by_path attribute,
+ * which should be the path to a special file that represents a
+ * block device
+ * @len: The number of characters in &path
+ *
+ * If a new BTB is created, because the block device was not previously linked
+ * to any LEDs, the block device's &linked_leds &sysfs directory is created.
+ *
+ * Context: Process context. Caller must hold &blkdev_trig_mutex.
+ * Return: Pointer to the BTB, error pointer on error.
+ */
+static struct blkdev_trig_bdev *blkdev_trig_get_btb(const char *path,
+ size_t len)
+{
+ struct block_device *bdev;
+ struct blkdev_trig_bdev *btb;
+ int err;
+
+ bdev = blkdev_trig_get_bdev(path, len);
+ if (IS_ERR(bdev))
+ return ERR_CAST(bdev);
+
+ btb = devres_find(&bdev->bd_device, blkdev_trig_btb_release,
+ NULL, NULL);
+ if (btb != NULL) {
+ err = 0;
+ goto exit_put_bdev;
+ }
+
+ if (blkdev_trig_next_index == ULONG_MAX) {
+ err = -EOVERFLOW;
+ goto exit_put_bdev;
+ }
+
+ btb = devres_alloc(blkdev_trig_btb_release, sizeof(*btb), GFP_KERNEL);
+ if (btb == NULL) {
+ err = -ENOMEM;
+ goto exit_put_bdev;
+ }
+
+ err = sysfs_create_group(bdev_kobj(bdev), &blkdev_trig_linked_leds);
+ if (err)
+ goto exit_free_btb;
+
+ btb->index = blkdev_trig_next_index++;
+ btb->bdev = bdev;
+ xa_init(&btb->linked_btls);
+
+ /* Populate BTB activity counters */
+ blkdev_trig_update_btb(btb, jiffies);
+
+ devres_add(&bdev->bd_device, btb);
+
+exit_free_btb:
+ if (err)
+ devres_free(btb);
+exit_put_bdev:
+ blkdev_put(bdev, BLKDEV_TRIG_FMODE);
+ return err ? ERR_PTR(err) : btb;
+}
+
+
+/*
+ *
+ * Activating and deactivating the trigger on an LED
+ *
+ */
+
+/**
+ * blkdev_trig_activate() - Called by the LEDs subsystem when an LED is
+ * associated with the trigger.
+ * @led: The LED
+ *
+ * Context: Process context. Takes and releases &blkdev_trig_mutex.
+ * Return: &0 on success, negative &errno on error.
+ */
+static int blkdev_trig_activate(struct led_classdev *led)
+{
+ struct blkdev_trig_led *btl;
+ int err;
+
+ btl = kzalloc(sizeof(*btl), GFP_KERNEL);
+ if (btl == NULL)
+ return -ENOMEM;
+
+ err = mutex_lock_interruptible(&blkdev_trig_mutex);
+ if (err)
+ goto exit_free;
+
+ if (blkdev_trig_next_index == ULONG_MAX) {
+ err = -EOVERFLOW;
+ goto exit_unlock;
+ }
+
+ btl->index = blkdev_trig_next_index++;
+ btl->last_checked = jiffies;
+ btl->mode = -1; /* set all bits */
+ btl->led = led;
+ btl->blink_msec = BLKDEV_TRIG_BLINK_DEF;
+ btl->check_jiffies = msecs_to_jiffies(BLKDEV_TRIG_CHECK_DEF);
+ xa_init(&btl->linked_btbs);
+
+ hlist_add_head(&btl->all_btls_node, &blkdev_trig_all_btls);
+ led_set_trigger_data(led, btl);
+
+exit_unlock:
+ mutex_unlock(&blkdev_trig_mutex);
+exit_free:
+ if (err)
+ kfree(btl);
+ return err;
+}
+
+/**
+ * blkdev_trig_deactivate() - Called by the the LEDs subsystem when an LED is
+ * disassociated from the trigger.
+ * @led: The LED
+ *
+ * The LEDs subsystem also calls this function when an LED associated with the
+ * trigger is removed or when the trigger is unregistered (if the module is
+ * unloaded).
+ *
+ * Context: Process context. Takes and releases &blkdev_trig_mutex.
+ */
+static void blkdev_trig_deactivate(struct led_classdev *led)
+{
+ struct blkdev_trig_led *btl = led_get_trigger_data(led);
+ struct blkdev_trig_bdev *btb;
+ unsigned long index;
+
+ mutex_lock(&blkdev_trig_mutex);
+
+ xa_for_each (&btl->linked_btbs, index, btb)
+ blkdev_trig_unlink_norelease(btl, btb);
+
+ hlist_del(&btl->all_btls_node);
+ kfree(btl);
+
+ mutex_unlock(&blkdev_trig_mutex);
+}
+
+
+/*
+ *
+ * Link-related attribute store functions
+ *
+ */
+
+/**
+ * link_dev_by_path_store() - &link_dev_by_path device attribute store function.
+ * @dev: The LED device
+ * @attr: The &link_dev_by_path attribute (&dev_attr_link_dev_by_path)
+ * @buf: The value written to the attribute, which should be the path to
+ * a special file that represents a block device to be linked to
+ * the LED (e.g. ``/dev/sda``)
+ * @count: The number of characters in &buf
+ *
+ * Context: Process context. Takes and releases &blkdev_trig_mutex.
+ * Return: &count on success, negative &errno on error.
+ */
+static ssize_t link_dev_by_path_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct blkdev_trig_led *btl = led_trigger_get_drvdata(dev);
+ struct blkdev_trig_bdev *btb;
+ int err;
+
+ err = mutex_lock_interruptible(&blkdev_trig_mutex);
+ if (err)
+ return err;
+
+ btb = blkdev_trig_get_btb(buf, count);
+ if (IS_ERR(btb)) {
+ err = PTR_ERR(btb);
+ goto exit_unlock;
+ }
+
+ if (xa_load(&btb->linked_btls, btl->index) != NULL) {
+ err = -EEXIST;
+ goto exit_put_btb;
+ }
+
+ err = blkdev_trig_link(btl, btb);
+
+exit_put_btb:
+ if (err)
+ blkdev_trig_put_btb(btb);
+exit_unlock:
+ mutex_unlock(&blkdev_trig_mutex);
+ return err ? : count;
+}
+
+/**
+ * unlink_dev_by_path_store() - &unlink_dev_by_path device attribute store
+ * function.
+ * @dev: The LED device
+ * @attr: The &unlink_dev_by_path attribute (&dev_attr_unlink_dev_by_path)
+ * @buf: The value written to the attribute, which should be the path to
+ * a special file that represents a block device to be unlinked
+ * from the LED (e.g. ``/dev/sda``)
+ * @count: The number of characters in &buf
+ *
+ * Context: Process context. Takes and releases &blkdev_trig_mutex.
+ * Return: &count on success, negative &errno on error.
+ */
+static ssize_t unlink_dev_by_path_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct blkdev_trig_led *btl = led_trigger_get_drvdata(dev);
+ struct block_device *bdev;
+ struct blkdev_trig_bdev *btb;
+ int err;
+
+ bdev = blkdev_trig_get_bdev(buf, count);
+ if (IS_ERR(bdev))
+ return PTR_ERR(bdev);
+
+ err = mutex_lock_interruptible(&blkdev_trig_mutex);
+ if (err)
+ goto exit_put_bdev;
+
+ btb = devres_find(&bdev->bd_device, blkdev_trig_btb_release,
+ NULL, NULL);
+ if (btb == NULL) {
+ err = -EUNATCH; /* bdev isn't linked to any LED */
+ goto exit_unlock;
+ }
+
+ if (xa_load(&btb->linked_btls, btl->index) == NULL) {
+ err = -EUNATCH; /* bdev isn't linked to this LED */
+ goto exit_unlock;
+ }
+
+ blkdev_trig_unlink_norelease(btl, btb);
+
+exit_unlock:
+ mutex_unlock(&blkdev_trig_mutex);
+exit_put_bdev:
+ blkdev_put(bdev, BLKDEV_TRIG_FMODE);
+ return err ? : count;
+}
+
+/**
+ * unlink_dev_by_name_store() - &unlink_dev_by_name device attribute store
+ * function.
+ * @dev: The LED device
+ * @attr: The &unlink_dev_by_name attribute (&dev_attr_unlink_dev_by_name)
+ * @buf: The value written to the attribute, which should be the kernel
+ * name of a block device to be unlinked from the LED (e.g.
+ * ``sda``)
+ * @count: The number of characters in &buf
+ *
+ * Context: Process context. Takes and releases &blkdev_trig_mutex.
+ * Return: &count on success, negative &errno on error.
+ */
+static ssize_t unlink_dev_by_name_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct blkdev_trig_led *btl = led_trigger_get_drvdata(dev);
+ struct blkdev_trig_bdev *btb;
+ unsigned long index;
+ int err;
+
+ err = mutex_lock_interruptible(&blkdev_trig_mutex);
+ if (err)
+ return err;
+
+ err = -EUNATCH;
+
+ xa_for_each (&btl->linked_btbs, index, btb) {
+
+ if (sysfs_streq(dev_name(&btb->bdev->bd_device), buf)) {
+ blkdev_trig_unlink_norelease(btl, btb);
+ err = 0;
+ break;
+ }
+ }
+
+ mutex_unlock(&blkdev_trig_mutex);
+ return err ? : count;
+}
+
+
+/*
+ *
+ * Atomic attribute show & store functions
+ *
+ */
+
+/**
+ * blink_time_show() - &blink_time device attribute show function.
+ * @dev: The LED device
+ * @attr: The &blink_time attribute (&dev_attr_blink_time)
+ * @buf: Output buffer
+ *
+ * Writes the value of &blkdev_trig_led.blink_msec to &buf.
+ *
+ * Context: Process context.
+ * Return: The number of characters written to &buf.
+ */
+static ssize_t blink_time_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ const struct blkdev_trig_led *btl = led_trigger_get_drvdata(dev);
+
+ return sysfs_emit(buf, "%u\n", READ_ONCE(btl->blink_msec));
+}
+
+/**
+ * blink_time_store() - &blink_time device attribute store function.
+ * @dev: The LED device
+ * @attr: The &blink_time attribute (&dev_attr_blink_time)
+ * @buf: The new value (as written to the &sysfs attribute)
+ * @count: The number of characters in &buf
+ *
+ * Sets &blkdev_trig_led.blink_msec to the value in &buf.
+ *
+ * Context: Process context.
+ * Return: &count on success, negative &errno on error.
+ */
+static ssize_t blink_time_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct blkdev_trig_led *btl = led_trigger_get_drvdata(dev);
+ unsigned int value;
+ int err;
+
+ err = kstrtouint(buf, 0, &value);
+ if (err)
+ return err;
+
+ if (value < BLKDEV_TRIG_BLINK_MIN || value > BLKDEV_TRIG_BLINK_MAX)
+ return -ERANGE;
+
+ WRITE_ONCE(btl->blink_msec, value);
+ return count;
+}
+
+/**
+ * check_interval_show() - &check_interval device attribute show function.
+ * @dev: The LED device
+ * @attr: The &check_interval attribute (&dev_attr_check_interval)
+ * @buf: Output buffer
+ *
+ * Writes the value of &blkdev_trig_led.check_jiffies (converted to
+ * milliseconds) to &buf.
+ *
+ * Context: Process context.
+ * Return: The number of characters written to &buf.
+ */
+static ssize_t check_interval_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct blkdev_trig_led *btl = led_trigger_get_drvdata(dev);
+
+ return sysfs_emit(buf, "%u\n",
+ jiffies_to_msecs(READ_ONCE(btl->check_jiffies)));
+}
+
+/**
+ * check_interval_store() - &check_interval device attribute store function
+ * @dev: The LED device
+ * @attr: The &check_interval attribute (&dev_attr_check_interval)
+ * @buf: The new value (as written to the &sysfs attribute)
+ * @count: The number of characters in &buf
+ *
+ * Sets &blkdev_trig_led.check_jiffies to the value in &buf (after converting
+ * from milliseconds).
+ *
+ * Context: Process context.
+ * Return: &count on success, negative &errno on error.
+ */
+static ssize_t check_interval_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct blkdev_trig_led *led = led_trigger_get_drvdata(dev);
+ unsigned int value;
+ int err;
+
+ err = kstrtouint(buf, 0, &value);
+ if (err)
+ return err;
+
+ if (value < BLKDEV_TRIG_CHECK_MIN || value > BLKDEV_TRIG_CHECK_MAX)
+ return -ERANGE;
+
+ WRITE_ONCE(led->check_jiffies, msecs_to_jiffies(value));
+
+ return count;
+}
+
+/**
+ * blkdev_trig_mode_show() - Helper for boolean attribute show functions.
+ * @led: The LED
+ * @buf: Output buffer
+ * @bit: Which bit to show
+ *
+ * Context: Process context.
+ * Return: The number of characters written to &buf.
+ */
+static int blkdev_trig_mode_show(const struct blkdev_trig_led *led, char *buf,
+ enum stat_group bit)
+{
+ return sysfs_emit(buf,
+ READ_ONCE(led->mode) & (1 << bit) ? "Y\n" : "N\n");
+}
+
+/**
+ * blkdev_trig_mode_store() - Helper for boolean attribute store functions.
+ * @led: The LED
+ * @buf: The new value (as written to the &sysfs attribute)
+ * @count: The number of characters in &buf
+ * @bit: Which bit to set
+ *
+ * Context: Process context.
+ * Return: &count on success, negative &errno on error.
+ */
+static int blkdev_trig_mode_store(struct blkdev_trig_led *led,
+ const char *buf, size_t count,
+ enum stat_group bit)
+{
+ bool set;
+ int err;
+
+ err = kstrtobool(buf, &set);
+ if (err)
+ return err;
+
+ if (set)
+ set_bit(bit, &led->mode);
+ else
+ clear_bit(bit, &led->mode);
+
+ return count;
+}
+
+/**
+ * blink_on_read_show() - &blink_on_read device attribute show function.
+ * @dev: The LED device
+ * @attr: The &blink_on_read attribute (&dev_attr_blink_on_read)
+ * @buf: Output buffer
+ *
+ * Writes ``Y`` or ``N`` to &buf, depending on whether the &STAT_READ bit in
+ * &blkdev_trig_led.mode is set or cleared.
+ *
+ * Context: Process context.
+ * Return: The number of characters written to &buf.
+ */
+static ssize_t blink_on_read_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ return blkdev_trig_mode_show(led_trigger_get_drvdata(dev),
+ buf, STAT_READ);
+}
+
+/**
+ * blink_on_read_store() - &blink_on_read device attribute store function.
+ * @dev: The LED device
+ * @attr: The &blink_on_read attribute (&dev_attr_blink_on_read)
+ * @buf: The new value (as written to the &sysfs attribute)
+ * @count: The number of characters in &buf
+ *
+ * Sets the &STAT_READ bit in &blkdev_trig_led.mode to the value in &buf
+ * (interpretted as a boolean).
+ *
+ * Context: Process context.
+ * Return: &count on success, negative &errno on error.
+ */
+static ssize_t blink_on_read_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ return blkdev_trig_mode_store(led_trigger_get_drvdata(dev),
+ buf, count, STAT_READ);
+}
+
+/**
+ * blink_on_write_show() - &blink_on_write device attribute show function.
+ * @dev: The LED device
+ * @attr: The &blink_on_write attribute (&dev_attr_blink_on_write)
+ * @buf: Output buffer
+ *
+ * Writes ``Y`` or ``N`` to &buf, depending on whether the &STAT_WRITE bit in
+ * in &blkdev_trig_led.mode is set or cleared.
+ *
+ * Context: Process context.
+ * Return: The number of characters written to &buf.
+ */
+static ssize_t blink_on_write_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ return blkdev_trig_mode_show(led_trigger_get_drvdata(dev),
+ buf, STAT_WRITE);
+}
+
+/**
+ * blink_on_write_store() - &blink_on_write device attribute store function.
+ * @dev: The LED device
+ * @attr: The &blink_on_write attribute (&dev_attr_blink_on_write)
+ * @buf: The new value (as written to the &sysfs attribute)
+ * @count: The number of characters in &buf
+ *
+ * Sets the &STAT_WRITE bit in &blkdev_trig_led.mode to the value in &buf
+ * (interpretted as a boolean).
+ *
+ * Context: Process context.
+ * Return: &count on success, negative &errno on error.
+ */
+static ssize_t blink_on_write_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ return blkdev_trig_mode_store(led_trigger_get_drvdata(dev),
+ buf, count, STAT_WRITE);
+}
+
+/**
+ * blink_on_flush_show() - &blink_on_flush device attribute show function.
+ * @dev: The LED device
+ * @attr: The &blink_on_flush attribute (&dev_attr_blink_on_flush)
+ * @buf: Output buffer
+ *
+ * Writes ``Y`` or ``N`` to &buf, depending whether the &STAT_FLUSH bit in
+ * &blkdev_trig_led.mode is set or cleared.
+ *
+ * Context: Process context.
+ * Return: The number of characters written to &buf.
+ */
+static ssize_t blink_on_flush_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ return blkdev_trig_mode_show(led_trigger_get_drvdata(dev),
+ buf, STAT_FLUSH);
+}
+
+/**
+ * blink_on_flush_store() - &blink_on_flush device attribute store function.
+ * @dev: The LED device
+ * @attr: The &blink_on_flush attribute (&dev_attr_blink_on_flush)
+ * @buf: The new value (as written to the &sysfs attribute)
+ * @count: The number of characters in &buf
+ *
+ * Sets the &STAT_FLUSH bit in &blkdev_trig_led.mode to the value in &buf
+ * (interpretted as a boolean).
+ *
+ * Context: Process context.
+ * Return: &count on success, negative &errno on error.
+ */
+static ssize_t blink_on_flush_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ return blkdev_trig_mode_store(led_trigger_get_drvdata(dev),
+ buf, count, STAT_FLUSH);
+}
+
+/**
+ * blink_on_discard_show() - &blink_on_discard device attribute show function.
+ * @dev: The LED device
+ * @attr: The &blink_on_discard attribute (&dev_attr_blink_on_discard)
+ * @buf: Output buffer
+ *
+ * Writes ``Y`` or ``N`` to &buf, depending on whether the &STAT_DISCARD bit in
+ * &blkdev_trig_led.mode is set or cleared.
+ *
+ * Context: Process context.
+ * Return: The number of characters written to &buf.
+ */
+static ssize_t blink_on_discard_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ return blkdev_trig_mode_show(led_trigger_get_drvdata(dev),
+ buf, STAT_DISCARD);
+}
+
+/**
+ * blink_on_discard_store() - &blink_on_discard device attribute store function.
+ * @dev: The LED device
+ * @attr: The &blink_on_discard attribute (&dev_attr_blink_on_discard)
+ * @buf: The new value (as written to the &sysfs attribute)
+ * @count: The number of characters in &buf
+ *
+ * Sets the &STAT_DISCARD bit in &blkdev_trig_led.mode to the value in &buf
+ * (interpretted as a boolean).
+ *
+ * Context: Process context.
+ * Return: &count on success, negative &errno on error.
+ */
+static ssize_t blink_on_discard_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ return blkdev_trig_mode_store(led_trigger_get_drvdata(dev),
+ buf, count, STAT_DISCARD);
+}
+
+/* Device attributes */
+static DEVICE_ATTR_WO(link_dev_by_path);
+static DEVICE_ATTR_WO(unlink_dev_by_path);
+static DEVICE_ATTR_WO(unlink_dev_by_name);
+static DEVICE_ATTR_RW(blink_time);
+static DEVICE_ATTR_RW(check_interval);
+static DEVICE_ATTR_RW(blink_on_read);
+static DEVICE_ATTR_RW(blink_on_write);
+static DEVICE_ATTR_RW(blink_on_flush);
+static DEVICE_ATTR_RW(blink_on_discard);
+
+/* Device attributes in LED directory (/sys/class/leds/<led>/...) */
+static struct attribute *blkdev_trig_attrs[] = {
+ &dev_attr_link_dev_by_path.attr,
+ &dev_attr_unlink_dev_by_path.attr,
+ &dev_attr_unlink_dev_by_name.attr,
+ &dev_attr_blink_time.attr,
+ &dev_attr_check_interval.attr,
+ &dev_attr_blink_on_read.attr,
+ &dev_attr_blink_on_write.attr,
+ &dev_attr_blink_on_flush.attr,
+ &dev_attr_blink_on_discard.attr,
+ NULL
+};
+
+/* Unnamed attribute group == no subdirectory */
+static const struct attribute_group blkdev_trig_attr_group = {
+ .attrs = blkdev_trig_attrs,
+};
+
+/* Attribute groups for the trigger */
+static const struct attribute_group *blkdev_trig_attr_groups[] = {
+ &blkdev_trig_attr_group, /* /sys/class/leds/<led>/... */
+ &blkdev_trig_linked_devs, /* /sys/class/leds/<led>/linked_devices/ */
+ NULL
+};
+
+/* Trigger registration data */
+static struct led_trigger blkdev_trig_trigger = {
+ .name = "blkdev",
+ .activate = blkdev_trig_activate,
+ .deactivate = blkdev_trig_deactivate,
+ .groups = blkdev_trig_attr_groups,
+};
+
+/**
+ * blkdev_trig_init() - Block device LED trigger initialization.
+ *
+ * Registers the ``blkdev`` LED trigger.
+ *
+ * Return: &0 on success, negative &errno on failure.
+ */
+static int __init blkdev_trig_init(void)
+{
+ return led_trigger_register(&blkdev_trig_trigger);
+}
+module_init(blkdev_trig_init);
+
+/**
+ * blkdev_trig_exit() - Block device LED trigger module exit.
+ *
+ * Unregisters the ``blkdev`` LED trigger.
+ */
+static void __exit blkdev_trig_exit(void)
+{
+ led_trigger_unregister(&blkdev_trig_trigger);
+}
+module_exit(blkdev_trig_exit);
+
+MODULE_DESCRIPTION("Block device LED trigger");
+MODULE_AUTHOR("Ian Pilcher <[email protected]>");
+MODULE_LICENSE("GPL v2");
--
2.38.1

2023-01-09 17:02:49

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH v13 0/2] Introduce block device LED trigger

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
>

--
Lee Jones [李琼斯]

2023-03-01 14:27:22

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH v13 0/2] Introduce block device LED trigger

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 [李琼斯]

2023-03-01 17:38:25

by Ian Pilcher

[permalink] [raw]
Subject: Re: [PATCH v13 0/2] Introduce block device LED trigger

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.

--
========================================================================
Google Where SkyNet meets Idiocracy
========================================================================


2023-03-02 16:04:39

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH v13 0/2] Introduce block device LED trigger

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.

--
Lee Jones [李琼斯]