* [PATCH v2] watchdog: Add tracing events for the most usual watchdog events
@ 2022-10-04 9:19 Uwe Kleine-König
2022-10-04 22:51 ` Steven Rostedt
0 siblings, 1 reply; 6+ messages in thread
From: Uwe Kleine-König @ 2022-10-04 9:19 UTC (permalink / raw)
To: Wim Van Sebroeck, Guenter Roeck
Cc: kernel, Ingo Molnar, linux-watchdog, Steven Rostedt, linux-kernel
To simplify debugging which process touches a watchdog and when, add
tracing events for .start(), .set_timeout(), .ping() and .stop().
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
Changes since (implict) v1 sent with Message-Id:
20220930144935.3373366-1-u.kleine-koenig@pengutronix.de:
- Make use of DECLARE_EVENT_CLASS as {requ,sugg}ested by Steven.
Best regards
Uwe
MAINTAINERS | 1 +
drivers/watchdog/watchdog_core.c | 4 ++
drivers/watchdog/watchdog_dev.c | 12 +++++-
include/trace/events/watchdog.h | 66 ++++++++++++++++++++++++++++++++
4 files changed, 81 insertions(+), 2 deletions(-)
create mode 100644 include/trace/events/watchdog.h
diff --git a/MAINTAINERS b/MAINTAINERS
index 72b9654f764c..507de599116b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -21890,6 +21890,7 @@ F: Documentation/watchdog/
F: drivers/watchdog/
F: include/linux/watchdog.h
F: include/uapi/linux/watchdog.h
+F: include/trace/events/watchdog.h
WHISKEYCOVE PMIC GPIO DRIVER
M: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
diff --git a/drivers/watchdog/watchdog_core.c b/drivers/watchdog/watchdog_core.c
index 3fe8a7edc252..c777a612d932 100644
--- a/drivers/watchdog/watchdog_core.c
+++ b/drivers/watchdog/watchdog_core.c
@@ -38,6 +38,9 @@
#include "watchdog_core.h" /* For watchdog_dev_register/... */
+#define CREATE_TRACE_POINTS
+#include <trace/events/watchdog.h>
+
static DEFINE_IDA(watchdog_ida);
static int stop_on_reboot = -1;
@@ -163,6 +166,7 @@ static int watchdog_reboot_notifier(struct notifier_block *nb,
int ret;
ret = wdd->ops->stop(wdd);
+ trace_watchdog_stop(wdd, ret);
if (ret)
return NOTIFY_BAD;
}
diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c
index 54903f3c851e..3323309c913c 100644
--- a/drivers/watchdog/watchdog_dev.c
+++ b/drivers/watchdog/watchdog_dev.c
@@ -47,6 +47,8 @@
#include "watchdog_core.h"
#include "watchdog_pretimeout.h"
+#include <trace/events/watchdog.h>
+
/* the dev_t structure to store the dynamically allocated watchdog devices */
static dev_t watchdog_devt;
/* Reference to watchdog device behind /dev/watchdog */
@@ -157,10 +159,13 @@ static int __watchdog_ping(struct watchdog_device *wdd)
wd_data->last_hw_keepalive = now;
- if (wdd->ops->ping)
+ if (wdd->ops->ping) {
err = wdd->ops->ping(wdd); /* ping the watchdog */
- else
+ trace_watchdog_ping(wdd, err);
+ } else {
err = wdd->ops->start(wdd); /* restart watchdog */
+ trace_watchdog_start(wdd, err);
+ }
if (err == 0)
watchdog_hrtimer_pretimeout_start(wdd);
@@ -259,6 +264,7 @@ static int watchdog_start(struct watchdog_device *wdd)
}
} else {
err = wdd->ops->start(wdd);
+ trace_watchdog_start(wdd, err);
if (err == 0) {
set_bit(WDOG_ACTIVE, &wdd->status);
wd_data->last_keepalive = started_at;
@@ -297,6 +303,7 @@ static int watchdog_stop(struct watchdog_device *wdd)
if (wdd->ops->stop) {
clear_bit(WDOG_HW_RUNNING, &wdd->status);
err = wdd->ops->stop(wdd);
+ trace_watchdog_stop(wdd, err);
} else {
set_bit(WDOG_HW_RUNNING, &wdd->status);
}
@@ -369,6 +376,7 @@ static int watchdog_set_timeout(struct watchdog_device *wdd,
if (wdd->ops->set_timeout) {
err = wdd->ops->set_timeout(wdd, timeout);
+ trace_watchdog_set_timeout(wdd, timeout, err);
} else {
wdd->timeout = timeout;
/* Disable pretimeout if it doesn't fit the new timeout */
diff --git a/include/trace/events/watchdog.h b/include/trace/events/watchdog.h
new file mode 100644
index 000000000000..77802d0d8c09
--- /dev/null
+++ b/include/trace/events/watchdog.h
@@ -0,0 +1,66 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM watchdog
+
+#if !defined(_TRACE_WATCHDOG_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_WATCHDOG_H
+
+#include <linux/watchdog.h>
+#include <linux/tracepoint.h>
+
+DECLARE_EVENT_CLASS(watchdog_template,
+
+ TP_PROTO(struct watchdog_device *wdd, int err),
+
+ TP_ARGS(wdd, err),
+
+ TP_STRUCT__entry(
+ __field(int, id)
+ __field(int, err)
+ ),
+
+ TP_fast_assign(
+ __entry->id = wdd->id;
+ __entry->err = err;
+ ),
+
+ TP_printk("watchdog%d err=%d", __entry->id, __entry->err)
+);
+
+DEFINE_EVENT(watchdog_template, watchdog_start,
+ TP_PROTO(struct watchdog_device *wdd, int err),
+ TP_ARGS(wdd, err));
+
+TRACE_EVENT(watchdog_set_timeout,
+
+ TP_PROTO(struct watchdog_device *wdd, unsigned int timeout, int err),
+
+ TP_ARGS(wdd, timeout, err),
+
+ TP_STRUCT__entry(
+ __field(int, id)
+ __field(unsigned int, timeout)
+ __field(int, err)
+ ),
+
+ TP_fast_assign(
+ __entry->id = wdd->id;
+ __entry->timeout = timeout;
+ __entry->err = err;
+ ),
+
+ TP_printk("watchdog%d timeout=%u err=%d", __entry->id, __entry->timeout, __entry->err)
+);
+
+DEFINE_EVENT(watchdog_template, watchdog_ping,
+ TP_PROTO(struct watchdog_device *wdd, int err),
+ TP_ARGS(wdd, err));
+
+DEFINE_EVENT(watchdog_template, watchdog_stop,
+ TP_PROTO(struct watchdog_device *wdd, int err),
+ TP_ARGS(wdd, err));
+
+#endif /* !defined(_TRACE_WATCHDOG_H) || defined(TRACE_HEADER_MULTI_READ) */
+
+/* This part must be outside protection */
+#include <trace/define_trace.h>
base-commit: 4fe89d07dcc2804c8b562f6c7896a45643d34b2f
--
2.37.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2] watchdog: Add tracing events for the most usual watchdog events
2022-10-04 9:19 [PATCH v2] watchdog: Add tracing events for the most usual watchdog events Uwe Kleine-König
@ 2022-10-04 22:51 ` Steven Rostedt
2022-10-05 7:19 ` Uwe Kleine-König
0 siblings, 1 reply; 6+ messages in thread
From: Steven Rostedt @ 2022-10-04 22:51 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Wim Van Sebroeck, Guenter Roeck, kernel, Ingo Molnar,
linux-watchdog, linux-kernel
On Tue, 4 Oct 2022 11:19:49 +0200
Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote:
> +DEFINE_EVENT(watchdog_template, watchdog_start,
> + TP_PROTO(struct watchdog_device *wdd, int err),
> + TP_ARGS(wdd, err));
> +
> +TRACE_EVENT(watchdog_set_timeout,
> +
> + TP_PROTO(struct watchdog_device *wdd, unsigned int timeout, int err),
> +
> + TP_ARGS(wdd, timeout, err),
> +
> + TP_STRUCT__entry(
> + __field(int, id)
> + __field(unsigned int, timeout)
> + __field(int, err)
> + ),
> +
> + TP_fast_assign(
> + __entry->id = wdd->id;
> + __entry->timeout = timeout;
> + __entry->err = err;
> + ),
> +
> + TP_printk("watchdog%d timeout=%u err=%d", __entry->id, __entry->timeout, __entry->err)
> +);
Nit, but I would probably put the above TRACE_EVENT() below the two
DEFINE_EVENT()s below. That way we have all the DEFINE_EVENT()s for a
specific DECLARE_EVENT_CLASS() together. Otherwise people may get confused.
-- Steve
> +
> +DEFINE_EVENT(watchdog_template, watchdog_ping,
> + TP_PROTO(struct watchdog_device *wdd, int err),
> + TP_ARGS(wdd, err));
> +
> +DEFINE_EVENT(watchdog_template, watchdog_stop,
> + TP_PROTO(struct watchdog_device *wdd, int err),
> + TP_ARGS(wdd, err));
> +
> +#endif /* !defined(_TRACE_WATCHDOG_H) || defined(TRACE_HEADER_MULTI_READ) */
> +
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] watchdog: Add tracing events for the most usual watchdog events
2022-10-04 22:51 ` Steven Rostedt
@ 2022-10-05 7:19 ` Uwe Kleine-König
2022-10-05 19:39 ` Guenter Roeck
0 siblings, 1 reply; 6+ messages in thread
From: Uwe Kleine-König @ 2022-10-05 7:19 UTC (permalink / raw)
To: Steven Rostedt
Cc: linux-watchdog, linux-kernel, Ingo Molnar, kernel,
Wim Van Sebroeck, Guenter Roeck
[-- Attachment #1: Type: text/plain, Size: 1597 bytes --]
On Tue, Oct 04, 2022 at 06:51:46PM -0400, Steven Rostedt wrote:
> On Tue, 4 Oct 2022 11:19:49 +0200
> Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote:
>
> > +DEFINE_EVENT(watchdog_template, watchdog_start,
> > + TP_PROTO(struct watchdog_device *wdd, int err),
> > + TP_ARGS(wdd, err));
> > +
> > +TRACE_EVENT(watchdog_set_timeout,
> > +
> > + TP_PROTO(struct watchdog_device *wdd, unsigned int timeout, int err),
> > +
> > + TP_ARGS(wdd, timeout, err),
> > +
> > + TP_STRUCT__entry(
> > + __field(int, id)
> > + __field(unsigned int, timeout)
> > + __field(int, err)
> > + ),
> > +
> > + TP_fast_assign(
> > + __entry->id = wdd->id;
> > + __entry->timeout = timeout;
> > + __entry->err = err;
> > + ),
> > +
> > + TP_printk("watchdog%d timeout=%u err=%d", __entry->id, __entry->timeout, __entry->err)
> > +);
>
> Nit, but I would probably put the above TRACE_EVENT() below the two
> DEFINE_EVENT()s below. That way we have all the DEFINE_EVENT()s for a
> specific DECLARE_EVENT_CLASS() together. Otherwise people may get confused.
I thought about that, too. The argument for the order I chose is that
having start at the start and stop at the end is also intuitive.
But I don't care much and would let the watchdog guys decide what they
prefer.
@Wim+Guenter: Feel free to reorder at application time or ask for a v3
if this v2 doesn't fit your preference.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] watchdog: Add tracing events for the most usual watchdog events
2022-10-05 7:19 ` Uwe Kleine-König
@ 2022-10-05 19:39 ` Guenter Roeck
2022-10-05 21:11 ` Steven Rostedt
0 siblings, 1 reply; 6+ messages in thread
From: Guenter Roeck @ 2022-10-05 19:39 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Steven Rostedt, linux-watchdog, linux-kernel, Ingo Molnar,
kernel, Wim Van Sebroeck
On Wed, Oct 05, 2022 at 09:19:46AM +0200, Uwe Kleine-König wrote:
> On Tue, Oct 04, 2022 at 06:51:46PM -0400, Steven Rostedt wrote:
> > On Tue, 4 Oct 2022 11:19:49 +0200
> > Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote:
> >
> > > +DEFINE_EVENT(watchdog_template, watchdog_start,
> > > + TP_PROTO(struct watchdog_device *wdd, int err),
> > > + TP_ARGS(wdd, err));
> > > +
> > > +TRACE_EVENT(watchdog_set_timeout,
> > > +
> > > + TP_PROTO(struct watchdog_device *wdd, unsigned int timeout, int err),
> > > +
> > > + TP_ARGS(wdd, timeout, err),
> > > +
> > > + TP_STRUCT__entry(
> > > + __field(int, id)
> > > + __field(unsigned int, timeout)
> > > + __field(int, err)
> > > + ),
> > > +
> > > + TP_fast_assign(
> > > + __entry->id = wdd->id;
> > > + __entry->timeout = timeout;
> > > + __entry->err = err;
> > > + ),
> > > +
> > > + TP_printk("watchdog%d timeout=%u err=%d", __entry->id, __entry->timeout, __entry->err)
> > > +);
> >
> > Nit, but I would probably put the above TRACE_EVENT() below the two
> > DEFINE_EVENT()s below. That way we have all the DEFINE_EVENT()s for a
> > specific DECLARE_EVENT_CLASS() together. Otherwise people may get confused.
>
> I thought about that, too. The argument for the order I chose is that
> having start at the start and stop at the end is also intuitive.
>
> But I don't care much and would let the watchdog guys decide what they
> prefer.
>
> @Wim+Guenter: Feel free to reorder at application time or ask for a v3
> if this v2 doesn't fit your preference.
For my part I would prefer a version with Steven's Reviewed-by: tag,
whatever it is.
Guenter
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] watchdog: Add tracing events for the most usual watchdog events
2022-10-05 19:39 ` Guenter Roeck
@ 2022-10-05 21:11 ` Steven Rostedt
2022-10-08 15:50 ` Guenter Roeck
0 siblings, 1 reply; 6+ messages in thread
From: Steven Rostedt @ 2022-10-05 21:11 UTC (permalink / raw)
To: Guenter Roeck
Cc: Uwe Kleine-König, linux-watchdog, linux-kernel, Ingo Molnar,
kernel, Wim Van Sebroeck
On Wed, 5 Oct 2022 12:39:24 -0700
Guenter Roeck <linux@roeck-us.net> wrote:
> > > Nit, but I would probably put the above TRACE_EVENT() below the two
> > > DEFINE_EVENT()s below. That way we have all the DEFINE_EVENT()s for a
> > > specific DECLARE_EVENT_CLASS() together. Otherwise people may get confused.
> >
> > I thought about that, too. The argument for the order I chose is that
> > having start at the start and stop at the end is also intuitive.
> >
> > But I don't care much and would let the watchdog guys decide what they
> > prefer.
> >
> > @Wim+Guenter: Feel free to reorder at application time or ask for a v3
> > if this v2 doesn't fit your preference.
>
> For my part I would prefer a version with Steven's Reviewed-by: tag,
> whatever it is.
I much rather have the DEFINE_EVENTS followed by the DECLARE_EVENT_CLASS()
as that's what most people look for.
For start and stop being together, I believe that will not trip many people
up, where as the DEFINE_EVENTS() scattering will.
I'm OK if maintainers of the code are fine with the scattering, but it will
break precedence.
-- Steve
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] watchdog: Add tracing events for the most usual watchdog events
2022-10-05 21:11 ` Steven Rostedt
@ 2022-10-08 15:50 ` Guenter Roeck
0 siblings, 0 replies; 6+ messages in thread
From: Guenter Roeck @ 2022-10-08 15:50 UTC (permalink / raw)
To: Steven Rostedt
Cc: Uwe Kleine-König, linux-watchdog, linux-kernel, Ingo Molnar,
kernel, Wim Van Sebroeck
On Wed, Oct 05, 2022 at 05:11:36PM -0400, Steven Rostedt wrote:
> On Wed, 5 Oct 2022 12:39:24 -0700
> Guenter Roeck <linux@roeck-us.net> wrote:
>
> > > > Nit, but I would probably put the above TRACE_EVENT() below the two
> > > > DEFINE_EVENT()s below. That way we have all the DEFINE_EVENT()s for a
> > > > specific DECLARE_EVENT_CLASS() together. Otherwise people may get confused.
> > >
> > > I thought about that, too. The argument for the order I chose is that
> > > having start at the start and stop at the end is also intuitive.
> > >
> > > But I don't care much and would let the watchdog guys decide what they
> > > prefer.
> > >
> > > @Wim+Guenter: Feel free to reorder at application time or ask for a v3
> > > if this v2 doesn't fit your preference.
> >
> > For my part I would prefer a version with Steven's Reviewed-by: tag,
> > whatever it is.
>
> I much rather have the DEFINE_EVENTS followed by the DECLARE_EVENT_CLASS()
> as that's what most people look for.
>
> For start and stop being together, I believe that will not trip many people
> up, where as the DEFINE_EVENTS() scattering will.
>
I agree. Uwe, please send v3 with those changes.
Thanks,
Guenter
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-10-08 15:50 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-04 9:19 [PATCH v2] watchdog: Add tracing events for the most usual watchdog events Uwe Kleine-König
2022-10-04 22:51 ` Steven Rostedt
2022-10-05 7:19 ` Uwe Kleine-König
2022-10-05 19:39 ` Guenter Roeck
2022-10-05 21:11 ` Steven Rostedt
2022-10-08 15:50 ` Guenter Roeck
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).