lkml.org 
[lkml]   [2022]   [Jun]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH V4 17/20] watchdog/dev: Add tracepoints
    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@linux-watchdog.org>
    > Cc: Guenter Roeck <linux@roeck-us.net>
    > Cc: Jonathan Corbet <corbet@lwn.net>
    > Cc: Steven Rostedt <rostedt@goodmis.org>
    > Cc: Ingo Molnar <mingo@redhat.com>
    > Cc: Thomas Gleixner <tglx@linutronix.de>
    > Cc: Peter Zijlstra <peterz@infradead.org>
    > Cc: Will Deacon <will@kernel.org>
    > Cc: Catalin Marinas <catalin.marinas@arm.com>
    > Cc: Marco Elver <elver@google.com>
    > Cc: Dmitry Vyukov <dvyukov@google.com>
    > Cc: "Paul E. McKenney" <paulmck@kernel.org>
    > Cc: Shuah Khan <skhan@linuxfoundation.org>
    > Cc: Gabriele Paoloni <gpaoloni@redhat.com>
    > Cc: Juri Lelli <juri.lelli@redhat.com>
    > Cc: Clark Williams <williams@redhat.com>
    > Cc: linux-doc@vger.kernel.org
    > Cc: linux-kernel@vger.kernel.org
    > Cc: linux-trace-devel@vger.kernel.org
    > Signed-off-by: Daniel Bristot de Oliveira <bristot@kernel.org>
    > ---
    > 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.


    > 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.

    > 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.

    > + ),
    > +
    > + TP_fast_assign(
    > + __entry->id = wdd->id;
    > + __entry->timeout = timeout;
    > + ),
    > +
    > + TP_printk("id=%d timeout=%llus",
    > + __entry->id, __entry->timeout)
    > +);
    > +
    > +DEFINE_EVENT(watchdog_timeout_template, watchdog_set_timeout,
    > + TP_PROTO(struct watchdog_device *wdd, u64 timeout),
    > + TP_ARGS(wdd, timeout));
    > +
    > +DEFINE_EVENT(watchdog_timeout_template, watchdog_set_pretimeout,
    > + TP_PROTO(struct watchdog_device *wdd, u64 timeout),
    > + TP_ARGS(wdd, timeout));
    > +
    > +DEFINE_EVENT(watchdog_timeout_template, watchdog_set_keep_alive,
    > + TP_PROTO(struct watchdog_device *wdd, u64 timeout),
    > + TP_ARGS(wdd, timeout));
    > +
    > +#endif /* _TRACE_WATCHDOG_H */
    > +
    > +/* This part must be outside protection */
    > +#include <trace/define_trace.h>

    \
     
     \ /
      Last update: 2022-06-16 15:47    [W:6.779 / U:0.556 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site