[RFC] [PATCH 6/10] Generic Watchdog Timer Driver

[Wim Van Sebroeck]

commit afe3de93859443b575407f39a2e655956c02e088
Author: Wim Van Sebroeck <wim@xxxxxxxxx>
Date:   Sun Jul 18 10:39:00 2010 +0000

    watchdog: WatchDog Timer Driver Core - Part 6
    
    Since we don't want that a driver module can be removed
    while the watchdog timer is still active, we lock the
    driver module (by incremeting the reference counter).
    After a clean close of the watchdog driver we unlock the
    driver module.
    If /dev/watchdog is closed uncleanly (and the watchdog is
    still active) then we do not unlock the driver module,
    but keep it, so that next time /dev/watchdog get's opened
    we can continue triggering the watchdog.
    
    Signed-off-by: Alan Cox <alan@xxxxxxxxxxxxxxxxxxx>
    Signed-off-by: Wim Van Sebroeck <wim@xxxxxxxxx>

diff --git a/Documentation/watchdog/src/watchdog-with-timer-example.c b/Documentation/watchdog/src/watchdog-with-timer-example.c
index f1d4f217..604fd91 100644
--- a/Documentation/watchdog/src/watchdog-with-timer-example.c
+++ b/Documentation/watchdog/src/watchdog-with-timer-example.c
@@ -127,6 +127,7 @@ static const struct watchdog_info wdt_info = {
 };
 
 static const struct watchdog_ops wdt_ops = {
+	.owner =	THIS_MODULE,
 	.start =	wdt_start,
 	.stop =		wdt_stop,
 	.ping =		wdt_ping,
diff --git a/Documentation/watchdog/watchdog-kernel-api.txt b/Documentation/watchdog/watchdog-kernel-api.txt
index f15e8d4..472bfea 100644
--- a/Documentation/watchdog/watchdog-kernel-api.txt
+++ b/Documentation/watchdog/watchdog-kernel-api.txt
@@ -63,6 +63,7 @@ It contains following fields:
 The list of watchdog operations is defined as:
 
 struct watchdog_ops {
+	struct module *owner;
 	/* mandatory operations */
 	int (*start)(struct watchdog_device *);
 	int (*stop)(struct watchdog_device *);
@@ -72,6 +73,10 @@ struct watchdog_ops {
 	int (*set_timeout)(struct watchdog_device *, int);
 };
 
+It is important that you first define the module owner of the watchdog timer
+driver's operations. This module owner will be used to lock the module when
+the watchdog is active. (You don't want to unload the module when you're
+watchdog timer device is still active).
 Some operations are mandatory and some are optional. The mandatory operations
 are:
 * start: this is a pointer to the routine that starts the watchdog timer
@@ -121,3 +126,8 @@ bit-operations. The status bit's that are defined are:
 * WDOG_DEV_OPEN: this status bit shows whether or not the watchdog device
   was opened via /dev/watchdog.
   (This bit should only be used by the WatchDog Timer Driver Core).
+* WDOG_ORPHAN: when /dev/watchdog was closed, but the watchdog is still active
+  then we don't unload the module. This bit is set when this situation occurs.
+  When re-opening /dev/watchdog before a reboot occurs, you can then still use
+  and ping the watchdog timer device.
+  (This bit should only be used by the WatchDog Timer Driver Core).
diff --git a/drivers/watchdog/core/watchdog_core.c b/drivers/watchdog/core/watchdog_core.c
index 52bc520..d1a824e 100644
--- a/drivers/watchdog/core/watchdog_core.c
+++ b/drivers/watchdog/core/watchdog_core.c
@@ -60,6 +60,10 @@ int register_watchdogdevice(struct watchdog_device *wdd)
 	if (wdd == NULL || wdd->info == NULL || wdd->ops == NULL)
 		return -ENODATA;
 
+	/* Make sure that the owner of the watchdog operations exists */
+	if (wdd->ops->owner == NULL)
+		return -ENODATA;
+
 	/* Make sure that the mandatory operations are supported */
 	if (wdd->ops->start == NULL || wdd->ops->stop == NULL)
 		return -ENODATA;
diff --git a/drivers/watchdog/core/watchdog_dev.c b/drivers/watchdog/core/watchdog_dev.c
index b80c6e6..d3dfac2 100644
--- a/drivers/watchdog/core/watchdog_dev.c
+++ b/drivers/watchdog/core/watchdog_dev.c
@@ -259,7 +259,7 @@ static long watchdog_ioctl(struct file *file, unsigned int cmd,
 
 static int watchdog_open(struct inode *inode, struct file *file)
 {
-	int err;
+	int err = -EBUSY;
 
 	trace("%p, %p", inode, file);
 
@@ -267,14 +267,26 @@ static int watchdog_open(struct inode *inode, struct file *file)
 	if (test_and_set_bit(WDOG_DEV_OPEN, &wdd->status))
 		return -EBUSY;
 
+	/* if the /dev/watchdog device is open, we don't want the module
+	 * to be unloaded. */
+	if (!try_module_get(wdd->ops->owner))
+		goto out;
+
 	/* start the watchdog */
 	err = watchdog_start(wdd);
 	if (err < 0)
-		goto out;
+		goto out_mod;
+
+	/* We leaked a reference to lock the module in on close
+	 * now we can reclaim it as we re-opened before triggering */
+	if (test_and_clear_bit(WDOG_ORPHAN, &wdd->status))
+		module_put(wdd->ops->owner);
 
 	/* dev/watchdog is a virtual (and thus non-seekable) filesystem */
 	return nonseekable_open(inode, file);
 
+out_mod:
+	module_put(wdd->ops->owner);
 out:
 	clear_bit(WDOG_DEV_OPEN, &wdd->status);
 	return err;
@@ -296,8 +308,13 @@ static int watchdog_release(struct inode *inode, struct file *file)
 
 	/* stop the watchdog */
 	err = watchdog_stop(wdd);
-	if (err < 0) {
+
+	/* If the watchdog stopped correctly we let the module unload again */
+	if (err == 0)
+		module_put(wdd->ops->owner);
+	else {	/* If not we deliberately leak a module reference */
 		printk(KERN_CRIT "%s: not stopping watchdog!\n", wdd->name);
+		set_bit(WDOG_ORPHAN, &wdd->status);
 		watchdog_ping(wdd);
 	}
 
diff --git a/include/linux/watchdog.h b/include/linux/watchdog.h
index e35f51f..ba08f38 100644
--- a/include/linux/watchdog.h
+++ b/include/linux/watchdog.h
@@ -64,6 +64,7 @@ struct watchdog_device;
 
 /* The watchdog-devices operations */
 struct watchdog_ops {
+	struct module *owner;
 	/* mandatory operations */
 	int (*start)(struct watchdog_device *);
 	int (*stop)(struct watchdog_device *);
@@ -84,6 +85,8 @@ struct watchdog_device {
 #define WDOG_ACTIVE		0	/* is the watchdog running/active */
 #define WDOG_DEV_OPEN		1	/* is the watchdog opened via
 					 * /dev/watchdog */
+#define WDOG_ORPHAN		2	/* is the device module still loaded
+					 * after closing /dev/watchdog */
 };
 
 /* drivers/watchdog/core/watchdog_core.c */
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/