From 3178d23ddd1c0b850106deee0a0deed2285b89ee Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Mon, 6 Sep 2021 09:28:39 +0200 Subject: [PATCH 01/12] main: use timestamp_is_set() in become_shutdown() --- src/core/main.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/core/main.c b/src/core/main.c index 0914f92a2e..1ddbedd078 100644 --- a/src/core/main.c +++ b/src/core/main.c @@ -1534,13 +1534,11 @@ static int become_shutdown( else if (streq(shutdown_verb, "kexec")) watchdog_timer = arg_kexec_watchdog; - if (watchdog_timer > 0 && watchdog_timer != USEC_INFINITY) { - + if (timestamp_is_set(watchdog_timer)) { char *e; - /* If we reboot or kexec let's set the shutdown - * watchdog and tell the shutdown binary to - * repeatedly ping it */ + /* If we reboot or kexec let's set the shutdown watchdog and + * tell the shutdown binary to repeatedly ping it */ r = watchdog_set_timeout(&watchdog_timer); watchdog_close(r < 0); @@ -1554,9 +1552,8 @@ static int become_shutdown( } else watchdog_close(true); - /* Avoid the creation of new processes forked by the - * kernel; at this point, we will not listen to the - * signals anyway */ + /* Avoid the creation of new processes forked by the kernel; at this + * point, we will not listen to the signals anyway */ if (detect_container() <= 0) (void) cg_uninstall_release_agent(SYSTEMD_CGROUP_CONTROLLER); From 12663295ca7fbff951c6f9747daecadc593f4c93 Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Mon, 6 Sep 2021 11:06:35 +0200 Subject: [PATCH 02/12] core: watchdog_runtimeout_wait() already returns USEC_INFINITY when the watchdog is disabled or closed --- src/core/manager.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/core/manager.c b/src/core/manager.c index b767b0ee6c..7d3602c71c 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -2978,7 +2978,7 @@ int manager_loop(Manager *m) { return log_error_errno(r, "Failed to enable SIGCHLD event source: %m"); while (m->objective == MANAGER_OK) { - usec_t wait_usec, watchdog_usec; + usec_t watchdog_usec; watchdog_usec = manager_get_watchdog(m, WATCHDOG_RUNTIME); if (m->runtime_watchdog_running) @@ -3020,12 +3020,7 @@ int manager_loop(Manager *m) { continue; /* Sleep for watchdog runtime wait time */ - if (timestamp_is_set(watchdog_usec)) - wait_usec = watchdog_runtime_wait(); - else - wait_usec = USEC_INFINITY; - - r = sd_event_run(m->event, wait_usec); + r = sd_event_run(m->event, watchdog_runtime_wait()); if (r < 0) return log_error_errno(r, "Failed to run event loop: %m"); } From 5bbf2db1e4eeabf2a36e28cf3aced0e87c4367d1 Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Mon, 6 Sep 2021 08:47:08 +0200 Subject: [PATCH 03/12] watchdog: make watchdog_ping() a NOP when the watchdog is disabled or closed This patch allows watchdog_ping() to be used unconditionally regardless of whether watchdog_set_timeout() or watchdog_close() has been previously called or not and in both cases watchdog_ping() does nothing. shutdown.c has been updated to cope with this change. --- src/shared/watchdog.c | 7 +++++++ src/shutdown/shutdown.c | 20 +++++++++++++++----- 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/src/shared/watchdog.c b/src/shared/watchdog.c index 361d5837dc..e0700b6013 100644 --- a/src/shared/watchdog.c +++ b/src/shared/watchdog.c @@ -136,6 +136,9 @@ int watchdog_ping(void) { usec_t ntime; int r; + if (!timestamp_is_set(watchdog_timeout)) + return 0; + ntime = now(clock_boottime_or_monotonic()); /* Never ping earlier than watchdog_timeout/4 and try to ping @@ -186,4 +189,8 @@ 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_set_timeout() to open the device again. */ + watchdog_timeout = USEC_INFINITY; } diff --git a/src/shutdown/shutdown.c b/src/shutdown/shutdown.c index 2ba1a21dae..a2ba96bf7c 100644 --- a/src/shutdown/shutdown.c +++ b/src/shutdown/shutdown.c @@ -308,9 +308,9 @@ static void bump_sysctl_printk_log_level(int min_level) { } int main(int argc, char *argv[]) { - bool need_umount, need_swapoff, need_loop_detach, need_dm_detach, need_md_detach, in_container, use_watchdog = false, can_initrd; + bool need_umount, need_swapoff, need_loop_detach, need_dm_detach, need_md_detach, in_container, can_initrd; _cleanup_free_ char *cgroup = NULL; - char *arguments[3], *watchdog_device; + char *arguments[3], *watchdog_device, *watchdog_usec; int cmd, r, umount_log_level = LOG_INFO; static const char* const dirs[] = {SYSTEM_SHUTDOWN_PATH, NULL}; @@ -370,7 +370,6 @@ int main(int argc, char *argv[]) { LOG_TARGET_KMSG)) bump_sysctl_printk_log_level(LOG_WARNING); - use_watchdog = getenv("WATCHDOG_USEC"); watchdog_device = getenv("WATCHDOG_DEVICE"); if (watchdog_device) { r = watchdog_set_device(watchdog_device); @@ -379,6 +378,18 @@ int main(int argc, char *argv[]) { watchdog_device); } + watchdog_usec = getenv("WATCHDOG_USEC"); + if (watchdog_usec) { + usec_t usec; + + r = safe_atou64(watchdog_usec, &usec); + if (r < 0) + log_warning_errno(r, "Failed to parse watchdog timeout '%s', ignoring: %m", + watchdog_usec); + else + (void) watchdog_set_timeout(&usec); + } + /* Lock us into memory */ (void) mlockall(MCL_CURRENT|MCL_FUTURE); @@ -409,8 +420,7 @@ int main(int argc, char *argv[]) { for (;;) { bool changed = false; - if (use_watchdog) - (void) watchdog_ping(); + (void) watchdog_ping(); /* Let's trim the cgroup tree on each iteration so that we leave an empty cgroup tree around, so that From ae4a0ec45c97024387f150305ada14c9117116f4 Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Mon, 6 Sep 2021 08:26:30 +0200 Subject: [PATCH 04/12] core: call watchdog_ping() unconditionally This basically reverts commit 61927b9f116bf45bfdbf19dc2981d4a4f527ae5f and relies on the fact that watchdog_ping() will open and setup the watchdog for us in case the device appears later on. Also unlike what is said in comment https://github.com/systemd/systemd/pull/17460#pullrequestreview-517434377, both m->watchdog[] and m->overriden_watchdog[] are not supposed to store the actual timeout used by the watchdog device but stores the value defined by the user. If the HW timeout value is really needed by the manager then it's probably better to read it via an helper defined in watchdog.c instead. However the HW timeout value is currently only needed by the watchdog code itself mainly when it calculates the time for the next ping. --- src/core/manager.c | 46 +++++++--------------------------------------- src/core/manager.h | 3 --- 2 files changed, 7 insertions(+), 42 deletions(-) diff --git a/src/core/manager.c b/src/core/manager.c index 7d3602c71c..2b8270f146 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -2978,13 +2978,8 @@ int manager_loop(Manager *m) { return log_error_errno(r, "Failed to enable SIGCHLD event source: %m"); while (m->objective == MANAGER_OK) { - usec_t watchdog_usec; - watchdog_usec = manager_get_watchdog(m, WATCHDOG_RUNTIME); - if (m->runtime_watchdog_running) - (void) watchdog_ping(); - else if (timestamp_is_set(watchdog_usec)) - manager_retry_runtime_watchdog(m); + (void) watchdog_ping(); if (!ratelimit_below(&rl)) { /* Yay, something is going seriously wrong, pause a little */ @@ -3198,7 +3193,6 @@ usec_t manager_get_watchdog(Manager *m, WatchdogType t) { } void manager_set_watchdog(Manager *m, WatchdogType t, usec_t timeout) { - int r = 0; assert(m); @@ -3210,22 +3204,16 @@ void manager_set_watchdog(Manager *m, WatchdogType t, usec_t timeout) { if (t == WATCHDOG_RUNTIME) if (!timestamp_is_set(m->watchdog_overridden[WATCHDOG_RUNTIME])) { - if (timestamp_is_set(timeout)) { - r = watchdog_set_timeout(&timeout); - - if (r >= 0) - m->runtime_watchdog_running = true; - } else { + if (timestamp_is_set(timeout)) + (void) watchdog_set_timeout(&timeout); + else watchdog_close(true); - m->runtime_watchdog_running = false; - } } m->watchdog[t] = timeout; } int manager_override_watchdog(Manager *m, WatchdogType t, usec_t timeout) { - int r = 0; assert(m); @@ -3239,36 +3227,16 @@ int manager_override_watchdog(Manager *m, WatchdogType t, usec_t timeout) { usec_t *p; p = timestamp_is_set(timeout) ? &timeout : &m->watchdog[t]; - if (timestamp_is_set(*p)) { - r = watchdog_set_timeout(p); - - if (r >= 0) - m->runtime_watchdog_running = true; - } else { + if (timestamp_is_set(*p)) + (void) watchdog_set_timeout(p); + else watchdog_close(true); - m->runtime_watchdog_running = false; - } } m->watchdog_overridden[t] = timeout; - return 0; } -void manager_retry_runtime_watchdog(Manager *m) { - int r = 0; - - assert(m); - - if (timestamp_is_set(m->watchdog_overridden[WATCHDOG_RUNTIME])) - r = watchdog_set_timeout(&m->watchdog_overridden[WATCHDOG_RUNTIME]); - else - r = watchdog_set_timeout(&m->watchdog[WATCHDOG_RUNTIME]); - - if (r >= 0) - m->runtime_watchdog_running = true; -} - int manager_reload(Manager *m) { _cleanup_(manager_reloading_stopp) Manager *reloading = NULL; _cleanup_fdset_free_ FDSet *fds = NULL; diff --git a/src/core/manager.h b/src/core/manager.h index 1220c9fb16..4f1c6fec63 100644 --- a/src/core/manager.h +++ b/src/core/manager.h @@ -248,8 +248,6 @@ struct Manager { usec_t watchdog[_WATCHDOG_TYPE_MAX]; usec_t watchdog_overridden[_WATCHDOG_TYPE_MAX]; - bool runtime_watchdog_running; /* Whether the runtime HW watchdog was started, so we know if we still need to get the real timeout from the hardware */ - dual_timestamp timestamps[_MANAGER_TIMESTAMP_MAX]; /* Data specific to the device subsystem */ @@ -566,7 +564,6 @@ ManagerTimestamp manager_timestamp_initrd_mangle(ManagerTimestamp s); usec_t manager_get_watchdog(Manager *m, WatchdogType t); void manager_set_watchdog(Manager *m, WatchdogType t, usec_t timeout); int manager_override_watchdog(Manager *m, WatchdogType t, usec_t timeout); -void manager_retry_runtime_watchdog(Manager *m); const char* oom_policy_to_string(OOMPolicy i) _const_; OOMPolicy oom_policy_from_string(const char *s) _pure_; From 2628b98f0ca511a0d454940399aeaae156d16eeb Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Mon, 6 Sep 2021 12:12:06 +0200 Subject: [PATCH 05/12] core: watchdog_set_timeout() doesn't need to return the timeout value used by the HW The manager currently doesn't need it and if it does in the future an helper should probably be introduced instead. --- src/core/main.c | 2 +- src/core/manager.c | 9 ++++----- src/shared/watchdog.c | 19 +++++++++---------- src/shared/watchdog.h | 2 +- src/shutdown/shutdown.c | 2 +- src/test/test-watchdog.c | 2 +- 6 files changed, 17 insertions(+), 19 deletions(-) diff --git a/src/core/main.c b/src/core/main.c index 1ddbedd078..d049591645 100644 --- a/src/core/main.c +++ b/src/core/main.c @@ -1539,7 +1539,7 @@ static int become_shutdown( /* If we reboot or kexec let's set the shutdown watchdog and * tell the shutdown binary to repeatedly ping it */ - r = watchdog_set_timeout(&watchdog_timer); + r = watchdog_set_timeout(watchdog_timer); watchdog_close(r < 0); /* Tell the binary how often to ping, ignore failure */ diff --git a/src/core/manager.c b/src/core/manager.c index 2b8270f146..b82628be51 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -3205,7 +3205,7 @@ void manager_set_watchdog(Manager *m, WatchdogType t, usec_t timeout) { if (t == WATCHDOG_RUNTIME) if (!timestamp_is_set(m->watchdog_overridden[WATCHDOG_RUNTIME])) { if (timestamp_is_set(timeout)) - (void) watchdog_set_timeout(&timeout); + (void) watchdog_set_timeout(timeout); else watchdog_close(true); } @@ -3224,11 +3224,10 @@ int manager_override_watchdog(Manager *m, WatchdogType t, usec_t timeout) { return 0; if (t == WATCHDOG_RUNTIME) { - usec_t *p; + usec_t usec = timestamp_is_set(timeout) ? timeout : m->watchdog[t]; - p = timestamp_is_set(timeout) ? &timeout : &m->watchdog[t]; - if (timestamp_is_set(*p)) - (void) watchdog_set_timeout(p); + if (timestamp_is_set(usec)) + (void) watchdog_set_timeout(usec); else watchdog_close(true); } diff --git a/src/shared/watchdog.c b/src/shared/watchdog.c index e0700b6013..c57dafbca5 100644 --- a/src/shared/watchdog.c +++ b/src/shared/watchdog.c @@ -96,23 +96,22 @@ int watchdog_set_device(char *path) { return r; } -int watchdog_set_timeout(usec_t *usec) { - int r; +int watchdog_set_timeout(usec_t timeout) { - watchdog_timeout = *usec; + /* 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 */ + 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 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) - r = open_watchdog(); - else - r = update_timeout(); + return open_watchdog(); - *usec = watchdog_timeout; - return r; + return update_timeout(); } usec_t watchdog_runtime_wait(void) { diff --git a/src/shared/watchdog.h b/src/shared/watchdog.h index b7587db3ab..08be44fd18 100644 --- a/src/shared/watchdog.h +++ b/src/shared/watchdog.h @@ -7,7 +7,7 @@ #include "util.h" int watchdog_set_device(char *path); -int watchdog_set_timeout(usec_t *usec); +int watchdog_set_timeout(usec_t timeout); int watchdog_ping(void); void watchdog_close(bool disarm); usec_t watchdog_runtime_wait(void); diff --git a/src/shutdown/shutdown.c b/src/shutdown/shutdown.c index a2ba96bf7c..2387e27985 100644 --- a/src/shutdown/shutdown.c +++ b/src/shutdown/shutdown.c @@ -387,7 +387,7 @@ int main(int argc, char *argv[]) { log_warning_errno(r, "Failed to parse watchdog timeout '%s', ignoring: %m", watchdog_usec); else - (void) watchdog_set_timeout(&usec); + (void) watchdog_set_timeout(usec); } /* Lock us into memory */ diff --git a/src/test/test-watchdog.c b/src/test/test-watchdog.c index cbef75fe07..d94ac91d2c 100644 --- a/src/test/test-watchdog.c +++ b/src/test/test-watchdog.c @@ -20,7 +20,7 @@ int main(int argc, char *argv[]) { t = slow ? 10 * USEC_PER_SEC : 1 * USEC_PER_SEC; count = slow ? 5 : 3; - r = watchdog_set_timeout(&t); + r = watchdog_set_timeout(t); if (r < 0) log_warning_errno(r, "Failed to open watchdog: %m"); if (r == -EPERM) From 564096795eb385c8216cb67c0fe894edccc39dde Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Mon, 6 Sep 2021 12:12:45 +0200 Subject: [PATCH 06/12] watchdog: update watchdog_timeout with the closest timeout found by the driver Store the actual timeout value found by the driver in watchdog_timeout since this value is more accurate for calculating the next time for pinging the device. --- src/shared/watchdog.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/shared/watchdog.c b/src/shared/watchdog.c index c57dafbca5..c49972c829 100644 --- a/src/shared/watchdog.c +++ b/src/shared/watchdog.c @@ -40,7 +40,12 @@ static int update_timeout(void) { if (ioctl(watchdog_fd, WDIOC_SETTIMEOUT, &sec) < 0) return log_warning_errno(errno, "Failed to set timeout to %is: %m", sec); - log_info("Set hardware watchdog to %s.", FORMAT_TIMESPAN(sec * USEC_PER_SEC, 0)); + /* 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) { From e94f9e720cda332b1c9f209cb579ba5bf7d82c61 Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Mon, 6 Sep 2021 12:42:50 +0200 Subject: [PATCH 07/12] watchdog: no need to ping the device twice in watchdog_ping() if the device has just been opened --- src/shared/watchdog.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/shared/watchdog.c b/src/shared/watchdog.c index c49972c829..98532dbcb4 100644 --- a/src/shared/watchdog.c +++ b/src/shared/watchdog.c @@ -138,11 +138,14 @@ usec_t watchdog_runtime_wait(void) { int watchdog_ping(void) { usec_t ntime; - int r; if (!timestamp_is_set(watchdog_timeout)) return 0; + if (watchdog_fd < 0) + /* open_watchdog() will automatically ping the device for us if necessary */ + return open_watchdog(); + ntime = now(clock_boottime_or_monotonic()); /* Never ping earlier than watchdog_timeout/4 and try to ping @@ -153,12 +156,6 @@ int watchdog_ping(void) { return 0; } - if (watchdog_fd < 0) { - r = open_watchdog(); - if (r < 0) - return r; - } - if (ioctl(watchdog_fd, WDIOC_KEEPALIVE, 0) < 0) return log_warning_errno(errno, "Failed to ping hardware watchdog: %m"); From 9692ead1978d022754f7da4376e0f225eb823fa8 Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Mon, 6 Sep 2021 14:31:30 +0200 Subject: [PATCH 08/12] watchdog: minor simplification of watchdog_runtime_wait() No functional change. --- src/shared/watchdog.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/shared/watchdog.c b/src/shared/watchdog.c index 98532dbcb4..b0b377325c 100644 --- a/src/shared/watchdog.c +++ b/src/shared/watchdog.c @@ -120,20 +120,19 @@ int watchdog_set_timeout(usec_t timeout) { } usec_t watchdog_runtime_wait(void) { - usec_t rtwait, ntime; if (!timestamp_is_set(watchdog_timeout)) return USEC_INFINITY; /* Sleep half the watchdog timeout since the last successful ping at most */ if (timestamp_is_set(watchdog_last_ping)) { - ntime = now(clock_boottime_or_monotonic()); - assert(ntime >= watchdog_last_ping); - rtwait = usec_sub_unsigned(watchdog_last_ping + (watchdog_timeout / 2), ntime); - } else - rtwait = watchdog_timeout / 2; + usec_t ntime = now(clock_boottime_or_monotonic()); - return rtwait; + assert(ntime >= watchdog_last_ping); + return usec_sub_unsigned(watchdog_last_ping + (watchdog_timeout / 2), ntime); + } + + return watchdog_timeout / 2; } int watchdog_ping(void) { From d74965e6fe583562ebaf0767da6325a72d11643a Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Tue, 14 Sep 2021 22:36:14 +0200 Subject: [PATCH 09/12] watchdog: rename watchdog_set_timeout() into watchdog_setup() "watchdog_set_timeout()" was misleading as the function is not just a setter - it must be called for activating the watchdog device. --- src/core/main.c | 2 +- src/core/manager.c | 4 ++-- src/shared/watchdog.c | 4 ++-- src/shared/watchdog.h | 2 +- src/shutdown/shutdown.c | 2 +- src/test/test-watchdog.c | 2 +- 6 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/core/main.c b/src/core/main.c index d049591645..8be00d129e 100644 --- a/src/core/main.c +++ b/src/core/main.c @@ -1539,7 +1539,7 @@ static int become_shutdown( /* If we reboot or kexec let's set the shutdown watchdog and * tell the shutdown binary to repeatedly ping it */ - r = watchdog_set_timeout(watchdog_timer); + r = watchdog_setup(watchdog_timer); watchdog_close(r < 0); /* Tell the binary how often to ping, ignore failure */ diff --git a/src/core/manager.c b/src/core/manager.c index b82628be51..cb07d3c6d5 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -3205,7 +3205,7 @@ void manager_set_watchdog(Manager *m, WatchdogType t, usec_t timeout) { if (t == WATCHDOG_RUNTIME) if (!timestamp_is_set(m->watchdog_overridden[WATCHDOG_RUNTIME])) { if (timestamp_is_set(timeout)) - (void) watchdog_set_timeout(timeout); + (void) watchdog_setup(timeout); else watchdog_close(true); } @@ -3227,7 +3227,7 @@ int manager_override_watchdog(Manager *m, WatchdogType t, usec_t timeout) { usec_t usec = timestamp_is_set(timeout) ? timeout : m->watchdog[t]; if (timestamp_is_set(usec)) - (void) watchdog_set_timeout(usec); + (void) watchdog_setup(usec); else watchdog_close(true); } diff --git a/src/shared/watchdog.c b/src/shared/watchdog.c index b0b377325c..b42d241528 100644 --- a/src/shared/watchdog.c +++ b/src/shared/watchdog.c @@ -101,7 +101,7 @@ int watchdog_set_device(char *path) { return r; } -int watchdog_set_timeout(usec_t timeout) { +int watchdog_setup(usec_t timeout) { /* Initialize the watchdog timeout with the caller value. This value is * going to be updated by update_timeout() with the closest value @@ -191,6 +191,6 @@ 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_set_timeout() to open the device again. */ + * call to watchdog_setup() to open the device again. */ watchdog_timeout = USEC_INFINITY; } diff --git a/src/shared/watchdog.h b/src/shared/watchdog.h index 08be44fd18..794176d310 100644 --- a/src/shared/watchdog.h +++ b/src/shared/watchdog.h @@ -7,7 +7,7 @@ #include "util.h" int watchdog_set_device(char *path); -int watchdog_set_timeout(usec_t timeout); +int watchdog_setup(usec_t timeout); int watchdog_ping(void); void watchdog_close(bool disarm); usec_t watchdog_runtime_wait(void); diff --git a/src/shutdown/shutdown.c b/src/shutdown/shutdown.c index 2387e27985..903977bcb1 100644 --- a/src/shutdown/shutdown.c +++ b/src/shutdown/shutdown.c @@ -387,7 +387,7 @@ int main(int argc, char *argv[]) { log_warning_errno(r, "Failed to parse watchdog timeout '%s', ignoring: %m", watchdog_usec); else - (void) watchdog_set_timeout(usec); + (void) watchdog_setup(usec); } /* Lock us into memory */ diff --git a/src/test/test-watchdog.c b/src/test/test-watchdog.c index d94ac91d2c..074ed1e451 100644 --- a/src/test/test-watchdog.c +++ b/src/test/test-watchdog.c @@ -20,7 +20,7 @@ int main(int argc, char *argv[]) { t = slow ? 10 * USEC_PER_SEC : 1 * USEC_PER_SEC; count = slow ? 5 : 3; - r = watchdog_set_timeout(t); + r = watchdog_setup(t); if (r < 0) log_warning_errno(r, "Failed to open watchdog: %m"); if (r == -EPERM) From bcc3a1830ea01a8c4b6cb529e33568e86d69eaf0 Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Wed, 15 Sep 2021 09:14:17 +0200 Subject: [PATCH 10/12] watchdog: constify watchdog_set_device() parameter --- src/shared/watchdog.c | 2 +- src/shared/watchdog.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/shared/watchdog.c b/src/shared/watchdog.c index b42d241528..53c7189e24 100644 --- a/src/shared/watchdog.c +++ b/src/shared/watchdog.c @@ -88,7 +88,7 @@ static int open_watchdog(void) { return update_timeout(); } -int watchdog_set_device(char *path) { +int watchdog_set_device(const char *path) { int r; r = free_and_strdup(&watchdog_device, path); diff --git a/src/shared/watchdog.h b/src/shared/watchdog.h index 794176d310..6e99bbdf56 100644 --- a/src/shared/watchdog.h +++ b/src/shared/watchdog.h @@ -6,7 +6,7 @@ #include "time-util.h" #include "util.h" -int watchdog_set_device(char *path); +int watchdog_set_device(const char *path); int watchdog_setup(usec_t timeout); int watchdog_ping(void); void watchdog_close(bool disarm); From 484752e26ae48362b5234269315482a81058b471 Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Wed, 15 Sep 2021 09:13:12 +0200 Subject: [PATCH 11/12] shutdown: introduce init_watchdog() No functional change. --- src/shutdown/shutdown.c | 45 +++++++++++++++++++++++------------------ 1 file changed, 25 insertions(+), 20 deletions(-) diff --git a/src/shutdown/shutdown.c b/src/shutdown/shutdown.c index 903977bcb1..7ad9930677 100644 --- a/src/shutdown/shutdown.c +++ b/src/shutdown/shutdown.c @@ -307,10 +307,33 @@ static void bump_sysctl_printk_log_level(int min_level) { log_debug_errno(r, "Failed to bump kernel.printk to %i: %m", min_level + 1); } +static void init_watchdog(void) { + const char *s; + int r; + + s = getenv("WATCHDOG_DEVICE"); + if (s) { + r = watchdog_set_device(s); + if (r < 0) + log_warning_errno(r, "Failed to set watchdog device to %s, ignoring: %m", s); + } + + s = getenv("WATCHDOG_USEC"); + if (s) { + usec_t usec; + + r = safe_atou64(s, &usec); + if (r < 0) + log_warning_errno(r, "Failed to parse watchdog timeout '%s', ignoring: %m", s); + else + (void) watchdog_setup(usec); + } +} + int main(int argc, char *argv[]) { bool need_umount, need_swapoff, need_loop_detach, need_dm_detach, need_md_detach, in_container, can_initrd; _cleanup_free_ char *cgroup = NULL; - char *arguments[3], *watchdog_device, *watchdog_usec; + char *arguments[3]; int cmd, r, umount_log_level = LOG_INFO; static const char* const dirs[] = {SYSTEM_SHUTDOWN_PATH, NULL}; @@ -370,25 +393,7 @@ int main(int argc, char *argv[]) { LOG_TARGET_KMSG)) bump_sysctl_printk_log_level(LOG_WARNING); - watchdog_device = getenv("WATCHDOG_DEVICE"); - if (watchdog_device) { - r = watchdog_set_device(watchdog_device); - if (r < 0) - log_warning_errno(r, "Failed to set watchdog device to %s, ignoring: %m", - watchdog_device); - } - - watchdog_usec = getenv("WATCHDOG_USEC"); - if (watchdog_usec) { - usec_t usec; - - r = safe_atou64(watchdog_usec, &usec); - if (r < 0) - log_warning_errno(r, "Failed to parse watchdog timeout '%s', ignoring: %m", - watchdog_usec); - else - (void) watchdog_setup(usec); - } + init_watchdog(); /* Lock us into memory */ (void) mlockall(MCL_CURRENT|MCL_FUTURE); From 10fd2b1180b7c298cfb593afd5c63ce26977a3cd Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Wed, 15 Sep 2021 11:20:17 +0200 Subject: [PATCH 12/12] watchdog: use MIN() in update_timeout() Also the previous expression was probably wrong as "(int) t >= INT_MAX" is likely to always evaluate to false. --- src/shared/watchdog.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/shared/watchdog.c b/src/shared/watchdog.c index 53c7189e24..be100931f7 100644 --- a/src/shared/watchdog.c +++ b/src/shared/watchdog.c @@ -36,7 +36,7 @@ static int update_timeout(void) { usec_t t; t = DIV_ROUND_UP(watchdog_timeout, USEC_PER_SEC); - sec = (int) t >= INT_MAX ? INT_MAX : t; /* Saturate */ + 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: %m", sec);