| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <e20d30c4-2971-5e1f-f7dd-be30560d5689@kernel.org>
Date: Fri, 17 Jun 2022 18:16:36 +0200
From: Daniel Bristot de Oliveira <bristot@...nel.org>
To: Guenter Roeck <linux@...ck-us.net>,
Steven Rostedt <rostedt@...dmis.org>
Cc: Wim Van Sebroeck <wim@...ux-watchdog.org>,
Jonathan Corbet <corbet@....net>,
Ingo Molnar <mingo@...hat.com>,
Thomas Gleixner <tglx@...utronix.de>,
Peter Zijlstra <peterz@...radead.org>,
Will Deacon <will@...nel.org>,
Catalin Marinas <catalin.marinas@....com>,
Marco Elver <elver@...gle.com>,
Dmitry Vyukov <dvyukov@...gle.com>,
"Paul E. McKenney" <paulmck@...nel.org>,
Shuah Khan <skhan@...uxfoundation.org>,
Gabriele Paoloni <gpaoloni@...hat.com>,
Juri Lelli <juri.lelli@...hat.com>,
Clark Williams <williams@...hat.com>,
linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-trace-devel@...r.kernel.org
Subject: Re: [PATCH V4 17/20] watchdog/dev: Add tracepoints
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@...ux-watchdog.org>
>>>> Cc: Guenter Roeck <linux@...ck-us.net>
>>>> Cc: Jonathan Corbet <corbet@....net>
>>>> Cc: Steven Rostedt <rostedt@...dmis.org>
>>>> Cc: Ingo Molnar <mingo@...hat.com>
>>>> Cc: Thomas Gleixner <tglx@...utronix.de>
>>>> Cc: Peter Zijlstra <peterz@...radead.org>
>>>> Cc: Will Deacon <will@...nel.org>
>>>> Cc: Catalin Marinas <catalin.marinas@....com>
>>>> Cc: Marco Elver <elver@...gle.com>
>>>> Cc: Dmitry Vyukov <dvyukov@...gle.com>
>>>> Cc: "Paul E. McKenney" <paulmck@...nel.org>
>>>> Cc: Shuah Khan <skhan@...uxfoundation.org>
>>>> Cc: Gabriele Paoloni <gpaoloni@...hat.com>
>>>> Cc: Juri Lelli <juri.lelli@...hat.com>
>>>> Cc: Clark Williams <williams@...hat.com>
>>>> Cc: linux-doc@...r.kernel.org
>>>> Cc: linux-kernel@...r.kernel.org
>>>> Cc: linux-trace-devel@...r.kernel.org
>>>> Signed-off-by: Daniel Bristot de Oliveira <bristot@...nel.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
Powered by blists - more mailing lists