* [RFC] watchdog: pretimeout: add a notifier call chain governor
@ 2021-04-13 15:45 Stefan Eichenberger
2021-04-13 16:15 ` Guenter Roeck
0 siblings, 1 reply; 5+ messages in thread
From: Stefan Eichenberger @ 2021-04-13 15:45 UTC (permalink / raw)
To: linux-watchdog; +Cc: Guenter Roeck, Wim Van Sebroeck
Add a governor which calls a notifier call chain. This allows the
registration of custom functions to the watchdog pretimeout. This is
useful when writing out-of-tree kernel modules and it is more similar to
the register_reboot_notifier or register_oom_notifier API.
Signed-off-by: Stefan Eichenberger <eichest@gmail.com>
---
drivers/watchdog/Kconfig | 16 ++++++
drivers/watchdog/Makefile | 1 +
drivers/watchdog/pretimeout_notifier.c | 73 ++++++++++++++++++++++++++
drivers/watchdog/watchdog_pretimeout.h | 2 +
include/linux/watchdog.h | 5 ++
5 files changed, 97 insertions(+)
create mode 100644 drivers/watchdog/pretimeout_notifier.c
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 355100dad60a..ec019120effc 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -105,6 +105,14 @@ config WATCHDOG_PRETIMEOUT_GOV_PANIC
Panic watchdog pretimeout governor, on watchdog pretimeout
event put the kernel into panic.
+config WATCHDOG_PRETIMEOUT_GOV_NOTIFIER
+ tristate "Notifier watchdog pretimeout governor"
+ depends on WATCHDOG_CORE
+ default WATCHDOG_CORE
+ help
+ Notifier watchdog pretimeout governor, on watchdog pretimeout
+ event call a notifier call chain.
+
choice
prompt "Default Watchdog Pretimeout Governor"
default WATCHDOG_PRETIMEOUT_DEFAULT_GOV_PANIC
@@ -129,6 +137,14 @@ config WATCHDOG_PRETIMEOUT_DEFAULT_GOV_PANIC
a watchdog pretimeout event happens, consider that
a watchdog feeder is dead and reboot is unavoidable.
+config WATCHDOG_PRETIMEOUT_DEFAULT_GOV_NOTIFIER
+ bool "notifier"
+ depends on WATCHDOG_PRETIMEOUT_GOV_NOTIFIER
+ help
+ Use notifier watchdog pretimeout governor by default,
+ if a watchdog pretiemout event happens, a notifier
+ call chain is called.
+
endchoice
endif # WATCHDOG_PRETIMEOUT_GOV
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index a7eade8b4d45..cb63f4eb89bf 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -12,6 +12,7 @@ watchdog-$(CONFIG_WATCHDOG_PRETIMEOUT_GOV) += watchdog_pretimeout.o
obj-$(CONFIG_WATCHDOG_PRETIMEOUT_GOV_NOOP) += pretimeout_noop.o
obj-$(CONFIG_WATCHDOG_PRETIMEOUT_GOV_PANIC) += pretimeout_panic.o
+obj-$(CONFIG_WATCHDOG_PRETIMEOUT_GOV_NOTIFIER) += pretimeout_notifier.o
# Only one watchdog can succeed. We probe the ISA/PCI/USB based
# watchdog-cards first, then the architecture specific watchdog
diff --git a/drivers/watchdog/pretimeout_notifier.c b/drivers/watchdog/pretimeout_notifier.c
new file mode 100644
index 000000000000..3f845c77ec44
--- /dev/null
+++ b/drivers/watchdog/pretimeout_notifier.c
@@ -0,0 +1,73 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2021 Stefan Eichenberger <eichest@gmail.com>
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/watchdog.h>
+
+#include "watchdog_pretimeout.h"
+
+BLOCKING_NOTIFIER_HEAD(watchdog_pretimeout_notifier_list);
+
+/**
+ * register_watchdog_pretimeout_notifier - Register function to be called on pretimeout
+ * @nb: Info about notifier function to be called
+ *
+ * Registers a function with the list of functions to be called when a watchdog pretimeout occurs.
+ *
+ * Currently always returns zero, as blocking_notifier_chain_register()
+ * always returns zero.
+ */
+int register_watchdog_pretimeout_notifier(struct notifier_block *nb)
+{
+ return blocking_notifier_chain_register(&watchdog_pretimeout_notifier_list, nb);
+}
+EXPORT_SYMBOL(register_watchdog_pretimeout_notifier);
+
+/**
+ * unregister_watchdog_pretimeout_notifier - Unregister previously registered pretimeout notifier
+ * @nb: Hook to be unregistered
+ *
+ * Unregisters a previously registered watchdog pretimeout notifier function.
+ *
+ * Returns zero on success, or %-ENOENT on failure.
+ */
+int unregister_watchdog_pretimeout_notifier(struct notifier_block *nb)
+{
+ return blocking_notifier_chain_unregister(&watchdog_pretimeout_notifier_list, nb);
+}
+EXPORT_SYMBOL(unregister_watchdog_pretimeout_notifier);
+
+/**
+ * pretimeout_notifier - Notify registred methods on pretimeout
+ * @wdd - watchdog_device
+ *
+ * Notify, watchdog has not been fed till pretimeout event.
+ */
+static void pretimeout_notifier(struct watchdog_device *wdd)
+{
+ blocking_notifier_call_chain(&watchdog_pretimeout_notifier_list, 0, wdd);
+}
+
+static struct watchdog_governor watchdog_gov_notifier = {
+ .name = "notifier",
+ .pretimeout = pretimeout_notifier,
+};
+
+static int __init watchdog_gov_notifier_register(void)
+{
+ return watchdog_register_governor(&watchdog_gov_notifier);
+}
+
+static void __exit watchdog_gov_notifier_unregister(void)
+{
+ watchdog_unregister_governor(&watchdog_gov_notifier);
+}
+module_init(watchdog_gov_notifier_register);
+module_exit(watchdog_gov_notifier_unregister);
+
+MODULE_AUTHOR("Stefan Eichenberger <eichest@gmail.com>");
+MODULE_DESCRIPTION("Notifier watchdog pretimeout governor");
+MODULE_LICENSE("GPL");
diff --git a/drivers/watchdog/watchdog_pretimeout.h b/drivers/watchdog/watchdog_pretimeout.h
index a3f1abc68839..bb00ca1fcadd 100644
--- a/drivers/watchdog/watchdog_pretimeout.h
+++ b/drivers/watchdog/watchdog_pretimeout.h
@@ -28,6 +28,8 @@ int watchdog_pretimeout_governor_set(struct watchdog_device *wdd,
#define WATCHDOG_PRETIMEOUT_DEFAULT_GOV "noop"
#elif IS_ENABLED(CONFIG_WATCHDOG_PRETIMEOUT_DEFAULT_GOV_PANIC)
#define WATCHDOG_PRETIMEOUT_DEFAULT_GOV "panic"
+#elif IS_ENABLED(CONFIG_WATCHDOG_PRETIMEOUT_DEFAULT_GOV_NOTIFIER)
+#define WATCHDOG_PRETIMEOUT_DEFAULT_GOV "notifier"
#endif
#else
diff --git a/include/linux/watchdog.h b/include/linux/watchdog.h
index 9b19e6bb68b5..760beab1007f 100644
--- a/include/linux/watchdog.h
+++ b/include/linux/watchdog.h
@@ -215,4 +215,9 @@ int watchdog_set_last_hw_keepalive(struct watchdog_device *, unsigned int);
/* devres register variant */
int devm_watchdog_register_device(struct device *dev, struct watchdog_device *);
+#if IS_ENABLED(CONFIG_WATCHDOG_PRETIMEOUT_GOV_NOTIFIER)
+int register_watchdog_pretimeout_notifier(struct notifier_block *nb);
+int unregister_watchdog_pretimeout_notifier(struct notifier_block *nb);
+#endif
+
#endif /* ifndef _LINUX_WATCHDOG_H */
--
2.27.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [RFC] watchdog: pretimeout: add a notifier call chain governor
2021-04-13 15:45 [RFC] watchdog: pretimeout: add a notifier call chain governor Stefan Eichenberger
@ 2021-04-13 16:15 ` Guenter Roeck
2021-04-13 16:56 ` Stefan Eichenberger
0 siblings, 1 reply; 5+ messages in thread
From: Guenter Roeck @ 2021-04-13 16:15 UTC (permalink / raw)
To: Stefan Eichenberger, linux-watchdog; +Cc: Wim Van Sebroeck
On 4/13/21 8:45 AM, Stefan Eichenberger wrote:
> Add a governor which calls a notifier call chain. This allows the
> registration of custom functions to the watchdog pretimeout. This is
> useful when writing out-of-tree kernel modules and it is more similar to
> the register_reboot_notifier or register_oom_notifier API.
>
> Signed-off-by: Stefan Eichenberger <eichest@gmail.com>
It seems questionable to implement such a notification as governor.
A governor is configurable, and letting userspace override notifications
seems very odd and unusual. Please provide a use case.
Guenter
> ---
> drivers/watchdog/Kconfig | 16 ++++++
> drivers/watchdog/Makefile | 1 +
> drivers/watchdog/pretimeout_notifier.c | 73 ++++++++++++++++++++++++++
> drivers/watchdog/watchdog_pretimeout.h | 2 +
> include/linux/watchdog.h | 5 ++
> 5 files changed, 97 insertions(+)
> create mode 100644 drivers/watchdog/pretimeout_notifier.c
>
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index 355100dad60a..ec019120effc 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -105,6 +105,14 @@ config WATCHDOG_PRETIMEOUT_GOV_PANIC
> Panic watchdog pretimeout governor, on watchdog pretimeout
> event put the kernel into panic.
>
> +config WATCHDOG_PRETIMEOUT_GOV_NOTIFIER
> + tristate "Notifier watchdog pretimeout governor"
> + depends on WATCHDOG_CORE
> + default WATCHDOG_CORE
> + help
> + Notifier watchdog pretimeout governor, on watchdog pretimeout
> + event call a notifier call chain.
> +
> choice
> prompt "Default Watchdog Pretimeout Governor"
> default WATCHDOG_PRETIMEOUT_DEFAULT_GOV_PANIC
> @@ -129,6 +137,14 @@ config WATCHDOG_PRETIMEOUT_DEFAULT_GOV_PANIC
> a watchdog pretimeout event happens, consider that
> a watchdog feeder is dead and reboot is unavoidable.
>
> +config WATCHDOG_PRETIMEOUT_DEFAULT_GOV_NOTIFIER
> + bool "notifier"
> + depends on WATCHDOG_PRETIMEOUT_GOV_NOTIFIER
> + help
> + Use notifier watchdog pretimeout governor by default,
> + if a watchdog pretiemout event happens, a notifier
> + call chain is called.
> +
> endchoice
>
> endif # WATCHDOG_PRETIMEOUT_GOV
> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> index a7eade8b4d45..cb63f4eb89bf 100644
> --- a/drivers/watchdog/Makefile
> +++ b/drivers/watchdog/Makefile
> @@ -12,6 +12,7 @@ watchdog-$(CONFIG_WATCHDOG_PRETIMEOUT_GOV) += watchdog_pretimeout.o
>
> obj-$(CONFIG_WATCHDOG_PRETIMEOUT_GOV_NOOP) += pretimeout_noop.o
> obj-$(CONFIG_WATCHDOG_PRETIMEOUT_GOV_PANIC) += pretimeout_panic.o
> +obj-$(CONFIG_WATCHDOG_PRETIMEOUT_GOV_NOTIFIER) += pretimeout_notifier.o
>
> # Only one watchdog can succeed. We probe the ISA/PCI/USB based
> # watchdog-cards first, then the architecture specific watchdog
> diff --git a/drivers/watchdog/pretimeout_notifier.c b/drivers/watchdog/pretimeout_notifier.c
> new file mode 100644
> index 000000000000..3f845c77ec44
> --- /dev/null
> +++ b/drivers/watchdog/pretimeout_notifier.c
> @@ -0,0 +1,73 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (C) 2021 Stefan Eichenberger <eichest@gmail.com>
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/watchdog.h>
> +
> +#include "watchdog_pretimeout.h"
> +
> +BLOCKING_NOTIFIER_HEAD(watchdog_pretimeout_notifier_list);
> +
> +/**
> + * register_watchdog_pretimeout_notifier - Register function to be called on pretimeout
> + * @nb: Info about notifier function to be called
> + *
> + * Registers a function with the list of functions to be called when a watchdog pretimeout occurs.
> + *
> + * Currently always returns zero, as blocking_notifier_chain_register()
> + * always returns zero.
> + */
> +int register_watchdog_pretimeout_notifier(struct notifier_block *nb)
> +{
> + return blocking_notifier_chain_register(&watchdog_pretimeout_notifier_list, nb);
> +}
> +EXPORT_SYMBOL(register_watchdog_pretimeout_notifier);
> +
> +/**
> + * unregister_watchdog_pretimeout_notifier - Unregister previously registered pretimeout notifier
> + * @nb: Hook to be unregistered
> + *
> + * Unregisters a previously registered watchdog pretimeout notifier function.
> + *
> + * Returns zero on success, or %-ENOENT on failure.
> + */
> +int unregister_watchdog_pretimeout_notifier(struct notifier_block *nb)
> +{
> + return blocking_notifier_chain_unregister(&watchdog_pretimeout_notifier_list, nb);
> +}
> +EXPORT_SYMBOL(unregister_watchdog_pretimeout_notifier);
> +
> +/**
> + * pretimeout_notifier - Notify registred methods on pretimeout
> + * @wdd - watchdog_device
> + *
> + * Notify, watchdog has not been fed till pretimeout event.
> + */
> +static void pretimeout_notifier(struct watchdog_device *wdd)
> +{
> + blocking_notifier_call_chain(&watchdog_pretimeout_notifier_list, 0, wdd);
> +}
> +
> +static struct watchdog_governor watchdog_gov_notifier = {
> + .name = "notifier",
> + .pretimeout = pretimeout_notifier,
> +};
> +
> +static int __init watchdog_gov_notifier_register(void)
> +{
> + return watchdog_register_governor(&watchdog_gov_notifier);
> +}
> +
> +static void __exit watchdog_gov_notifier_unregister(void)
> +{
> + watchdog_unregister_governor(&watchdog_gov_notifier);
> +}
> +module_init(watchdog_gov_notifier_register);
> +module_exit(watchdog_gov_notifier_unregister);
> +
> +MODULE_AUTHOR("Stefan Eichenberger <eichest@gmail.com>");
> +MODULE_DESCRIPTION("Notifier watchdog pretimeout governor");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/watchdog/watchdog_pretimeout.h b/drivers/watchdog/watchdog_pretimeout.h
> index a3f1abc68839..bb00ca1fcadd 100644
> --- a/drivers/watchdog/watchdog_pretimeout.h
> +++ b/drivers/watchdog/watchdog_pretimeout.h
> @@ -28,6 +28,8 @@ int watchdog_pretimeout_governor_set(struct watchdog_device *wdd,
> #define WATCHDOG_PRETIMEOUT_DEFAULT_GOV "noop"
> #elif IS_ENABLED(CONFIG_WATCHDOG_PRETIMEOUT_DEFAULT_GOV_PANIC)
> #define WATCHDOG_PRETIMEOUT_DEFAULT_GOV "panic"
> +#elif IS_ENABLED(CONFIG_WATCHDOG_PRETIMEOUT_DEFAULT_GOV_NOTIFIER)
> +#define WATCHDOG_PRETIMEOUT_DEFAULT_GOV "notifier"
> #endif
>
> #else
> diff --git a/include/linux/watchdog.h b/include/linux/watchdog.h
> index 9b19e6bb68b5..760beab1007f 100644
> --- a/include/linux/watchdog.h
> +++ b/include/linux/watchdog.h
> @@ -215,4 +215,9 @@ int watchdog_set_last_hw_keepalive(struct watchdog_device *, unsigned int);
> /* devres register variant */
> int devm_watchdog_register_device(struct device *dev, struct watchdog_device *);
>
> +#if IS_ENABLED(CONFIG_WATCHDOG_PRETIMEOUT_GOV_NOTIFIER)
> +int register_watchdog_pretimeout_notifier(struct notifier_block *nb);
> +int unregister_watchdog_pretimeout_notifier(struct notifier_block *nb);
> +#endif
> +
> #endif /* ifndef _LINUX_WATCHDOG_H */
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [RFC] watchdog: pretimeout: add a notifier call chain governor
2021-04-13 16:15 ` Guenter Roeck
@ 2021-04-13 16:56 ` Stefan Eichenberger
2021-04-14 11:50 ` Guenter Roeck
0 siblings, 1 reply; 5+ messages in thread
From: Stefan Eichenberger @ 2021-04-13 16:56 UTC (permalink / raw)
To: Guenter Roeck; +Cc: linux-watchdog, Wim Van Sebroeck
Hi Guenter,
Thanks a lot for the fast feedback!
On Tue, Apr 13, 2021 at 09:15:35AM -0700, Guenter Roeck wrote:
> On 4/13/21 8:45 AM, Stefan Eichenberger wrote:
> > Add a governor which calls a notifier call chain. This allows the
> > registration of custom functions to the watchdog pretimeout. This is
> > useful when writing out-of-tree kernel modules and it is more similar to
> > the register_reboot_notifier or register_oom_notifier API.
> >
> > Signed-off-by: Stefan Eichenberger <eichest@gmail.com>
>
> It seems questionable to implement such a notification as governor.
> A governor is configurable, and letting userspace override notifications
> seems very odd and unusual. Please provide a use case.
What I would like to do is to have an out-of-tree module which will
write a pattern into a reserved memory region when a watchdog occurs.
After a reboot the module then read this reserved memory region and can
differentiate between different reset reasons. Here the example kernel
module:
https://github.com/embear-engineering/sample-kernel-modules/blob/use-watchdog-register/reset-reason/reset-reason.c
Registering to the watchdog happens on line 180.
I think I could just implement a governor in the module but it is not
really flexible because then I would have to compile the kernel with
e.g. the noop governor and then switch to my governor after the module
loaded. Then I thought why not using a notifier chain similar to reboot,
panic, or oom.
Regards,
Stefan
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC] watchdog: pretimeout: add a notifier call chain governor
2021-04-13 16:56 ` Stefan Eichenberger
@ 2021-04-14 11:50 ` Guenter Roeck
2021-04-14 12:42 ` Stefan Eichenberger
0 siblings, 1 reply; 5+ messages in thread
From: Guenter Roeck @ 2021-04-14 11:50 UTC (permalink / raw)
To: Stefan Eichenberger; +Cc: linux-watchdog, Wim Van Sebroeck
On 4/13/21 9:56 AM, Stefan Eichenberger wrote:
> Hi Guenter,
>
> Thanks a lot for the fast feedback!
>
> On Tue, Apr 13, 2021 at 09:15:35AM -0700, Guenter Roeck wrote:
>> On 4/13/21 8:45 AM, Stefan Eichenberger wrote:
>>> Add a governor which calls a notifier call chain. This allows the
>>> registration of custom functions to the watchdog pretimeout. This is
>>> useful when writing out-of-tree kernel modules and it is more similar to
>>> the register_reboot_notifier or register_oom_notifier API.
>>>
>>> Signed-off-by: Stefan Eichenberger <eichest@gmail.com>
>>
>> It seems questionable to implement such a notification as governor.
>> A governor is configurable, and letting userspace override notifications
>> seems very odd and unusual. Please provide a use case.
>
> What I would like to do is to have an out-of-tree module which will
> write a pattern into a reserved memory region when a watchdog occurs.
> After a reboot the module then read this reserved memory region and can
> differentiate between different reset reasons. Here the example kernel
> module:
> https://github.com/embear-engineering/sample-kernel-modules/blob/use-watchdog-register/reset-reason/reset-reason.c
>
> Registering to the watchdog happens on line 180.
>
> I think I could just implement a governor in the module but it is not
> really flexible because then I would have to compile the kernel with
> e.g. the noop governor and then switch to my governor after the module
> loaded. Then I thought why not using a notifier chain similar to reboot,
> panic, or oom.
>
I have two problems with this: First, we do not support in-kernel code purely
for out-of-tree drivers. Second, the idea of using a configurable notifier
(ie one that can be disabled by userspace action) seems odd. If anything,
that would have to be an unconditional notifier. However, again, that would
require a use case in the upstream kernel.
Guenter
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC] watchdog: pretimeout: add a notifier call chain governor
2021-04-14 11:50 ` Guenter Roeck
@ 2021-04-14 12:42 ` Stefan Eichenberger
0 siblings, 0 replies; 5+ messages in thread
From: Stefan Eichenberger @ 2021-04-14 12:42 UTC (permalink / raw)
To: Guenter Roeck; +Cc: linux-watchdog, Wim Van Sebroeck
On Wed, Apr 14, 2021 at 04:50:38AM -0700, Guenter Roeck wrote:
> On 4/13/21 9:56 AM, Stefan Eichenberger wrote:
> > Hi Guenter,
> >
> > Thanks a lot for the fast feedback!
> >
> > On Tue, Apr 13, 2021 at 09:15:35AM -0700, Guenter Roeck wrote:
> >> On 4/13/21 8:45 AM, Stefan Eichenberger wrote:
> >>> Add a governor which calls a notifier call chain. This allows the
> >>> registration of custom functions to the watchdog pretimeout. This is
> >>> useful when writing out-of-tree kernel modules and it is more similar to
> >>> the register_reboot_notifier or register_oom_notifier API.
> >>>
> >>> Signed-off-by: Stefan Eichenberger <eichest@gmail.com>
> >>
> >> It seems questionable to implement such a notification as governor.
> >> A governor is configurable, and letting userspace override notifications
> >> seems very odd and unusual. Please provide a use case.
> >
> > What I would like to do is to have an out-of-tree module which will
> > write a pattern into a reserved memory region when a watchdog occurs.
> > After a reboot the module then read this reserved memory region and can
> > differentiate between different reset reasons. Here the example kernel
> > module:
> > https://github.com/embear-engineering/sample-kernel-modules/blob/use-watchdog-register/reset-reason/reset-reason.c
> >
> > Registering to the watchdog happens on line 180.
> >
> > I think I could just implement a governor in the module but it is not
> > really flexible because then I would have to compile the kernel with
> > e.g. the noop governor and then switch to my governor after the module
> > loaded. Then I thought why not using a notifier chain similar to reboot,
> > panic, or oom.
> >
>
> I have two problems with this: First, we do not support in-kernel code purely
> for out-of-tree drivers. Second, the idea of using a configurable notifier
> (ie one that can be disabled by userspace action) seems odd. If anything,
> that would have to be an unconditional notifier. However, again, that would
> require a use case in the upstream kernel.
I see your point. Thanks for checking anyways.
Regards,
Stefan
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-04-14 12:43 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-13 15:45 [RFC] watchdog: pretimeout: add a notifier call chain governor Stefan Eichenberger
2021-04-13 16:15 ` Guenter Roeck
2021-04-13 16:56 ` Stefan Eichenberger
2021-04-14 11:50 ` Guenter Roeck
2021-04-14 12:42 ` Stefan Eichenberger
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).