[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <16ab4e96-756c-bd00-5275-0d8d03e233fc@roeck-us.net>
Date: Sat, 8 Oct 2022 15:13:21 -0700
From: Guenter Roeck <linux@...ck-us.net>
To: Uwe Kleine-König <u.kleine-koenig@...gutronix.de>,
Wim Van Sebroeck <wim@...ux-watchdog.org>
Cc: Ingo Molnar <mingo@...hat.com>,
Steven Rostedt <rostedt@...dmis.org>,
linux-watchdog@...r.kernel.org, kernel@...gutronix.de,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3] watchdog: Add tracing events for the most usual
watchdog events
On 10/8/22 10:46, Uwe Kleine-König wrote:
> 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@...gutronix.de>
Reviewed-by: Guenter Roeck <linux@...ck-us.net>
> ---
> Changes since v2 sent with Message-Id:
> 20221004091950.3419662-1-u.kleine-koenig@...gutronix.de:
>
> - Reorder events in the header to have all DEFINE_EVENTS near their
> DECLARE_EVENT_CLASS.
>
> Changes since (implict) v1 sent with Message-Id:
> 20220930144935.3373366-1-u.kleine-koenig@...gutronix.de:
>
> - Make use of DECLARE_EVENT_CLASS as {requ,sugg}ested by Steven.
>
> 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@...ux.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..beb9bb3424c8
> --- /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));
> +
> +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));
> +
> +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)
> +);
> +
> +#endif /* !defined(_TRACE_WATCHDOG_H) || defined(TRACE_HEADER_MULTI_READ) */
> +
> +/* This part must be outside protection */
> +#include <trace/define_trace.h>
>
> base-commit: 4fe89d07dcc2804c8b562f6c7896a45643d34b2f
Powered by blists - more mailing lists