From: Guenter Roeck <linux@roeck-us.net>
To: Woody Lin <woodylin@google.com>,
Wim Van Sebroeck <wim@linux-watchdog.org>
Cc: linux-watchdog@vger.kernel.org
Subject: Re: [PATCH v3] softdog: Add options 'soft_reboot_cmd' and 'soft_active_on_boot'
Date: Wed, 1 Jul 2020 07:10:35 -0700 [thread overview]
Message-ID: <d2ea870d-a9d0-fe8f-c11b-aa54750f1d0d@roeck-us.net> (raw)
In-Reply-To: <20200701110339.2833449-1-woodylin@google.com>
On 7/1/20 4:03 AM, Woody Lin wrote:
> Add module parameters 'soft_reboot_cmd' and 'soft_active_on_boot' for
> customizing softdog configuration; config reboot command by assigning
> soft_reboot_cmd, and set soft_active_on_boot to start up softdog
> timer at module initialization stage.
>
> Signed-off-by: Woody Lin <woodylin@google.com>
Sigh. Now I'll have to look up old versions and compare to figure out
changes from v2. Why do people always believe that kernel maintainers
have endless amounts of time available ?
[ Sorry, yes, I know, we are supposed to remain friendly all the time.
It is just _so_ frustrating to have to deal with this all the time. ]
Guenter
> ---
> drivers/watchdog/softdog.c | 56 ++++++++++++++++++++++++++++++++++++++
> 1 file changed, 56 insertions(+)
>
> diff --git a/drivers/watchdog/softdog.c b/drivers/watchdog/softdog.c
> index 3e4885c1545e..8c8d214b6aa7 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,11 +51,33 @@ 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_cmd;
> +module_param(soft_reboot_cmd, charp, 0000);
> +MODULE_PARM_DESC(soft_reboot_cmd,
> + "Set reboot command. Emergency reboot takes place if unset");
> +
> +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_cmd);
> + return -EPERM; /* Should not reach here */
> +}
> +
> +static void reboot_work_fn(struct work_struct *unused)
> +{
> + kthread_run(reboot_kthread_fn, NULL, "softdog_reboot");
> +}
> +
> static enum hrtimer_restart softdog_fire(struct hrtimer *timer)
> {
> + static bool soft_reboot_fired;
> module_put(THIS_MODULE);
> if (soft_noboot) {
> pr_crit("Triggered - Reboot ignored\n");
> @@ -62,6 +86,33 @@ static enum hrtimer_restart softdog_fire(struct hrtimer *timer)
> panic("Software Watchdog Timer expired");
> } else {
> pr_crit("Initiating system reboot\n");
> + if (!soft_reboot_fired && soft_reboot_cmd != NULL) {
> + static DECLARE_WORK(reboot_work, reboot_work_fn);
> + /*
> + * The 'kernel_restart' is a 'might-sleep' operation.
> + * Also, executing it in system-wide workqueues blocks
> + * any driver from using the same workqueue in its
> + * shutdown callback function. Thus, we should execute
> + * the 'kernel_restart' in a standalone kernel thread.
> + * But since starting a kernel thread is also a
> + * 'might-sleep' operation, so the 'reboot_work' is
> + * required as a launcher of the kernel thread.
> + *
> + * After request the reboot, restart the timer to
> + * schedule an 'emergency_restart' reboot after
> + * 'TIMER_MARGIN' seconds. It's because if the softdog
> + * hangs, it might be because of scheduling issues. And
> + * if that is the case, both 'schedule_work' and
> + * 'kernel_restart' may possibly be malfunctional at the
> + * same time.
> + */
> + soft_reboot_fired = true;
> + schedule_work(&reboot_work);
> + hrtimer_add_expires_ns(timer,
> + (u64)TIMER_MARGIN * NSEC_PER_SEC);
> +
> + return HRTIMER_RESTART;
> + }
> emergency_restart();
> pr_crit("Reboot didn't ?????\n");
> }
> @@ -145,12 +196,17 @@ static int __init softdog_init(void)
> softdog_preticktock.function = softdog_pretimeout;
> }
>
> + if (soft_active_on_boot)
> + softdog_ping(&softdog_dev);
> +
> 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_cmd=%s soft_active_on_boot=%d\n",
> + soft_reboot_cmd, soft_active_on_boot);
>
> return 0;
> }
>
next prev parent reply other threads:[~2020-07-01 14:10 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-01 11:03 [PATCH v3] softdog: Add options 'soft_reboot_cmd' and 'soft_active_on_boot' Woody Lin
2020-07-01 14:10 ` Guenter Roeck [this message]
2020-07-01 14:32 ` Woody Lin
2020-07-02 15:07 ` Guenter Roeck
2020-07-07 4:03 ` Guenter Roeck
2020-07-07 8:46 ` Woody Lin
2020-07-07 11:47 ` Guenter Roeck
2020-07-08 7:17 ` Woody Lin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=d2ea870d-a9d0-fe8f-c11b-aa54750f1d0d@roeck-us.net \
--to=linux@roeck-us.net \
--cc=linux-watchdog@vger.kernel.org \
--cc=wim@linux-watchdog.org \
--cc=woodylin@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).