Date: Sun, 10 Dec 2006 11:04:39 +0100 (CET) From: Nick Hibma <nick@van-laarhoven.org> To: FreeBSD CURRENT Mailing List <current@FreeBSD.ORG> Subject: Slight interface change on the watchdog fido Message-ID: <20061210110419.H42195@localhost>
next in thread | raw e-mail | index | archive | help
I'm planning on committing the following change to make the implementations of
the various hardware watchdogs more consistent. A timeout of 0 passed to the
ioctl is now a valid input and will disable the watchdog. Previously it
produced an error for the Elan chip watchdog, which is _not_ what you want to
see when disarming a watchdog :-) I'd appreciate review of the individual
changes in the files by the following people. The change as a whole was
discussed with phk.
cognet@freebsd.org i80321_wdog.c (*)
des ichwd.c (**)
ambrisko ipmi.c
marius mk48txx.c
phk kern_clock.c, elan-mmcr.c, watchdog.c (**)
(*) The i80321_wdog.c cannot be disarmed. Is this correct?
(**) These have been tested to arm, disarm, and fire. Others have only been
compile tested.
This change has been tested on 6.2-STABLE and 7-CURRENT.
Commit message:
Convert all watchdog event handlers to the following format:
- If the timeout value passed is >0 and acceptable arm the watchdog and set the
*error to 0 (a watchdog is armed).
- Otherwise disarm the watchdog and leave *error as is (valid disarm).
- If the timeout value passed is 0, disarm the watchdog. set *error to an error
if disarm failed.
The default of *error in the ioctl is a failed arm and a succesful disarm.
Also:
- Return an error if WD_PASSIVE is passed in to the ioctl as only
WD_ACTIVE is implemented at the moment. See sys/watchdog.h for an
explanation of the difference between WD_ACTIVE and WD_PASSIVE.
- Remove the I_HAVE_TOTALLY_LOST_MY_SENSE_OF_HUMOR define. If you've lost your
sense of humor, than don't add a define.
i80321_wdog.c
Don't roll your own passive watchdog tickle as this would defeat the
purpose of an active (userland) watchdog tickle.
ichwd.c / ipmi.c:
WD_ACTIVE means userland pats of the watchdog, not whether the
watchdog is active. See sys/watchdog.h.
kern_clock.c:
Remove a check for WD_ACTIVE as this does not make sense here.
This reverts r1.181, as that commit doesn't make sense.
Index: sys/arm/xscale/i80321/i80321_wdog.c
===================================================================
RCS file: /home/ncvs/src/sys/arm/xscale/i80321/i80321_wdog.c,v
retrieving revision 1.2
diff -u -r1.2 i80321_wdog.c
--- sys/arm/xscale/i80321/i80321_wdog.c 15 Jan 2005 18:38:10 -0000 1.2
+++ sys/arm/xscale/i80321/i80321_wdog.c 9 Dec 2006 12:40:40 -0000
@@ -62,7 +62,6 @@
device_t dev;
int armed;
int wdog_period;
- struct callout_handle wdog_callout;
};
static __inline void
@@ -83,8 +82,6 @@
return;
wdtcr_write(WDTCR_ENABLE1);
wdtcr_write(WDTCR_ENABLE2);
- sc->wdog_callout = timeout(iopwdog_tickle, sc,
- hz * (sc->wdog_period - 1));
}
static int
@@ -112,14 +109,19 @@
{
struct iopwdog_softc *sc = private;
- if (cmd == 0)
- return;
- if ((((uint64_t)1 << (cmd & WD_INTERVAL))) >
- (uint64_t)sc->wdog_period * 1000000000)
- return;
- sc->armed = 1;
- iopwdog_tickle(sc);
- *error = 0;
+ cmd &= WD_INTERVAL;
+ if (cmd > 0 && cmd <= 63
+ && (uint64_t)1 << (cmd & WD_INTERVAL) <=
+ (uint64_t)sc->wdog_period * 1000000000) {
+ /* Valid value -> Enable watchdog */
+ iopwdog_tickle(sc);
+ sc->armed = 1;
+ *error = 0;
+ } else {
+ /* XXX Can't disable this watchdog? */
+ if (sc->armed)
+ *error = EOPNOTSUPP;
+ }
}
static int
Index: sys/dev/ichwd/ichwd.c
===================================================================
RCS file: /home/ncvs/src/sys/dev/ichwd/ichwd.c,v
retrieving revision 1.5.2.1
diff -u -r1.5.2.1 ichwd.c
--- sys/dev/ichwd/ichwd.c 15 Jun 2006 15:15:07 -0000 1.5.2.1
+++ sys/dev/ichwd/ichwd.c 9 Dec 2006 12:40:46 -0000
@@ -178,38 +178,20 @@
struct ichwd_softc *sc = arg;
unsigned int timeout;
+ /* convert from power-of-two-ns to WDT ticks */
+ cmd &= WD_INTERVAL;
+ timeout = ((uint64_t)1 << cmd) / ICHWD_TICK;
+ if (cmd > 0 && cmd <= 63
+ && timeout >= ICHWD_MIN_TIMEOUT && timeout <= ICHWD_MAX_TIMEOUT) {
+ if (timeout != sc->timeout)
+ ichwd_tmr_set(sc, timeout);
- /* disable / enable */
- if (!(cmd & WD_ACTIVE)) {
+ ichwd_tmr_reload(sc);
+ *error = 0;
+ } else {
if (sc->active)
ichwd_tmr_disable(sc);
- *error = 0;
- return;
}
- if (!sc->active)
- ichwd_tmr_enable(sc);
-
- cmd &= WD_INTERVAL;
- /* convert from power-of-to-ns to WDT ticks */
- if (cmd >= 64) {
- *error = EINVAL;
- return;
- }
- timeout = ((uint64_t)1 << cmd) / ICHWD_TICK;
- if (timeout < ICHWD_MIN_TIMEOUT || timeout > ICHWD_MAX_TIMEOUT) {
- *error = EINVAL;
- return;
- }
-
- /* set new initial value */
- if (timeout != sc->timeout)
- ichwd_tmr_set(sc, timeout);
-
- /* reload */
- ichwd_tmr_reload(sc);
-
- *error = 0;
- return;
}
static unsigned int pmbase = 0;
Index: sys/dev/ipmi/ipmi.c
===================================================================
RCS file: /home/ncvs/src/sys/dev/ipmi/ipmi.c,v
retrieving revision 1.3.2.3
diff -u -r1.3.2.3 ipmi.c
--- sys/dev/ipmi/ipmi.c 19 Oct 2006 14:50:48 -0000 1.3.2.3
+++ sys/dev/ipmi/ipmi.c 9 Dec 2006 12:40:47 -0000
@@ -649,25 +649,14 @@
struct ipmi_softc *sc = arg;
unsigned int timeout;
- /* disable / enable */
- if (!(cmd & WD_ACTIVE)) {
- ipmi_set_watchdog(sc, 0);
- *error = 0;
- return;
- }
-
cmd &= WD_INTERVAL;
- /* convert from power-of-to-ns to WDT ticks */
- if (cmd >= 64) {
- *error = EINVAL;
- return;
+ if (cmd > 0 && cmd <= 63) {
+ timeout = ((uint64_t)1 << cmd) / 1800000000;
+ ipmi_set_watchdog(sc, timeout);
+ *error = 0;
+ } else {
+ ipmi_set_watchdog(sc, 0);
}
- timeout = ((uint64_t)1 << cmd) / 1800000000;
-
- /* reload */
- ipmi_set_watchdog(sc, timeout);
-
- *error = 0;
}
#ifdef CLONING
Index: sys/dev/watchdog/watchdog.c
===================================================================
RCS file: /home/ncvs/src/sys/dev/watchdog/watchdog.c,v
retrieving revision 1.2
diff -u -r1.2 watchdog.c
--- sys/dev/watchdog/watchdog.c 16 Jun 2004 09:47:02 -0000 1.2
+++ sys/dev/watchdog/watchdog.c 9 Dec 2006 12:40:51 -0000
@@ -55,9 +55,16 @@
return (EINVAL);
if ((u & (WD_ACTIVE | WD_PASSIVE)) == (WD_ACTIVE | WD_PASSIVE))
return (EINVAL);
- if ((u & WD_INTERVAL) == WD_TO_NEVER)
+ if (u & WD_PASSIVE)
+ return (ENOSYS); /* XXX Not implemented yet */
+ if ((u & WD_INTERVAL) == WD_TO_NEVER) {
u = 0;
- error = EOPNOTSUPP;
+ /* Assume all is well; watchdog signals failure. */
+ error = 0;
+ } else {
+ /* Assume no watchdog available; watchdog flags success */
+ error = EOPNOTSUPP;
+ }
EVENTHANDLER_INVOKE(watchdog_list, u, &error);
return (error);
}
Index: sys/i386/i386/elan-mmcr.c
===================================================================
RCS file: /home/ncvs/src/sys/i386/i386/elan-mmcr.c,v
retrieving revision 1.31.2.1
diff -u -r1.31.2.1 elan-mmcr.c
--- sys/i386/i386/elan-mmcr.c 16 Aug 2005 22:47:14 -0000 1.31.2.1
+++ sys/i386/i386/elan-mmcr.c 9 Dec 2006 12:40:53 -0000
@@ -367,11 +367,11 @@
static void
elan_watchdog(void *foo __unused, u_int spec, int *error)
{
- u_int u, v;
+ u_int u, v, w;
static u_int cur;
u = spec & WD_INTERVAL;
- if (spec && u <= 35) {
+ if (spec && u > 0 && u <= 35) {
u = imax(u - 5, 24);
v = 2 << (u - 24);
v |= 0xc000;
@@ -383,7 +383,7 @@
* for other reasons. Save and restore the GP echo mode
* around our hardware tom-foolery.
*/
- u = elan_mmcr->GPECHO;
+ w = elan_mmcr->GPECHO;
elan_mmcr->GPECHO = 0;
if (v != cur) {
/* Clear the ENB bit */
@@ -401,17 +401,17 @@
elan_mmcr->WDTMRCTL = 0xaaaa;
elan_mmcr->WDTMRCTL = 0x5555;
}
- elan_mmcr->GPECHO = u;
+ elan_mmcr->GPECHO = w;
*error = 0;
return;
} else {
- u = elan_mmcr->GPECHO;
+ w = elan_mmcr->GPECHO;
elan_mmcr->GPECHO = 0;
elan_mmcr->WDTMRCTL = 0x3333;
elan_mmcr->WDTMRCTL = 0xcccc;
elan_mmcr->WDTMRCTL = 0x4080;
- elan_mmcr->WDTMRCTL = u;
- elan_mmcr->GPECHO = u;
+ elan_mmcr->WDTMRCTL = w; /* XXX What does this
statement do? */
+ elan_mmcr->GPECHO = w;
cur = 0;
return;
}
Index: sys/kern/kern_clock.c
===================================================================
RCS file: /home/ncvs/src/sys/kern/kern_clock.c,v
retrieving revision 1.178.2.3
diff -u -r1.178.2.3 kern_clock.c
--- sys/kern/kern_clock.c 10 Mar 2006 19:37:33 -0000 1.178.2.3
+++ sys/kern/kern_clock.c 9 Dec 2006 12:40:55 -0000
@@ -541,7 +541,7 @@
u_int u;
u = cmd & WD_INTERVAL;
- if ((cmd & WD_ACTIVE) && u >= WD_TO_1SEC) {
+ if (u >= WD_TO_1SEC) {
watchdog_ticks = (1 << (u - WD_TO_1SEC)) * hz;
watchdog_enabled = 1;
*err = 0;
Index: sys/sys/watchdog.h
===================================================================
RCS file: /home/ncvs/src/sys/sys/watchdog.h,v
retrieving revision 1.3
diff -u -r1.3 watchdog.h
--- sys/sys/watchdog.h 28 Feb 2004 20:06:58 -0000 1.3
+++ sys/sys/watchdog.h 9 Dec 2006 12:41:08 -0000
@@ -30,11 +30,7 @@
#include <sys/ioccom.h>
-#ifdef I_HAVE_TOTALLY_LOST_MY_SENSE_OF_HUMOUR
-#define _PATH_WATCHDOG "watchdog"
-#else
#define _PATH_WATCHDOG "fido"
-#endif
#define WDIOCPATPAT _IOW('W', 42, u_int)
Index: share/man/man4/Makefile
===================================================================
RCS file: /home/ncvs/src/share/man/man4/Makefile,v
retrieving revision 1.320.2.21
diff -u -r1.320.2.21 Makefile
--- share/man/man4/Makefile 23 Nov 2006 09:21:24 -0000 1.320.2.21
+++ share/man/man4/Makefile 9 Dec 2006 12:41:08 -0000
@@ -500,6 +500,7 @@
MLINKS+=wi.4 if_wi.4
MLINKS+=xe.4 if_xe.4
MLINKS+=xl.4 if_xl.4
+MLINKS+=watchdog.4 SW_WATCHDOG.4
.if ${MACHINE_ARCH} == "amd64" || ${MACHINE_ARCH} == "i386"
_amdsmb.4= amdsmb.4
Index: share/man/man4/watchdog.4
===================================================================
RCS file: /home/ncvs/src/share/man/man4/watchdog.4,v
retrieving revision 1.6.8.1
diff -u -r1.6.8.1 watchdog.4
--- share/man/man4/watchdog.4 1 May 2006 19:42:45 -0000 1.6.8.1
+++ share/man/man4/watchdog.4 9 Dec 2006 12:41:08 -0000
@@ -32,54 +32,84 @@
.Nm watchdog
.Nd "hardware and software watchdog"
.Sh SYNOPSIS
-.Cd "options CPU_ELAN"
-.Cd "options CPU_GEODE"
-.Cd "options SW_WATCHDOG"
-.Pp
.In sys/watchdog.h
.Sh DESCRIPTION
The
.Nm
facility is used for controlling hardware and software watchdogs.
.Pp
-The interface is through a device
.Pa /dev/fido
-which responds to a single
+responds to a single
.Xr ioctl 2
call,
.Dv WDIOCPATPAT .
+It takes a single argument which represents a timeout value specified as a
+power of two nanoseconds, or-ed with a flag selecting active or passive
control
+of the watchdog.
.Pp
-The call takes a single argument which represents a timeout value
-specified as an integer power of two nanoseconds.
-.Pp
-The
.Dv WD_ACTIVE
-flag signals that the
+indicates that the
.Nm
-will be kept from
-timing out from userland, for instance by the
+will be kept from timing out from userland, for instance by the
.Xr watchdogd 8
daemon.
-.Pp
-To disable the watchdogs, an argument of zero should be used.
+.Dv WD_PASSIVE
+indicates that the
+.Nm
+will be kept from timing out from the kernel.
.Pp
The
.Xr ioctl 2
call will return success if just one of the available
.Xr watchdog 9
-implementations support the request.
-If the call fails, for instance if none of
+implementations supports setting the timeout to the specified timeout. This
+means that at least one watchdog is armed. If the call fails, for instance if
+none of
.Xr watchdog 9
-implementations support the timeout
-length, all watchdogs are disabled and must be explicitly re-enabled.
+implementations support the timeout length, all watchdogs are disabled and
must
+be explicitly re-enabled.
+.Pp
+To disable the watchdogs pass
+.Dv WD_TO_NEVER .
+If disarming the watchdog(s) failed an error is returned. The watchdog might
+still be armed!
.Sh EXAMPLES
-.\" XXX insert some descriptive text here
.Bd -literal -offset indent
-u_int u = WD_ACTIVE | WD_TO_8SEC;
-int fd = open("/dev/fido", O_RDWR);
+#include <paths.h>
+#include <sys/watchdog.h>
+
+#define WDPATH "/dev/" _PATH_WATCHDOG
+int wdfd = -1;
-ioctl(fd, WDIOCPATPAT, &u);
+static void
+wd_init(void) +{
+ wdfd = open(WDPATH, O_RDWR);
+ if (wdfd == -1)
+ err(1, WDPATH);
+}
+static void
+wd_reset(u_int timeout)
+{
+ if (ioctl(wdfd, WDIOCPATPAT, &timeout) == -1)
+ err(1, "WDIOCPATPAT");
+}
+
+/* in main() */
+wd_init();
+wd_reset(WD_ACTIVE|WD_TO_8SEC);
+/* potential freeze point */
+wd_reset(WD_TO_NEVER);
.Ed
+.Pp
+Enables a watchdog to recover from a potentially freezing piece of code.
+.Pp
+.Bd -literal -offset indent
+options SW_WATCHDOG
+.Ed
+.Pp
+in your kernel config adds a software watchdog in the kernel, dropping to KDB
+or panic-ing when firing.
.Sh SEE ALSO
.Xr watchdogd 8 ,
.Xr watchdog 9
@@ -88,14 +118,17 @@
.Nm
code first appeared in
.Fx 5.1 .
+.Sh BUGS
+The
+.Dv WD_PASSIVE
+option has not yet been implemented.
.Sh AUTHORS
.An -nosplit
The
.Nm
facility was written by
.An Poul-Henning Kamp Aq phk@FreeBSD.org .
-The software watchdog code
-and this manual page were written by
+The software watchdog code and this manual page were written by
.An Sean Kelly Aq smkelly@FreeBSD.org .
Some contributions were made by
.An Jeff Roberson Aq jeff@FreeBSD.org .
Index: watchdog.9
===================================================================
RCS file: /home/ncvs/src/share/man/man9/watchdog.9,v
retrieving revision 1.3
diff -u -r1.3 watchdog.9
--- watchdog.9 6 Jul 2004 07:39:50 -0000 1.3
+++ watchdog.9 9 Dec 2006 13:14:54 -0000
@@ -35,6 +35,7 @@
.Ft void
.Fn watchdog_fn "void *private" "u_int cmd" "int *error"
.Fn EVENTHANDLER_REGISTER watchdog_list watchdog_fn private 0
+.Fn EVENTHANDLER_DEREGISTER watchdog_list eventhandler_tag
.Sh DESCRIPTION
To implement a watchdog in software or hardware, only a single
function needs to be written and registered on the global
@@ -60,7 +61,9 @@
If the watchdog cannot be configured to the proposed timeout, it
must be disabled and the
.Fa error
-argument left untouched.
+argument left untouched. If the watchdog cannot be disabled, the
+.Fa error
+argument must be set to indicate the error.
.Pp
There is no specification of what the watchdog should do when it
times out, but a hardware reset or similar
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20061210110419.H42195>