[PATCH 1/6] watchdog: watchdog_dev: WATCHDOG_KEEP_ON feature
From: Janusz Użycki <hidden>
Date: 2014-09-30 12:58:15
Also in:
linux-watchdog
I sent also the changelog: [PATCH 1/6] watchdog: watchdog_dev: WATCHDOG_KEEP_ON feature * clean up old comments * watchdog_keepon_start() renamed to watchdog_timer_start() * watchdog_keepon_stop() renamed to watchdog_timer_stop() * watchdog_timer_register() added * watchdog_timer_unregister() added * watchdog_timer_register() is the last action in watchdog_dev_register(). FIXME: Really should be in probe()? * TODO: should watchdog_timer_start() call watchdog_start()? * watchdog_release() uses watchdog_timer_restart() helper function * TODO: change WDOG_KEEP_ON to autostart module parameter Here I am still not sure about module parameters. > autostart: > Set to 0 to disable, -1 to enable with unlimited timeout, > or <n> for an initial timeout of <n> seconds. [1/6]: support of 0 and -1, where -1 is instead of WDOG_KEEP_ON? [2/6]: support of <n> instead of boottime what about: * int init_timeout; /* initial timeout in seconds */ It looks as <n> so why duplicate the autostart parameter? * keep_running It looks like -1 without watchdog_start() * this_watchdog_is_running() Where defined? * watchdog_set_autostart() How to split new patches? Janusz W dniu 2014-09-30 14:46, Janusz Uzycki pisze:
quoted hunk ↗ jump to hunk
Some applications require to start watchdog before userspace software. This patch enables such feature. Only the flag is necessary to enable it. Moreover kernel's ping is re-enabled when userspace software closed watchdog using the magic character. The features improves kernel's reliability if hardware watchdog is available. Signed-off-by: Janusz Uzycki <redacted> --- drivers/watchdog/watchdog_dev.c | 65 ++++++++++++++++++++++++++- include/linux/watchdog.h | 4 ++ 2 files changed, 68 insertions(+), 1 deletion(-)diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c index 6aaefba..afc14f7 100644 --- a/drivers/watchdog/watchdog_dev.c +++ b/drivers/watchdog/watchdog_dev.c@@ -41,6 +41,7 @@ #include <linux/miscdevice.h> /* For handling misc devices */ #include <linux/init.h> /* For __init/__exit/... */ #include <linux/uaccess.h> /* For copy_to_user/put_user/... */ +#include <linux/jiffies.h> /* for ping timer */ #include "watchdog_core.h"@@ -277,6 +278,60 @@ out_ioctl: return err; } +static void watchdog_ping_timer_cb(unsigned long data) +{ + struct watchdog_device *wdd = (struct watchdog_device *)data; + watchdog_ping(wdd); + /* call next ping half the timeout value */ + mod_timer(&wdd->ping_timer, + jiffies + msecs_to_jiffies(wdd->timeout * 500)); +} + +static void watchdog_timer_start(struct watchdog_device *wdd) +{ + /* TODO: watchdog_start(): + * it should probably be handled by the caller, to let us separate + * cases where the watchdog is already running (for example on close). */ + watchdog_start(wdd); + watchdog_ping_timer_cb((unsigned long)wdd); +} + +static void watchdog_timer_stop(struct watchdog_device *wdd) +{ + del_timer_sync(&wdd->ping_timer); +} + +static int watchdog_timer_restart(struct watchdog_device *wdd) +{ + if (test_bit(WDOG_KEEP_ON, &wdd->status)) { + watchdog_timer_start(wdd); + return 0; + } + return watchdog_stop(wdd); +} + +static int watchdog_timer_register(struct watchdog_device *wdd) +{ + if (test_bit(WDOG_KEEP_ON, &wdd->status)) { + if (!try_module_get(wdd->ops->owner)) + return -ENODEV; + setup_timer(&wdd->ping_timer, watchdog_ping_timer_cb, + (unsigned long)wdd); + watchdog_timer_start(wdd); + } + return 0; +} + +static int watchdog_timer_unregister(struct watchdog_device *wdd) +{ + if (test_bit(WDOG_KEEP_ON, &wdd->status)) { + watchdog_timer_stop(wdd); + watchdog_stop(wdd); + module_put(wdd->ops->owner); + } + return 0; +} + /* * watchdog_write: writes to the watchdog. * @file: file from VFS@@ -430,6 +485,9 @@ static int watchdog_open(struct inode *inode, struct file *file) if (!try_module_get(wdd->ops->owner)) goto out; + if (test_bit(WDOG_KEEP_ON, &wdd->status)) + watchdog_timer_stop(wdd); + err = watchdog_start(wdd); if (err < 0) goto out_mod;@@ -473,7 +531,7 @@ static int watchdog_release(struct inode *inode, struct file *file) err = 0; else if (test_and_clear_bit(WDOG_ALLOW_RELEASE, &wdd->status) || !(wdd->info->options & WDIOF_MAGICCLOSE)) - err = watchdog_stop(wdd); + err = watchdog_timer_restart(wdd); /* If the watchdog was not stopped, send a keepalive ping */ if (err < 0) {@@ -553,7 +611,10 @@ int watchdog_dev_register(struct watchdog_device *watchdog) misc_deregister(&watchdog_miscdev); old_wdd = NULL; } + return err; } + + err = watchdog_timer_register(watchdog); return err; }@@ -575,6 +636,8 @@ int watchdog_dev_unregister(struct watchdog_device *watchdog) misc_deregister(&watchdog_miscdev); old_wdd = NULL; } + + watchdog_timer_unregister(watchdog); return 0; }diff --git a/include/linux/watchdog.h b/include/linux/watchdog.h index 2a3038e..0afeabd 100644 --- a/include/linux/watchdog.h +++ b/include/linux/watchdog.h@@ -12,6 +12,7 @@ #include <linux/bitops.h> #include <linux/device.h> #include <linux/cdev.h> +#include <linux/timer.h> /* for ping timer */ #include <uapi/linux/watchdog.h> struct watchdog_ops;@@ -95,6 +96,8 @@ struct watchdog_device { #define WDOG_ALLOW_RELEASE 2 /* Did we receive the magic char ? */ #define WDOG_NO_WAY_OUT 3 /* Is 'nowayout' feature set ? */ #define WDOG_UNREGISTERED 4 /* Has the device been unregistered */ +#define WDOG_KEEP_ON 5 /* Is 'keep on' feature set? */ + struct timer_list ping_timer; /* timer to keep on hardware ping */ }; #ifdef CONFIG_WATCHDOG_NOWAYOUT@@ -104,6 +107,7 @@ struct watchdog_device { #define WATCHDOG_NOWAYOUT 0 #define WATCHDOG_NOWAYOUT_INIT_STATUS 0 #endif +#define WATCHDOG_KEEP_ON (1 << WDOG_KEEP_ON) /* Use the following function to check whether or not the watchdog is active */ static inline bool watchdog_active(struct watchdog_device *wdd)