| Message ID | 20210903204548.2745354-10-arequipeno@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Introduce block device LED trigger | expand |
On Fri, Sep 03, 2021 at 03:45:39PM -0500, Ian Pilcher wrote: > Use a delayed workqueue to periodically check configured block devices for > activity since the last check. Blink LEDs associated with devices on which > the configured type of activity (read/write) has occurred. > > Signed-off-by: Ian Pilcher <arequipeno@gmail.com> > --- > drivers/leds/trigger/ledtrig-blkdev.c | 88 +++++++++++++++++++++++++++ > 1 file changed, 88 insertions(+) > > diff --git a/drivers/leds/trigger/ledtrig-blkdev.c b/drivers/leds/trigger/ledtrig-blkdev.c > index 1f319529c3be..37ba9bb3542e 100644 > --- a/drivers/leds/trigger/ledtrig-blkdev.c > +++ b/drivers/leds/trigger/ledtrig-blkdev.c > @@ -7,7 +7,9 @@ > */ > > #include <linux/ctype.h> > +#include <linux/leds.h> > #include <linux/module.h> > +#include <linux/part_stat.h> > > #include "ledtrig-blkdev.h" > > @@ -68,6 +70,10 @@ static unsigned int ledtrig_blkdev_count; > /* How often to check for drive activity - in jiffies */ > static unsigned int ledtrig_blkdev_interval; > > +/* Delayed work used to periodically check for activity & blink LEDs */ > +static void blkdev_process(struct work_struct *const work); > +static DECLARE_DELAYED_WORK(ledtrig_blkdev_work, blkdev_process); > + > > /* > * > @@ -110,3 +116,85 @@ static bool blkdev_write_mode(const enum ledtrig_blkdev_mode mode) > { > return mode != LEDTRIG_BLKDEV_MODE_RO; > } > + > + > +/* > + * > + * Periodically check for device acitivity and blink LEDs > + * > + */ > + > +static void blkdev_blink(const struct ledtrig_blkdev_led *const led) > +{ > + unsigned long delay_on = READ_ONCE(led->blink_msec); > + unsigned long delay_off = 1; /* 0 leaves LED turned on */ > + > + led_blink_set_oneshot(led->led_dev, &delay_on, &delay_off, 0); > +} > + > +static void blkdev_update_disk(struct ledtrig_blkdev_disk *const disk, > + const unsigned int generation) > +{ > + const struct block_device *const part0 = disk->gd->part0; > + const unsigned long read_ios = part_stat_read(part0, ios[STAT_READ]); > + const unsigned long write_ios = part_stat_read(part0, ios[STAT_WRITE]) > + + part_stat_read(part0, ios[STAT_DISCARD]) > + + part_stat_read(part0, ios[STAT_FLUSH]); > + > + if (disk->read_ios != read_ios) { > + disk->read_act = true; > + disk->read_ios = read_ios; > + } else { > + disk->read_act = false; > + } > + > + if (disk->write_ios != write_ios) { > + disk->write_act = true; > + disk->write_ios = write_ios; > + } else { > + disk->write_act = false; > + } > + > + disk->generation = generation; > +} > + > +static void blkdev_process(struct work_struct *const work) > +{ > + static unsigned int generation; > + > + struct ledtrig_blkdev_led *led; > + struct ledtrig_blkdev_link *link; > + unsigned long delay; > + > + if (!mutex_trylock(&ledtrig_blkdev_mutex)) > + goto exit_reschedule; > + > + hlist_for_each_entry(led, &ledtrig_blkdev_leds, leds_node) { > + > + hlist_for_each_entry(link, &led->disks, led_disks_node) { > + > + struct ledtrig_blkdev_disk *const disk = link->disk; > + > + if (disk->generation != generation) > + blkdev_update_disk(disk, generation); > + > + if (disk->read_act && blkdev_read_mode(led->mode)) { > + blkdev_blink(led); > + break; > + } > + > + if (disk->write_act && blkdev_write_mode(led->mode)) { > + blkdev_blink(led); > + break; > + } > + } > + } > + > + ++generation; > + > + mutex_unlock(&ledtrig_blkdev_mutex); > + > +exit_reschedule: > + delay = READ_ONCE(ledtrig_blkdev_interval); > + WARN_ON_ONCE(!schedule_delayed_work(&ledtrig_blkdev_work, delay)); You just rebooted a machine if it hit this :( Please never use WARN_ON() in new code unless the machine is really broken and you can not do anything else here. thanks, greg k-h
On 9/4/21 1:01 AM, Greg KH wrote: > Please never use WARN_ON() in new code unless the machine is really > broken and you can not do anything else here. Wait what? I thought that was BUG_ON.
On Sun, Sep 05, 2021 at 09:39:57AM -0500, Ian Pilcher wrote: > On 9/4/21 1:01 AM, Greg KH wrote: > > Please never use WARN_ON() in new code unless the machine is really > > broken and you can not do anything else here. > > Wait what? I thought that was BUG_ON. Not whan panic-on-warn is set, which is getting more and more common these days. thanks, greg k-h
On 9/5/21 9:51 AM, Greg KH wrote: > On Sun, Sep 05, 2021 at 09:39:57AM -0500, Ian Pilcher wrote: >> On 9/4/21 1:01 AM, Greg KH wrote: >>> Please never use WARN_ON() in new code unless the machine is really >>> broken and you can not do anything else here. >> >> Wait what? I thought that was BUG_ON. > > Not whan panic-on-warn is set, which is getting more and more common > these days. Fair enough. What is the recommend approach to reporting a "this should never" happen situation these days? Thanks!
On Sun, Sep 05, 2021 at 09:56:58AM -0500, Ian Pilcher wrote: > On 9/5/21 9:51 AM, Greg KH wrote: > > On Sun, Sep 05, 2021 at 09:39:57AM -0500, Ian Pilcher wrote: > > > On 9/4/21 1:01 AM, Greg KH wrote: > > > > Please never use WARN_ON() in new code unless the machine is really > > > > broken and you can not do anything else here. > > > > > > Wait what? I thought that was BUG_ON. > > > > Not whan panic-on-warn is set, which is getting more and more common > > these days. > > Fair enough. What is the recommend approach to reporting a "this should > never" happen situation these days? dev_err() and handle the error properly.
On Sun, Sep 05, 2021 at 05:12:39PM +0200, Greg KH wrote: > On Sun, Sep 05, 2021 at 09:56:58AM -0500, Ian Pilcher wrote: > > On 9/5/21 9:51 AM, Greg KH wrote: > > > On Sun, Sep 05, 2021 at 09:39:57AM -0500, Ian Pilcher wrote: > > > > On 9/4/21 1:01 AM, Greg KH wrote: > > > > > Please never use WARN_ON() in new code unless the machine is really > > > > > broken and you can not do anything else here. > > > > > > > > Wait what? I thought that was BUG_ON. > > > > > > Not whan panic-on-warn is set, which is getting more and more common > > > these days. > > > > Fair enough. What is the recommend approach to reporting a "this should > > never" happen situation these days? > > dev_err() and handle the error properly. > > WARN_ON is the right choice for reporting recoverable kernel bugs, and BUG_ON for unrecoverable ones; see the two comments in include/asm-generic/bug.h which explain this. Please don't use dev_err() if it's a kernel bug (and not just unexpected input from userspace or hardware behaving weirdly), as that prevents the bug from being reported if it occurs. Greg, you've been corrected on this before, e.g. https://lore.kernel.org/linux-fsdevel/20210707023548.15872-1-desmondcheongzx@gmail.com/T/#u. Please stop spreading false information as it is destroying your credibility :-( - Eric
diff --git a/drivers/leds/trigger/ledtrig-blkdev.c b/drivers/leds/trigger/ledtrig-blkdev.c index 1f319529c3be..37ba9bb3542e 100644 --- a/drivers/leds/trigger/ledtrig-blkdev.c +++ b/drivers/leds/trigger/ledtrig-blkdev.c @@ -7,7 +7,9 @@ */ #include <linux/ctype.h> +#include <linux/leds.h> #include <linux/module.h> +#include <linux/part_stat.h> #include "ledtrig-blkdev.h" @@ -68,6 +70,10 @@ static unsigned int ledtrig_blkdev_count; /* How often to check for drive activity - in jiffies */ static unsigned int ledtrig_blkdev_interval; +/* Delayed work used to periodically check for activity & blink LEDs */ +static void blkdev_process(struct work_struct *const work); +static DECLARE_DELAYED_WORK(ledtrig_blkdev_work, blkdev_process); + /* * @@ -110,3 +116,85 @@ static bool blkdev_write_mode(const enum ledtrig_blkdev_mode mode) { return mode != LEDTRIG_BLKDEV_MODE_RO; } + + +/* + * + * Periodically check for device acitivity and blink LEDs + * + */ + +static void blkdev_blink(const struct ledtrig_blkdev_led *const led) +{ + unsigned long delay_on = READ_ONCE(led->blink_msec); + unsigned long delay_off = 1; /* 0 leaves LED turned on */ + + led_blink_set_oneshot(led->led_dev, &delay_on, &delay_off, 0); +} + +static void blkdev_update_disk(struct ledtrig_blkdev_disk *const disk, + const unsigned int generation) +{ + const struct block_device *const part0 = disk->gd->part0; + const unsigned long read_ios = part_stat_read(part0, ios[STAT_READ]); + const unsigned long write_ios = part_stat_read(part0, ios[STAT_WRITE]) + + part_stat_read(part0, ios[STAT_DISCARD]) + + part_stat_read(part0, ios[STAT_FLUSH]); + + if (disk->read_ios != read_ios) { + disk->read_act = true; + disk->read_ios = read_ios; + } else { + disk->read_act = false; + } + + if (disk->write_ios != write_ios) { + disk->write_act = true; + disk->write_ios = write_ios; + } else { + disk->write_act = false; + } + + disk->generation = generation; +} + +static void blkdev_process(struct work_struct *const work) +{ + static unsigned int generation; + + struct ledtrig_blkdev_led *led; + struct ledtrig_blkdev_link *link; + unsigned long delay; + + if (!mutex_trylock(&ledtrig_blkdev_mutex)) + goto exit_reschedule; + + hlist_for_each_entry(led, &ledtrig_blkdev_leds, leds_node) { + + hlist_for_each_entry(link, &led->disks, led_disks_node) { + + struct ledtrig_blkdev_disk *const disk = link->disk; + + if (disk->generation != generation) + blkdev_update_disk(disk, generation); + + if (disk->read_act && blkdev_read_mode(led->mode)) { + blkdev_blink(led); + break; + } + + if (disk->write_act && blkdev_write_mode(led->mode)) { + blkdev_blink(led); + break; + } + } + } + + ++generation; + + mutex_unlock(&ledtrig_blkdev_mutex); + +exit_reschedule: + delay = READ_ONCE(ledtrig_blkdev_interval); + WARN_ON_ONCE(!schedule_delayed_work(&ledtrig_blkdev_work, delay)); +}
Use a delayed workqueue to periodically check configured block devices for activity since the last check. Blink LEDs associated with devices on which the configured type of activity (read/write) has occurred. Signed-off-by: Ian Pilcher <arequipeno@gmail.com> --- drivers/leds/trigger/ledtrig-blkdev.c | 88 +++++++++++++++++++++++++++ 1 file changed, 88 insertions(+)