Linux Watchdog driver development
 help / color / mirror / Atom feed
From: "Janusz Użycki" <j.uzycki@elproma.com.pl>
To: linux-watchdog@vger.kernel.org
Cc: Wim Van Sebroeck <wim@iguana.be>
Subject: watchdog: WatchDog Timer Driver Core: ping a hardware watchdog in kernel's space
Date: Thu, 04 Sep 2014 17:47:34 +0200	[thread overview]
Message-ID: <54088996.4040500@elproma.com.pl> (raw)
In-Reply-To: <S1751381AbaIDOwq/20140904145246Z+988@vger.kernel.org>

[-- Attachment #1: Type: text/plain, Size: 1260 bytes --]

Hi,

Some applications require to start watchdog before userspace software. 
This patch enables such feature. Only WATCHDOG_KERNEL_PING flag is 
necessary to enable it (attached example for stmp3xxx_rtc_wdt.c). 
Moreover kernel's ping is re-enabled when userspace software closed 
watchdog using the magic character. The features improves kernel's 
reliable if hardware watchdog is available.
* Can you comment the proposed patch?
* Shoud dynamic or static timer_list be used (small structure...)?
* I also added wdd->ops->ref/unref calls but I'm afraid that even 
original code is buggy in watchdog_dev.c. Is any driver that uses the 
pointers? In my opinion watchdog_open() should call wdd->ops->ref() 
before watchdog_start() and watchdog_release() should call  
wdd->ops->unref() before module_put(). Otherwise fault is possible if 
watchdog module is unloaded.
* I noticed that current watchdog core does not support suspend/resume 
case. If you want to use suspend without the patch you need to close a 
watchdog in userspace using the magic character before suspend command. 
With the patch you must to use WDIOC_SETOPTIONS ioctl and 
WDIOS_DISABLECARD/WDIOS_ENABLECARD. Is there any other method to suspend 
with watchdog?

best regards
Janusz Uzycki


[-- Attachment #2: wdt_kernel_ping.diff --]
[-- Type: text/plain, Size: 4939 bytes --]

Signed-off-by: Janusz Uzycki <j.uzycki@elproma.com.pl>

diff --git a/linux-3.14.17/drivers/watchdog/stmp3xxx_rtc_wdt.c b/linux-3.14.17/drivers/watchdog/stmp3xxx_rtc_wdt.c
index b4d6b34..52c5f1c 100644
--- a/linux-3.14.17/drivers/watchdog/stmp3xxx_rtc_wdt.c
+++ b/linux-3.14.17/drivers/watchdog/stmp3xxx_rtc_wdt.c
@@ -78,6 +78,7 @@ static int stmp3xxx_wdt_probe(struct platform_device *pdev)
 
 	stmp3xxx_wdd.timeout = clamp_t(unsigned, heartbeat, 1, STMP3XXX_MAX_TIMEOUT);
 
+	stmp3xxx_wdd.status |= WATCHDOG_KERNEL_PING;
 	ret = watchdog_register_device(&stmp3xxx_wdd);
 	if (ret < 0) {
 		dev_err(&pdev->dev, "cannot register watchdog device\n");
diff --git a/linux-3.14.17/drivers/watchdog/watchdog_dev.c b/linux-3.14.17/drivers/watchdog/watchdog_dev.c
index 6aaefba..d4fae2c 100644
--- a/linux-3.14.17/drivers/watchdog/watchdog_dev.c
+++ b/linux-3.14.17/drivers/watchdog/watchdog_dev.c
@@ -41,6 +41,8 @@
 #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/slab.h>		/* for kernel ping timer (dynamic) */
+#include <linux/jiffies.h>	/* for kernel ping timer */
 
 #include "watchdog_core.h"
 
@@ -277,6 +279,34 @@ out_ioctl:
 	return err;
 }
 
+/* kernel ping feature */
+static void watchdog_kping_timer_cb(unsigned long data)
+{
+	struct watchdog_device *wdd = (struct watchdog_device *)data;
+	if (test_bit(WDOG_KERNEL_PING, &wdd->status) && !test_bit(WDOG_DEV_OPEN, &wdd->status)) {
+		watchdog_ping(wdd);
+		/* call next ping a second or 0.5s before hardware timeout */
+		mod_timer(wdd->kping_timer, wdd->timeout > 1 ?
+				jiffies + (wdd->timeout - 1) * HZ : jiffies + HZ/2);
+	}
+}
+
+static void watchdog_kping_start(struct watchdog_device *wdd)
+{
+	if (wdd->ops->ref)
+		wdd->ops->ref(wdd);
+	watchdog_start(wdd);
+	watchdog_ping(wdd);
+	mod_timer(wdd->kping_timer, jiffies + HZ/2);
+}
+
+static void watchdog_kping_stop(struct watchdog_device *wdd)
+{
+	del_timer_sync(wdd->kping_timer);
+	if (wdd->ops->unref)
+		wdd->ops->unref(wdd);
+}
+
 /*
  *	watchdog_write: writes to the watchdog.
  *	@file: file from VFS
@@ -430,6 +460,9 @@ static int watchdog_open(struct inode *inode, struct file *file)
 	if (!try_module_get(wdd->ops->owner))
 		goto out;
 
+	if (test_bit(WDOG_KERNEL_PING, &wdd->status))
+		watchdog_kping_stop(wdd);
+
 	err = watchdog_start(wdd);
 	if (err < 0)
 		goto out_mod;
@@ -472,8 +505,13 @@ static int watchdog_release(struct inode *inode, struct file *file)
 	if (!test_bit(WDOG_ACTIVE, &wdd->status))
 		err = 0;
 	else if (test_and_clear_bit(WDOG_ALLOW_RELEASE, &wdd->status) ||
-		 !(wdd->info->options & WDIOF_MAGICCLOSE))
-		err = watchdog_stop(wdd);
+		 !(wdd->info->options & WDIOF_MAGICCLOSE)) {
+		if (test_bit(WDOG_KERNEL_PING, &wdd->status)) {
+			watchdog_kping_start(wdd);
+			err = 0;
+		} else
+			err = watchdog_stop(wdd);
+	}
 
 	/* If the watchdog was not stopped, send a keepalive ping */
 	if (err < 0) {
@@ -524,6 +562,16 @@ int watchdog_dev_register(struct watchdog_device *watchdog)
 {
 	int err, devno;
 
+	if (test_bit(WDOG_KERNEL_PING, &watchdog->status)) {
+		if (!try_module_get(watchdog->ops->owner))
+			return -ENODEV;
+		watchdog->kping_timer = kzalloc(sizeof(*watchdog->kping_timer), GFP_KERNEL);
+		if (!watchdog->kping_timer)
+			return -ENOMEM;
+		setup_timer(watchdog->kping_timer, watchdog_kping_timer_cb, (unsigned long)watchdog);
+		watchdog_kping_start(watchdog);
+	}
+
 	if (watchdog->id == 0) {
 		old_wdd = watchdog;
 		watchdog_miscdev.parent = watchdog->parent;
@@ -575,6 +623,15 @@ int watchdog_dev_unregister(struct watchdog_device *watchdog)
 		misc_deregister(&watchdog_miscdev);
 		old_wdd = NULL;
 	}
+
+	if (test_bit(WDOG_KERNEL_PING, &watchdog->status) || watchdog->kping_timer) {
+		watchdog_kping_stop(watchdog);
+		watchdog_stop(watchdog);
+		kfree(watchdog->kping_timer);
+		watchdog->kping_timer = NULL;
+		watchdog_stop(watchdog);
+		module_put(watchdog->ops->owner);
+	}
 	return 0;
 }
 
diff --git a/linux-3.14.17/include/linux/watchdog.h b/linux-3.14.17/include/linux/watchdog.h
index 2a3038e..8cf9148 100644
--- a/linux-3.14.17/include/linux/watchdog.h
+++ b/linux-3.14.17/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 kernel ping timer */
 #include <uapi/linux/watchdog.h>
 
 struct watchdog_ops;
@@ -95,6 +96,9 @@ 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_KERNEL_PING	5	/* Ping is done by kernel if device not opened by userland */
+	struct timer_list *kping_timer;	/* kernel ping timer */
+#define WATCHDOG_KERNEL_PING	(1 << WDOG_KERNEL_PING)
 };
 
 #ifdef CONFIG_WATCHDOG_NOWAYOUT

       reply	other threads:[~2014-09-04 15:54 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <S1751381AbaIDOwq/20140904145246Z+988@vger.kernel.org>
2014-09-04 15:47 ` Janusz Użycki [this message]
2014-09-07 17:18   ` watchdog: WatchDog Timer Driver Core: ping a hardware watchdog in kernel's space Guenter Roeck
2014-09-08  1:14     ` watchdog: watchdog_dev: WATCHDOG_KEEP_ON feature Janusz Użycki
2014-09-08  1:18       ` Janusz Użycki
2014-09-08  3:24         ` Guenter Roeck
2014-09-08  3:16       ` Guenter Roeck
2014-09-08 12:14         ` Janusz Użycki
2014-09-10 17:24           ` Janusz Użycki
2014-09-11 10:47             ` Janusz Użycki
2014-09-17 11:09         ` Janusz Użycki
2014-09-18 11:07           ` watchdog's pm support preffered implementation Janusz Użycki
2014-09-18 21:40             ` Janusz Użycki
2014-09-18 22:02               ` Janusz Użycki
2014-09-19  3:11                 ` Guenter Roeck
2014-09-19  9:46                   ` Janusz Użycki
2014-09-19 11:23                     ` timers vs drivers suspend race Janusz Użycki
2014-09-19 13:44                       ` Janusz Użycki
2014-09-19 16:21                     ` watchdog's pm support preffered implementation Guenter Roeck
2014-09-23 12:07                       ` Janusz Użycki

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=54088996.4040500@elproma.com.pl \
    --to=j.uzycki@elproma.com.pl \
    --cc=linux-watchdog@vger.kernel.org \
    --cc=wim@iguana.be \
    /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