Re: [RFC] watchdog: pretimeout: add a notifier call chain governor

[Guenter Roeck <linux@roeck-us.net>]

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 */
>