From f16890f8d2e3994608274b5e46dd847d9ec3ee6a Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Fri, 17 Sep 2021 15:16:38 +0200 Subject: [PATCH] watchdog: passing 0 to watchdog_setup now closes the watchdog Passing 0 meant "disable the watchdog although still kept it opened". However this case didn't seem to be useful especially since PID1 closes the device if it is passed the nul timeout. Hence let's change the meaning of watchdog_setup(0) to match PID1's behavior which allows to simplify the code a bit. Hence this patch also drops enable_watchdog(). --- src/core/main.c | 21 +++++++++------------ src/core/manager.c | 13 +++---------- src/shared/watchdog.c | 29 ++++++++++++++++++++--------- 3 files changed, 32 insertions(+), 31 deletions(-) diff --git a/src/core/main.c b/src/core/main.c index ff7f189370d..8792ef88fc5 100644 --- a/src/core/main.c +++ b/src/core/main.c @@ -1483,9 +1483,9 @@ static int become_shutdown( }; _cleanup_strv_free_ char **env_block = NULL; + usec_t watchdog_timer = 0; size_t pos = 7; int r; - usec_t watchdog_timer = 0; assert(shutdown_verb); assert(!command_line[pos]); @@ -1534,19 +1534,16 @@ static int become_shutdown( else if (streq(shutdown_verb, "kexec")) watchdog_timer = arg_kexec_watchdog; - if (timestamp_is_set(watchdog_timer)) { - /* If we reboot or kexec let's set the shutdown watchdog and tell the shutdown binary to - * repeatedly ping it */ - r = watchdog_setup(watchdog_timer); - watchdog_close(r < 0); + /* If we reboot or kexec let's set the shutdown watchdog and tell the + * shutdown binary to repeatedly ping it */ + r = watchdog_setup(watchdog_timer); + watchdog_close(r < 0); - /* Tell the binary how often to ping, ignore failure */ - (void) strv_extendf(&env_block, "WATCHDOG_USEC="USEC_FMT, watchdog_timer); + /* Tell the binary how often to ping, ignore failure */ + (void) strv_extendf(&env_block, "WATCHDOG_USEC="USEC_FMT, watchdog_timer); - if (arg_watchdog_device) - (void) strv_extendf(&env_block, "WATCHDOG_DEVICE=%s", arg_watchdog_device); - } else - watchdog_close(true); + if (arg_watchdog_device) + (void) strv_extendf(&env_block, "WATCHDOG_DEVICE=%s", arg_watchdog_device); /* Avoid the creation of new processes forked by the kernel; at this * point, we will not listen to the signals anyway */ diff --git a/src/core/manager.c b/src/core/manager.c index 2e7db509d1f..b231e36cbd1 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -3203,12 +3203,8 @@ void manager_set_watchdog(Manager *m, WatchdogType t, usec_t timeout) { return; if (t == WATCHDOG_RUNTIME) - if (!timestamp_is_set(m->watchdog_overridden[WATCHDOG_RUNTIME])) { - if (timestamp_is_set(timeout)) - (void) watchdog_setup(timeout); - else - watchdog_close(true); - } + if (!timestamp_is_set(m->watchdog_overridden[WATCHDOG_RUNTIME])) + (void) watchdog_setup(timeout); m->watchdog[t] = timeout; } @@ -3226,10 +3222,7 @@ int manager_override_watchdog(Manager *m, WatchdogType t, usec_t timeout) { if (t == WATCHDOG_RUNTIME) { usec_t usec = timestamp_is_set(timeout) ? timeout : m->watchdog[t]; - if (timestamp_is_set(usec)) - (void) watchdog_setup(usec); - else - watchdog_close(true); + (void) watchdog_setup(usec); } m->watchdog_overridden[t] = timeout; diff --git a/src/shared/watchdog.c b/src/shared/watchdog.c index 1b5239888f1..b6fddd193f5 100644 --- a/src/shared/watchdog.c +++ b/src/shared/watchdog.c @@ -16,7 +16,7 @@ static int watchdog_fd = -1; static char *watchdog_device; -static usec_t watchdog_timeout; /* USEC_INFINITY → don't change timeout */ +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) { @@ -87,12 +87,11 @@ static int watchdog_ping_now(void) { static int update_timeout(void) { int r; + assert(watchdog_timeout > 0); + if (watchdog_fd < 0) return 0; - if (watchdog_timeout == 0) - return watchdog_set_enable(false); - if (watchdog_timeout != USEC_INFINITY) { r = watchdog_set_timeout(); if (r < 0) { @@ -158,8 +157,19 @@ int watchdog_set_device(const char *path) { int watchdog_setup(usec_t timeout) { + /* 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 && watchdog_timeout == timeout) + if (watchdog_fd >= 0 && (timeout == watchdog_timeout || timeout == USEC_INFINITY)) return 0; /* Initialize the watchdog timeout with the caller value. This value is @@ -213,6 +223,11 @@ int watchdog_ping(void) { } 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; @@ -234,8 +249,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 = 0; } -- 2.47.3