On 2/14/22 11:45, 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
Some watchdogs have a bark functionality, I think it should be event for that too.
> 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 | 41 ++++++++++++-
> include/linux/watchdog.h | 7 +--
> include/trace/events/watchdog.h | 103 ++++++++++++++++++++++++++++++++
> 3 files changed, 142 insertions(+), 9 deletions(-)
> create mode 100644 include/trace/events/watchdog.h
>
> diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c
> index 3a3d8b5c7ad5..0beeac5d4541 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);
> }
> @@ -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),
> + ktime_t t = ktime_sub(earliest_keepalive, now);
> + hrtimer_start(&wd_data->timer, t,
> HRTIMER_MODE_REL_HARD);
> + trace_watchdog_set_keep_alive(wdd, ktime_to_ms(t));
> 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);
> @@ -252,6 +260,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);
> @@ -298,6 +308,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);
> @@ -370,6 +381,7 @@ static int watchdog_set_timeout(struct watchdog_device *wdd,
> if (watchdog_timeout_invalid(wdd, timeout))
> return -EINVAL;
>
> + trace_watchdog_set_timeout(wdd, timeout);
> if (wdd->ops->set_timeout) {
> err = wdd->ops->set_timeout(wdd, timeout);
> } else {
> @@ -432,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)
> @@ -457,6 +486,7 @@ static ssize_t nowayout_store(struct device *dev, struct device_attribute *attr,
> /* nowayout cannot be disabled once set */
> if (test_bit(WDOG_NO_WAY_OUT, &wdd->status) && !value)
> return -EPERM;
> +
> watchdog_set_nowayout(wdd, value);
> return len;
> }
> @@ -858,6 +888,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;
> @@ -880,6 +912,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);
> @@ -940,6 +973,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);
> @@ -952,6 +986,7 @@ static int watchdog_release(struct inode *inode, struct file *file)
> module_put(wd_data->cdev.owner);
> put_device(&wd_data->dev);
> }
> +
> return 0;
> }
>
> 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..5d5617ab611a
> --- /dev/null
> +++ b/include/trace/events/watchdog.h
> @@ -0,0 +1,103 @@
> +/* 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>
> +
> +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)
> +);
> +
> +/*
> + * Add a comment
> + */
> +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_keep_alive,
> + 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));
> +
> +
> +TRACE_EVENT(watchdog_set_timeout,
> +
> + TP_PROTO(struct watchdog_device *wdd, u64 timeout),
> +
> + TP_ARGS(wdd, timeout),
> +
> + TP_STRUCT__entry(
> + __field(__u32, id)
> + __field(__u64, timeout)
> + ),
> +
> + TP_fast_assign(
> + __entry->id = wdd->id;
> + __entry->timeout = timeout;
> + ),
> +
> + TP_printk("id=%d timeout=%llus",
> + __entry->id, __entry->timeout)
> +);
> +
> +TRACE_EVENT(watchdog_set_keep_alive,
> +
> + TP_PROTO(struct watchdog_device *wdd, u64 timeout),
> +
> + TP_ARGS(wdd, timeout),
> +
> + TP_STRUCT__entry(
> + __field(__u32, id)
> + __field(__u64, timeout)
> + ),
> +
> + TP_fast_assign(
> + __entry->id = wdd->id;
> + __entry->timeout = timeout;
> + ),
> +
> + TP_printk("id=%d keep_alive=%llums",
> + __entry->id, __entry->timeout)
> +);
> +
> +#endif /* _TRACE_WATCHDOG_H */
> +
> +/* This part must be outside protection */
> +#include <trace/define_trace.h>
