Merge pull request #20787 from fbuihuu/watchdog-more-rework

Watchdog more rework

1 files changed, 128 insertions, 60 deletions

diff --git a/src/shared/watchdog.c b/src/shared/watchdog.c
index 3f6a2f0228..4de6003039 100644
--- a/src/shared/watchdog.c
+++ b/src/shared/watchdog.c
@@ -15,59 +15,114 @@
 #include "watchdog.h"
 
 static int watchdog_fd = -1;
-static char *watchdog_device = NULL;
-static usec_t watchdog_timeout = USEC_INFINITY;
+static char *watchdog_device;
+static usec_t watchdog_timeout; /* 0 → close device and USEC_INFINITY → don't change timeout */
 static usec_t watchdog_last_ping = USEC_INFINITY;
 
+static int watchdog_set_enable(bool enable) {
+        int flags = enable ? WDIOS_ENABLECARD : WDIOS_DISABLECARD;
+        int r;
+
+        assert(watchdog_fd >= 0);
+
+        r = ioctl(watchdog_fd, WDIOC_SETOPTIONS, &flags);
+        if (r < 0) {
+                if (!enable)
+                        return log_warning_errno(errno, "Failed to disable hardware watchdog, ignoring: %m");
+
+                /* ENOTTY means the watchdog is always enabled so we're fine */
+                log_full_errno(ERRNO_IS_NOT_SUPPORTED(errno) ? LOG_DEBUG : LOG_WARNING, errno,
+                               "Failed to enable hardware watchdog, ignoring: %m");
+                if (!ERRNO_IS_NOT_SUPPORTED(errno))
+                        return -errno;
+        }
+
+        return 0;
+}
+
+static int watchdog_get_timeout(void) {
+        int sec = 0;
+
+        assert(watchdog_fd > 0);
+
+        if (ioctl(watchdog_fd, WDIOC_GETTIMEOUT, &sec) < 0)
+                return -errno;
+
+        assert(sec > 0);
+        watchdog_timeout = sec * USEC_PER_SEC;
+
+        return 0;
+}
+
+static int watchdog_set_timeout(void) {
+        usec_t t;
+        int sec;
+
+        assert(watchdog_fd >= 0);
+        assert(timestamp_is_set(watchdog_timeout));
+
+        t = DIV_ROUND_UP(watchdog_timeout, USEC_PER_SEC);
+        sec = MIN(t, (usec_t) INT_MAX); /* Saturate */
+
+        if (ioctl(watchdog_fd, WDIOC_SETTIMEOUT, &sec) < 0)
+                return -errno;
+
+        assert(sec > 0);/*  buggy driver ? */
+        watchdog_timeout = sec * USEC_PER_SEC;
+
+        return 0;
+}
+
+static int watchdog_ping_now(void) {
+        assert(watchdog_fd >= 0);
+
+        if (ioctl(watchdog_fd, WDIOC_KEEPALIVE, 0) < 0)
+                return log_warning_errno(errno, "Failed to ping hardware watchdog, ignoring: %m");
+
+        watchdog_last_ping = now(clock_boottime_or_monotonic());
+
+        return 0;
+}
+
 static int update_timeout(void) {
+        int r;
+
+        assert(watchdog_timeout > 0);
+
         if (watchdog_fd < 0)
                 return 0;
-        if (watchdog_timeout == USEC_INFINITY)
-                return 0;
 
-        if (watchdog_timeout == 0) {
-                int flags;
+        if (watchdog_timeout != USEC_INFINITY) {
+                r = watchdog_set_timeout();
+                if (r < 0) {
+                        if (!ERRNO_IS_NOT_SUPPORTED(r))
+                                return log_error_errno(r, "Failed to set timeout to %s: %m",
+                                                       FORMAT_TIMESPAN(watchdog_timeout, 0));
 
-                flags = WDIOS_DISABLECARD;
-                if (ioctl(watchdog_fd, WDIOC_SETOPTIONS, &flags) < 0)
-                        return log_warning_errno(errno, "Failed to disable hardware watchdog, ignoring: %m");
-        } else {
-                int sec, flags;
-                usec_t t;
-
-                t = DIV_ROUND_UP(watchdog_timeout, USEC_PER_SEC);
-                sec = MIN(t, (usec_t) INT_MAX); /* Saturate */
-                if (ioctl(watchdog_fd, WDIOC_SETTIMEOUT, &sec) < 0)
-                        return log_warning_errno(errno, "Failed to set timeout to %is, ignoring: %m", sec);
-
-                /* Just in case the driver is buggy */
-                assert(sec > 0);
-
-                /* watchdog_timeout stores the actual timeout used by the HW */
-                watchdog_timeout = sec * USEC_PER_SEC;
-                log_info("Set hardware watchdog to %s.", FORMAT_TIMESPAN(watchdog_timeout, 0));
-
-                flags = WDIOS_ENABLECARD;
-                if (ioctl(watchdog_fd, WDIOC_SETOPTIONS, &flags) < 0) {
-                        /* ENOTTY means the watchdog is always enabled so we're fine */
-                        log_full_errno(ERRNO_IS_NOT_SUPPORTED(errno) ? LOG_DEBUG : LOG_WARNING, errno,
-                                       "Failed to enable hardware watchdog, ignoring: %m");
-                        if (!ERRNO_IS_NOT_SUPPORTED(errno))
-                                return -errno;
+                        log_info("Modifying watchdog timeout is not supported, reusing the programmed timeout.");
+                        watchdog_timeout = USEC_INFINITY;
                 }
+        }
 
-                if (ioctl(watchdog_fd, WDIOC_KEEPALIVE, 0) < 0)
-                        return log_warning_errno(errno, "Failed to ping hardware watchdog, ignoring: %m");
-
-                watchdog_last_ping = now(clock_boottime_or_monotonic());
+        if (watchdog_timeout == USEC_INFINITY) {
+                r = watchdog_get_timeout();
+                if (r < 0)
+                        return log_error_errno(errno, "Failed to query watchdog HW timeout: %m");
         }
 
-        return 0;
+        r = watchdog_set_enable(true);
+        if (r < 0)
+                return r;
+
+        log_info("Watchdog running with a timeout of %s.", FORMAT_TIMESPAN(watchdog_timeout, 0));
+
+        return watchdog_ping_now();
 }
 
 static int open_watchdog(void) {
         struct watchdog_info ident;
         const char *fn;
+        int r;
 
         if (watchdog_fd >= 0)
                 return 0;
@@ -85,7 +140,11 @@ static int open_watchdog(void) {
                          ident.firmware_version,
                          fn);
 
-        return update_timeout();
+        r = update_timeout();
+        if (r < 0)
+                watchdog_close(true);
+
+        return r;
 }
 
 int watchdog_set_device(const char *path) {
@@ -102,21 +161,38 @@ int watchdog_set_device(const char *path) {
 }
 
 int watchdog_setup(usec_t timeout) {
+        usec_t previous_timeout;
+        int r;
+
+        /* timeout=0 closes the device whereas passing timeout=USEC_INFINITY
+         * opens it (if needed) without configuring any particular timeout and
+         * thus reuses the programmed value (therefore it's a nop if the device
+         * is already opened).
+         */
+
+        if (timeout == 0) {
+                watchdog_close(true);
+                return 0;
+        }
+
+        /* Let's shortcut duplicated requests */
+        if (watchdog_fd >= 0 && (timeout == watchdog_timeout || timeout == USEC_INFINITY))
+                return 0;
 
         /* Initialize the watchdog timeout with the caller value. This value is
          * going to be updated by update_timeout() with the closest value
          * supported by the driver */
+        previous_timeout = watchdog_timeout;
         watchdog_timeout = timeout;
 
-        /* If we didn't open the watchdog yet and didn't get any explicit
-         * timeout value set, don't do anything */
-        if (watchdog_fd < 0 && watchdog_timeout == USEC_INFINITY)
-                return 0;
-
         if (watchdog_fd < 0)
                 return open_watchdog();
 
-        return update_timeout();
+        r = update_timeout();
+        if (r < 0)
+                watchdog_timeout = previous_timeout;
+
+        return r;
 }
 
 usec_t watchdog_runtime_wait(void) {
@@ -138,7 +214,7 @@ usec_t watchdog_runtime_wait(void) {
 int watchdog_ping(void) {
         usec_t ntime;
 
-        if (!timestamp_is_set(watchdog_timeout))
+        if (watchdog_timeout == 0)
                 return 0;
 
         if (watchdog_fd < 0)
@@ -155,24 +231,20 @@ int watchdog_ping(void) {
                         return 0;
         }
 
-        if (ioctl(watchdog_fd, WDIOC_KEEPALIVE, 0) < 0)
-                return log_warning_errno(errno, "Failed to ping hardware watchdog, ignoring: %m");
-
-        watchdog_last_ping = ntime;
-        return 0;
+        return watchdog_ping_now();
 }
 
 void watchdog_close(bool disarm) {
+
+        /* Once closed, pinging the device becomes a NOP and we request a new
+         * call to watchdog_setup() to open the device again. */
+        watchdog_timeout = 0;
+
         if (watchdog_fd < 0)
                 return;
 
         if (disarm) {
-                int flags;
-
-                /* Explicitly disarm it */
-                flags = WDIOS_DISABLECARD;
-                if (ioctl(watchdog_fd, WDIOC_SETOPTIONS, &flags) < 0)
-                        log_warning_errno(errno, "Failed to disable hardware watchdog, ignoring: %m");
+                (void) watchdog_set_enable(false);
 
                 /* To be sure, use magic close logic, too */
                 for (;;) {
@@ -189,8 +261,4 @@ void watchdog_close(bool disarm) {
         }
 
         watchdog_fd = safe_close(watchdog_fd);
-
-        /* Once closed, pinging the device becomes a NOP and we request a new
-         * call to watchdog_setup() to open the device again. */
-        watchdog_timeout = USEC_INFINITY;
 }