Ping, anyone?

As the original author of the watchdog interfaces, I have no objections to this change, but I have not reviewed the diff.

Ping, anyone?

I seem to have watchdogs only in old 32 bit hardware, so I am unfortunately unable to test this patch.

So if I'm reading this correctly, there are a couple of changes here that aren't quite described in the commit log:

1. In the existing interface, the "pat" operation (both ioctl and in-kernel) accepts a mask of a control operation and a timeout value. In the new interface, the pat interface only accepts the timeout and a new control operation handles things like resetting the watchdog. I actually think just splitting that change out into its own commit (i.e. adding the new function and ioctl) earlier in the series might be nicer (if it is indeed separable).

2. Do we need to keep the old ioctls around for userspace to still use? If not, I'd rather not have them in the header at all, and not add "_SBT" suffixes. That is, I'd rather do

#define WDIOCPATPAT	_IOW('W', 42, sbintime_t)	/* pat the watchdog */

in the header, and only in the kernel code under a suitable #ifdef:

#ifdef COMPAT_FREEBSD14
#define WDIOCPATPAT_14 _IOW('W', 42, u_int)
...
#endif

The _SBT prefix won't age well, so I'd rather keep the names. It will also let us ensure any existing userspace software is fixed for 15.0. In terms of ioctl numbers, I do agree with phk@ that we have lots of numbers, so might as well err on the side of caution and pick new values.

In D47312#1110694, @jhb wrote:

So if I'm reading this correctly, there are a couple of changes here that aren't quite described in the commit log:

1. In the existing interface, the "pat" operation (both ioctl and in-kernel) accepts a mask of a control operation and a timeout value. In the new interface, the pat interface only accepts the timeout and a new control operation handles things like resetting the watchdog. I actually think just splitting that change out into its own commit (i.e. adding the new function and ioctl) earlier in the series might be nicer (if it is indeed separable).

Good idea. I think it is easily separable, since the control is effectively just the control bits of the pat ioctl, separated out.

2. Do we need to keep the old ioctls around for userspace to still use? If not, I'd rather not have them in the header at all, and not add "_SBT" suffixes. That is, I'd rather do

#define WDIOCPATPAT	_IOW('W', 42, sbintime_t)	/* pat the watchdog */

in the header, and only in the kernel code under a suitable #ifdef:

#ifdef COMPAT_FREEBSD14
#define WDIOCPATPAT_14 _IOW('W', 42, u_int)
...
#endif

The _SBT prefix won't age well, so I'd rather keep the names. It will also let us ensure any existing userspace software is fixed for 15.0. In terms of ioctl numbers, I do agree with phk@ that we have lots of numbers, so might as well err on the side of caution and pick new values.

I thought about this, and I'm glad you brought it up. I erred on the side of maintaining source compatibility, in case other watchdog daemons exist in the wild. However, if we're not guaranteeing source compatibility, only binary compatibility, then I'm content with migrating over, and dropping the _SBT. I will, at the least, change the ioctl numbers.

In D47312#1110676, @phk wrote:

I seem to have watchdogs only in old 32 bit hardware, so I am unfortunately unable to test this patch.

I don't quite understand this. sbintime_t is always 64 bits. I think just regression testing would be sufficient on any hardware (showing that it doesn't break), which should be fine with any hardware.

thanks for doing this!

Honestly, I'd just suggest making new ioctls, and it's up to you if you want to keep backwards compatibility.

To the extent I have looked at this, I'm fine with it.

I only noticed Alfreds "pre-timeout" stuff now. That is a pretty crude way to do it, but it does have the advantage of not dragging in a lot of generality (name space for watchdogs etc.) for the only common case for having multiple different timeouts in our application space. I wonder if anybody ever uses it.