On 6/3/20 3:39 AM, Woody Lin wrote:
> Add module parameters 'soft_reboot_target' and 'soft_active_on_boot' for
> customizing softdog configuration; config reboot target by assigning
> soft_reboot_target, and set soft_active_on_boot to start up softdog
> timer at module initialization stage.
>
> Signed-off-by: Woody Lin <woodylin@xxxxxxxxxx>
> ---
> drivers/watchdog/softdog.c | 38 ++++++++++++++++++++++++++++++++++++++
> 1 file changed, 38 insertions(+)
>
> diff --git a/drivers/watchdog/softdog.c b/drivers/watchdog/softdog.c
> index 3e4885c1545e..f5027acddbbb 100644
> --- a/drivers/watchdog/softdog.c
> +++ b/drivers/watchdog/softdog.c
> @@ -20,11 +20,13 @@
> #include <linux/hrtimer.h>
> #include <linux/init.h>
> #include <linux/kernel.h>
> +#include <linux/kthread.h>
> #include <linux/module.h>
> #include <linux/moduleparam.h>
> #include <linux/reboot.h>
> #include <linux/types.h>
> #include <linux/watchdog.h>
> +#include <linux/workqueue.h>
>
> #define TIMER_MARGIN 60 /* Default is 60 seconds */
> static unsigned int soft_margin = TIMER_MARGIN; /* in seconds */
> @@ -49,9 +51,32 @@ module_param(soft_panic, int, 0);
> MODULE_PARM_DESC(soft_panic,
> "Softdog action, set to 1 to panic, 0 to reboot (default=0)");
>
> +static char *soft_reboot_target;
> +module_param(soft_reboot_target, charp, 0000);
> +MODULE_PARM_DESC(soft_reboot_target,
> + "Softdog action, set reboot target (default=emergency)");
> +
> +static bool soft_active_on_boot;
> +module_param(soft_active_on_boot, bool, 0000);
> +MODULE_PARM_DESC(soft_active_on_boot,
> + "Set to true to active Softdog on boot (default=false)");
> +
> static struct hrtimer softdog_ticktock;
> static struct hrtimer softdog_preticktock;
>
> +static int reboot_kthread_fn(void *data)
> +{
> + kernel_restart(soft_reboot_target);
> + emergency_restart(); /* Should not reach here */
> + return -EPERM; /* Should not reach here */
> +}
> +
> +static void reboot_work_fn(struct work_struct *unused)
> +{
> + if (IS_ERR(kthread_run(reboot_kthread_fn, NULL, "softdog_reboot")))
> + emergency_restart();
> +}
> +
> static enum hrtimer_restart softdog_fire(struct hrtimer *timer)
> {
> module_put(THIS_MODULE);
> @@ -62,6 +87,12 @@ static enum hrtimer_restart softdog_fire(struct hrtimer *timer)
> panic("Software Watchdog Timer expired");
> } else {
> pr_crit("Initiating system reboot\n");
> + if (soft_reboot_target != NULL) {
> + static DECLARE_WORK(reboot_work, reboot_work_fn);
> +
> + schedule_work(&reboot_work);
> + return HRTIMER_NORESTART;
> + }
So this adds a double indirection: It first schedules a worker,
then the worker creates a kernel thread, which finally calls
kernel_restart(). If the softdog hangs, it may well be because
of scheduling issues. If so, both the worker and the kernel thread
may never execute, and the system would not reboot even though
the softdog did fire.
Is this double indirection really necessary ?
Thanks,
Guenter
> emergency_restart();
> pr_crit("Reboot didn't ?????\n");
> }
> @@ -145,12 +176,19 @@ static int __init softdog_init(void)
> softdog_preticktock.function = softdog_pretimeout;
> }
>
> + if (soft_active_on_boot) {
> + set_bit(WDOG_HW_RUNNING, &softdog_dev.status);
> + set_bit(WDOG_ACTIVE, &softdog_dev.status);
> + }
> +
> ret = watchdog_register_device(&softdog_dev);
> if (ret)
> return ret;
>
> pr_info("initialized. soft_noboot=%d soft_margin=%d sec soft_panic=%d (nowayout=%d)\n",
> soft_noboot, softdog_dev.timeout, soft_panic, nowayout);
> + pr_info(" soft_reboot_target=%s soft_active_on_boot=%d\n",
> + soft_reboot_target, soft_active_on_boot);
>
> return 0;
> }
>
