On 6/16/22 08:47, Daniel Bristot de Oliveira wrote:
On 6/16/22 15:44, Guenter Roeck wrote:On 6/16/22 01:44, Daniel Bristot de Oliveira wrote:Add a set of tracepoints, enabling the observability of the watchdog device interactions with user-space. The events are: watchdog:watchdog_open watchdog:watchdog_close watchdog:watchdog_start watchdog:watchdog_stop watchdog:watchdog_set_timeout watchdog:watchdog_ping watchdog:watchdog_nowayout watchdog:watchdog_set_keep_alive watchdog:watchdog_keep_alive watchdog:watchdog_set_pretimeout watchdog:watchdog_pretimeout Cc: Wim Van Sebroeck <wim@xxxxxxxxxxxxxxxxxx> Cc: Guenter Roeck <linux@xxxxxxxxxxxx> Cc: Jonathan Corbet <corbet@xxxxxxx> Cc: Steven Rostedt <rostedt@xxxxxxxxxxx> Cc: Ingo Molnar <mingo@xxxxxxxxxx> Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx> Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx> Cc: Will Deacon <will@xxxxxxxxxx> Cc: Catalin Marinas <catalin.marinas@xxxxxxx> Cc: Marco Elver <elver@xxxxxxxxxx> Cc: Dmitry Vyukov <dvyukov@xxxxxxxxxx> Cc: "Paul E. McKenney" <paulmck@xxxxxxxxxx> Cc: Shuah Khan <skhan@xxxxxxxxxxxxxxxxxxx> Cc: Gabriele Paoloni <gpaoloni@xxxxxxxxxx> Cc: Juri Lelli <juri.lelli@xxxxxxxxxx> Cc: Clark Williams <williams@xxxxxxxxxx> Cc: linux-doc@xxxxxxxxxxxxxxx Cc: linux-kernel@xxxxxxxxxxxxxxx Cc: linux-trace-devel@xxxxxxxxxxxxxxx Signed-off-by: Daniel Bristot de Oliveira <bristot@xxxxxxxxxx> --- drivers/watchdog/watchdog_dev.c | 43 ++++++++++- drivers/watchdog/watchdog_pretimeout.c | 2 + include/linux/watchdog.h | 7 +- include/trace/events/watchdog.h | 101 +++++++++++++++++++++++++ 4 files changed, 143 insertions(+), 10 deletions(-) create mode 100644 include/trace/events/watchdog.h diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c index 54903f3c851e..2f28dc5ab763 100644 --- a/drivers/watchdog/watchdog_dev.c +++ b/drivers/watchdog/watchdog_dev.c @@ -44,6 +44,9 @@ #include <linux/watchdog.h> /* For watchdog specific items */ #include <linux/uaccess.h> /* For copy_to_user/put_user/... */ +#define CREATE_TRACE_POINTS +#include <trace/events/watchdog.h> + #include "watchdog_core.h" #include "watchdog_pretimeout.h" @@ -130,9 +133,11 @@ static inline void watchdog_update_worker(struct watchdog_device *wdd) if (watchdog_need_worker(wdd)) { ktime_t t = watchdog_next_keepalive(wdd); - if (t > 0) + if (t > 0) { hrtimer_start(&wd_data->timer, t, HRTIMER_MODE_REL_HARD); + trace_watchdog_set_keep_alive(wdd, ktime_to_ms(t)); + } } else { hrtimer_cancel(&wd_data->timer); } @@ -141,7 +146,7 @@ static inline void watchdog_update_worker(struct watchdog_device *wdd) static int __watchdog_ping(struct watchdog_device *wdd) { struct watchdog_core_data *wd_data = wdd->wd_data; - ktime_t earliest_keepalive, now; + ktime_t earliest_keepalive, now, next_keepalive; int err; earliest_keepalive = ktime_add(wd_data->last_hw_keepalive, @@ -149,14 +154,16 @@ static int __watchdog_ping(struct watchdog_device *wdd) now = ktime_get(); if (ktime_after(earliest_keepalive, now)) { - hrtimer_start(&wd_data->timer, - ktime_sub(earliest_keepalive, now), + next_keepalive = ktime_sub(earliest_keepalive, now); + hrtimer_start(&wd_data->timer, next_keepalive, HRTIMER_MODE_REL_HARD); + trace_watchdog_set_keep_alive(wdd, ktime_to_ms(next_keepalive)); return 0; } wd_data->last_hw_keepalive = now; + trace_watchdog_ping(wdd); if (wdd->ops->ping) err = wdd->ops->ping(wdd); /* ping the watchdog */ else @@ -215,6 +222,7 @@ static void watchdog_ping_work(struct kthread_work *work) wd_data = container_of(work, struct watchdog_core_data, work); mutex_lock(&wd_data->lock); + trace_watchdog_keep_alive(wd_data->wdd); if (watchdog_worker_should_ping(wd_data)) __watchdog_ping(wd_data->wdd); mutex_unlock(&wd_data->lock); @@ -250,6 +258,8 @@ static int watchdog_start(struct watchdog_device *wdd) set_bit(_WDOG_KEEPALIVE, &wd_data->status); + trace_watchdog_start(wdd); + started_at = ktime_get(); if (watchdog_hw_running(wdd) && wdd->ops->ping) { err = __watchdog_ping(wdd); @@ -294,6 +304,7 @@ static int watchdog_stop(struct watchdog_device *wdd) return -EBUSY; } + trace_watchdog_stop(wdd); if (wdd->ops->stop) { clear_bit(WDOG_HW_RUNNING, &wdd->status); err = wdd->ops->stop(wdd); @@ -367,6 +378,7 @@ static int watchdog_set_timeout(struct watchdog_device *wdd, if (watchdog_timeout_invalid(wdd, timeout)) return -EINVAL; + trace_watchdog_set_timeout(wdd, timeout);The driver has no obligation to set the timeout to the requested value. It might be more valuable to report both the requested and the actual values.Ack! how do I get the actual value?
Read it from the data structure after the driver function returned.
if (wdd->ops->set_timeout) { err = wdd->ops->set_timeout(wdd, timeout); } else { @@ -399,6 +411,8 @@ static int watchdog_set_pretimeout(struct watchdog_device *wdd, if (watchdog_pretimeout_invalid(wdd, timeout)) return -EINVAL; + trace_watchdog_set_pretimeout(wdd, timeout); +Again, the driver has no obligation to set the timeout to the requested value./me takes note.if (wdd->ops->set_pretimeout && (wdd->info->options & WDIOF_PRETIMEOUT)) err = wdd->ops->set_pretimeout(wdd, timeout); else @@ -430,6 +444,23 @@ static int watchdog_get_timeleft(struct watchdog_device *wdd, return 0; } +/** + * watchdog_set_nowayout - set nowaout bit + * @wdd: The watchdog device to set nowayoutbit + * @nowayout A boolean on/off switcher + * + * If nowayout boolean is true, the nowayout option is set. No action is + * taken if nowayout is false. + */ +void watchdog_set_nowayout(struct watchdog_device *wdd, bool nowayout) +{ + if (nowayout) { + set_bit(WDOG_NO_WAY_OUT, &wdd->status); + trace_watchdog_nowayout(wdd); + } +} +EXPORT_SYMBOL(watchdog_set_nowayout); + #ifdef CONFIG_WATCHDOG_SYSFS static ssize_t nowayout_show(struct device *dev, struct device_attribute *attr, char *buf) @@ -861,6 +892,8 @@ static int watchdog_open(struct inode *inode, struct file *file) goto out_clear; } + trace_watchdog_open(wdd); + err = watchdog_start(wdd); if (err < 0) goto out_mod; @@ -883,6 +916,7 @@ static int watchdog_open(struct inode *inode, struct file *file) return stream_open(inode, file); out_mod: + trace_watchdog_close(wdd); module_put(wd_data->wdd->ops->owner); out_clear: clear_bit(_WDOG_DEV_OPEN, &wd_data->status); @@ -944,6 +978,7 @@ static int watchdog_release(struct inode *inode, struct file *file) /* make sure that /dev/watchdog can be re-opened */ clear_bit(_WDOG_DEV_OPEN, &wd_data->status); + trace_watchdog_close(wdd); done: running = wdd && watchdog_hw_running(wdd); mutex_unlock(&wd_data->lock); diff --git a/drivers/watchdog/watchdog_pretimeout.c b/drivers/watchdog/watchdog_pretimeout.c index 376a495ab80c..58c391ed2205 100644 --- a/drivers/watchdog/watchdog_pretimeout.c +++ b/drivers/watchdog/watchdog_pretimeout.c @@ -8,6 +8,7 @@ #include <linux/spinlock.h> #include <linux/string.h> #include <linux/watchdog.h> +#include <trace/events/watchdog.h> #include "watchdog_core.h" #include "watchdog_pretimeout.h" @@ -107,6 +108,7 @@ void watchdog_notify_pretimeout(struct watchdog_device *wdd) return; } + trace_watchdog_pretimeout(wdd); wdd->gov->pretimeout(wdd); spin_unlock_irqrestore(&pretimeout_lock, flags); } diff --git a/include/linux/watchdog.h b/include/linux/watchdog.h index 99660197a36c..11d93407e492 100644 --- a/include/linux/watchdog.h +++ b/include/linux/watchdog.h @@ -139,12 +139,7 @@ static inline bool watchdog_hw_running(struct watchdog_device *wdd) return test_bit(WDOG_HW_RUNNING, &wdd->status); } -/* Use the following function to set the nowayout feature */ -static inline void watchdog_set_nowayout(struct watchdog_device *wdd, bool nowayout) -{ - if (nowayout) - set_bit(WDOG_NO_WAY_OUT, &wdd->status); -} +void watchdog_set_nowayout(struct watchdog_device *wdd, bool nowayout); /* Use the following function to stop the watchdog on reboot */ static inline void watchdog_stop_on_reboot(struct watchdog_device *wdd) diff --git a/include/trace/events/watchdog.h b/include/trace/events/watchdog.h new file mode 100644 index 000000000000..145cd6cfaa02 --- /dev/null +++ b/include/trace/events/watchdog.h @@ -0,0 +1,101 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#undef TRACE_SYSTEM +#define TRACE_SYSTEM watchdog + +#if !defined(_TRACE_WATCHDOG_H) || defined(TRACE_HEADER_MULTI_READ) +#define _TRACE_WATCHDOG_H + +#include <linux/tracepoint.h> + +/* + * These are all events whose sole argument is the watchdog id. + */ +DECLARE_EVENT_CLASS(dev_operations_template, + + TP_PROTO(struct watchdog_device *wdd), + + TP_ARGS(wdd), + + TP_STRUCT__entry( + __field(__u32, id) + ), + + TP_fast_assign( + __entry->id = wdd->id; + ), + + TP_printk("id=%d", + __entry->id) +); + +DEFINE_EVENT(dev_operations_template, watchdog_open, + TP_PROTO(struct watchdog_device *wdd), + TP_ARGS(wdd)); + +DEFINE_EVENT(dev_operations_template, watchdog_close, + TP_PROTO(struct watchdog_device *wdd), + TP_ARGS(wdd)); + +DEFINE_EVENT(dev_operations_template, watchdog_start, + TP_PROTO(struct watchdog_device *wdd), + TP_ARGS(wdd)); + +DEFINE_EVENT(dev_operations_template, watchdog_stop, + TP_PROTO(struct watchdog_device *wdd), + TP_ARGS(wdd)); + +DEFINE_EVENT(dev_operations_template, watchdog_ping, + TP_PROTO(struct watchdog_device *wdd), + TP_ARGS(wdd)); + +DEFINE_EVENT(dev_operations_template, watchdog_nowayout, + TP_PROTO(struct watchdog_device *wdd), + TP_ARGS(wdd)); + +DEFINE_EVENT(dev_operations_template, watchdog_keep_alive, + TP_PROTO(struct watchdog_device *wdd), + TP_ARGS(wdd)); + +DEFINE_EVENT(dev_operations_template, watchdog_pretimeout, + TP_PROTO(struct watchdog_device *wdd), + TP_ARGS(wdd)); + +/* + * These are all events with a device ID and a given timeout. + */ +DECLARE_EVENT_CLASS(watchdog_timeout_template, + + TP_PROTO(struct watchdog_device *wdd, u64 timeout), + + TP_ARGS(wdd, timeout), + + TP_STRUCT__entry( + __field(__u32, id) + __field(__u64, timeout)Why u64 ? timeout is unsigned long.ack! I will change it. (I am seeing unsigned int, am I missing something?).
Yes, you are correct. Guenter
