| Message ID | e153b772306577bcb3915474ed10eb3dcb228eda.1655368610.git.bristot@kernel.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | The Runtime Verification (RV) interface | expand |
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>
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@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. > > Ack! how do I get the actual value? >> 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?). Thanks! -- Daniel
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@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. >> >> > > 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
On 6/17/22 01:55, Guenter Roeck wrote: > 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@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. >>> >>> >> >> Ack! how do I get the actual value? >> > Read it from the data structure after the driver function returned. Got it! I will add this field too. -- Daniel
On 6/17/22 01:55, Guenter Roeck wrote: >> >> ack! I will change it. (I am seeing unsigned int, am I missing something?). > > Yes, you are correct. Fixed! -- Daniel
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); 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); + 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) + ), + + 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>
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