diff options
| -rw-r--r-- | Documentation/watchdog/watchdog-kernel-api.txt | 28 | ||||
| -rw-r--r-- | drivers/watchdog/watchdog_dev.c | 55 | ||||
| -rw-r--r-- | include/linux/watchdog.h | 5 |
3 files changed, 86 insertions, 2 deletions
diff --git a/Documentation/watchdog/watchdog-kernel-api.txt b/Documentation/watchdog/watchdog-kernel-api.txt index 08d34e11bc5..086638f6c82 100644 --- a/Documentation/watchdog/watchdog-kernel-api.txt +++ b/Documentation/watchdog/watchdog-kernel-api.txt @@ -1,6 +1,6 @@ The Linux WatchDog Timer Driver Core kernel API. =============================================== -Last reviewed: 21-May-2012 +Last reviewed: 22-May-2012 Wim Van Sebroeck <wim@iguana.be> @@ -93,6 +93,8 @@ struct watchdog_ops { unsigned int (*status)(struct watchdog_device *); int (*set_timeout)(struct watchdog_device *, unsigned int); unsigned int (*get_timeleft)(struct watchdog_device *); + void (*ref)(struct watchdog_device *); + void (*unref)(struct watchdog_device *); long (*ioctl)(struct watchdog_device *, unsigned int, unsigned long); }; @@ -100,6 +102,21 @@ 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. (This to avoid a system crash when you unload the module and /dev/watchdog is still open). + +If the watchdog_device struct is dynamically allocated, just locking the module +is not enough and a driver also needs to define the ref and unref operations to +ensure the structure holding the watchdog_device does not go away. + +The simplest (and usually sufficient) implementation of this is to: +1) Add a kref struct to the same structure which is holding the watchdog_device +2) Define a release callback for the kref which frees the struct holding both +3) Call kref_init on this kref *before* calling watchdog_register_device() +4) Define a ref operation calling kref_get on this kref +5) Define a unref operation calling kref_put on this kref +6) When it is time to cleanup: + * Do not kfree() the struct holding both, the last kref_put will do this! + * *After* calling watchdog_unregister_device() call kref_put on the kref + 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 @@ -140,6 +157,10 @@ they are supported. These optional routines/operations are: (Note: the WDIOF_SETTIMEOUT needs to be set in the options field of the watchdog's info structure). * get_timeleft: this routines returns the time that's left before a reset. +* ref: the operation that calls kref_get on the kref of a dynamically + allocated watchdog_device struct. +* unref: the operation that calls kref_put on the kref of a dynamically + allocated watchdog_device struct. * ioctl: if this routine is present then it will be called first before we do our own internal ioctl call handling. This routine should return -ENOIOCTLCMD if a command is not supported. The parameters that are passed to the ioctl @@ -159,6 +180,11 @@ bit-operations. The status bits that are defined are: (This bit should only be used by the WatchDog Timer Driver Core). * WDOG_NO_WAY_OUT: this bit stores the nowayout setting for the watchdog. If this bit is set then the watchdog timer will not be able to stop. +* WDOG_UNREGISTERED: this bit gets set by the WatchDog Timer Driver Core + after calling watchdog_unregister_device, and then checked before calling + any watchdog_ops, so that you can be sure that no operations (other then + unref) will get called after unregister, even if userspace still holds a + reference to /dev/watchdog To set the WDOG_NO_WAY_OUT status bit (before registering your watchdog timer device) you can either: diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c index 4d295d229a0..672d169bf1d 100644 --- a/drivers/watchdog/watchdog_dev.c +++ b/drivers/watchdog/watchdog_dev.c @@ -65,6 +65,11 @@ static int watchdog_ping(struct watchdog_device *wddev) mutex_lock(&wddev->lock); + if (test_bit(WDOG_UNREGISTERED, &wddev->status)) { + err = -ENODEV; + goto out_ping; + } + if (!watchdog_active(wddev)) goto out_ping; @@ -93,6 +98,11 @@ static int watchdog_start(struct watchdog_device *wddev) mutex_lock(&wddev->lock); + if (test_bit(WDOG_UNREGISTERED, &wddev->status)) { + err = -ENODEV; + goto out_start; + } + if (watchdog_active(wddev)) goto out_start; @@ -121,6 +131,11 @@ static int watchdog_stop(struct watchdog_device *wddev) mutex_lock(&wddev->lock); + if (test_bit(WDOG_UNREGISTERED, &wddev->status)) { + err = -ENODEV; + goto out_stop; + } + if (!watchdog_active(wddev)) goto out_stop; @@ -158,8 +173,14 @@ static int watchdog_get_status(struct watchdog_device *wddev, mutex_lock(&wddev->lock); + if (test_bit(WDOG_UNREGISTERED, &wddev->status)) { + err = -ENODEV; + goto out_status; + } + *status = wddev->ops->status(wddev); +out_status: mutex_unlock(&wddev->lock); return err; } @@ -185,8 +206,14 @@ static int watchdog_set_timeout(struct watchdog_device *wddev, mutex_lock(&wddev->lock); + if (test_bit(WDOG_UNREGISTERED, &wddev->status)) { + err = -ENODEV; + goto out_timeout; + } + err = wddev->ops->set_timeout(wddev, timeout); +out_timeout: mutex_unlock(&wddev->lock); return err; } @@ -210,8 +237,14 @@ static int watchdog_get_timeleft(struct watchdog_device *wddev, mutex_lock(&wddev->lock); + if (test_bit(WDOG_UNREGISTERED, &wddev->status)) { + err = -ENODEV; + goto out_timeleft; + } + *timeleft = wddev->ops->get_timeleft(wddev); +out_timeleft: mutex_unlock(&wddev->lock); return err; } @@ -233,8 +266,14 @@ static int watchdog_ioctl_op(struct watchdog_device *wddev, unsigned int cmd, mutex_lock(&wddev->lock); + if (test_bit(WDOG_UNREGISTERED, &wddev->status)) { + err = -ENODEV; + goto out_ioctl; + } + err = wddev->ops->ioctl(wddev, cmd, arg); +out_ioctl: mutex_unlock(&wddev->lock); return err; } @@ -398,6 +437,9 @@ static int watchdog_open(struct inode *inode, struct file *file) file->private_data = wdd; + if (wdd->ops->ref) + wdd->ops->ref(wdd); + /* dev/watchdog is a virtual (and thus non-seekable) filesystem */ return nonseekable_open(inode, file); @@ -434,7 +476,10 @@ static int watchdog_release(struct inode *inode, struct file *file) /* If the watchdog was not stopped, send a keepalive ping */ if (err < 0) { - dev_crit(wdd->dev, "watchdog did not stop!\n"); + mutex_lock(&wdd->lock); + if (!test_bit(WDOG_UNREGISTERED, &wdd->status)) + dev_crit(wdd->dev, "watchdog did not stop!\n"); + mutex_unlock(&wdd->lock); watchdog_ping(wdd); } @@ -444,6 +489,10 @@ static int watchdog_release(struct inode *inode, struct file *file) /* make sure that /dev/watchdog can be re-opened */ clear_bit(WDOG_DEV_OPEN, &wdd->status); + /* Note wdd may be gone after this, do not use after this! */ + if (wdd->ops->unref) + wdd->ops->unref(wdd); + return 0; } @@ -515,6 +564,10 @@ int watchdog_dev_register(struct watchdog_device *watchdog) int watchdog_dev_unregister(struct watchdog_device *watchdog) { + mutex_lock(&watchdog->lock); + set_bit(WDOG_UNREGISTERED, &watchdog->status); + mutex_unlock(&watchdog->lock); + cdev_del(&watchdog->cdev); if (watchdog->id == 0) { misc_deregister(&watchdog_miscdev); diff --git a/include/linux/watchdog.h b/include/linux/watchdog.h index da1dc1b5274..da70f0facd2 100644 --- a/include/linux/watchdog.h +++ b/include/linux/watchdog.h @@ -71,6 +71,8 @@ struct watchdog_device; * @status: The routine that shows the status of the watchdog device. * @set_timeout:The routine for setting the watchdog devices timeout value. * @get_timeleft:The routine that get's the time that's left before a reset. + * @ref: The ref operation for dyn. allocated watchdog_device structs + * @unref: The unref operation for dyn. allocated watchdog_device structs * @ioctl: The routines that handles extra ioctl calls. * * The watchdog_ops structure contains a list of low-level operations @@ -88,6 +90,8 @@ struct watchdog_ops { unsigned int (*status)(struct watchdog_device *); int (*set_timeout)(struct watchdog_device *, unsigned int); unsigned int (*get_timeleft)(struct watchdog_device *); + void (*ref)(struct watchdog_device *); + void (*unref)(struct watchdog_device *); long (*ioctl)(struct watchdog_device *, unsigned int, unsigned long); }; @@ -135,6 +139,7 @@ struct watchdog_device { #define WDOG_DEV_OPEN 1 /* Opened via /dev/watchdog ? */ #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 */ }; #ifdef CONFIG_WATCHDOG_NOWAYOUT |