From f00aff0a0278fbbe883dac9b907fdf5092ff52bc Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 24 Oct 2018 12:35:49 +0200 Subject: [PATCH 01/11] sleep: let's use write_string_file() instead of write_string_stream() if we can Let's shorten things. --- src/sleep/sleep.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/sleep/sleep.c b/src/sleep/sleep.c index 0085cb0196f..4b49d6aaef4 100644 --- a/src/sleep/sleep.c +++ b/src/sleep/sleep.c @@ -197,15 +197,9 @@ static int read_wakealarm(uint64_t *result) { } static int write_wakealarm(const char *str) { - - _cleanup_fclose_ FILE *f = NULL; int r; - f = fopen("/sys/class/rtc/rtc0/wakealarm", "we"); - if (!f) - return log_error_errno(errno, "Failed to open /sys/class/rtc/rtc0/wakealarm: %m"); - - r = write_string_stream(f, str, 0); + r = write_string_file("/sys/class/rtc/rtc0/wakealarm", str, 0); if (r < 0) return log_error_errno(r, "Failed to write '%s' to /sys/class/rtc/rtc0/wakealarm: %m", str); From 58220e6b913400fb2453bb43c4f188cdc40aaecf Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 24 Oct 2018 12:38:04 +0200 Subject: [PATCH 02/11] sleep: don't make up errors, propagate the right ones --- src/sleep/sleep.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/sleep/sleep.c b/src/sleep/sleep.c index 4b49d6aaef4..cab3442185b 100644 --- a/src/sleep/sleep.c +++ b/src/sleep/sleep.c @@ -190,10 +190,13 @@ static int execute(char **modes, char **states) { static int read_wakealarm(uint64_t *result) { _cleanup_free_ char *t = NULL; + int r; - if (read_one_line_file("/sys/class/rtc/rtc0/since_epoch", &t) >= 0) - return safe_atou64(t, result); - return -EBADF; + r = read_one_line_file("/sys/class/rtc/rtc0/since_epoch", &t); + if (r < 0) + return r; + + return safe_atou64(t, result); } static int write_wakealarm(const char *str) { From 6ce63245f6167551f85be9d6582c613e64d18680 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 24 Oct 2018 12:38:13 +0200 Subject: [PATCH 03/11] sleep: log about the correct errors --- src/sleep/sleep.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/sleep/sleep.c b/src/sleep/sleep.c index cab3442185b..a651870641a 100644 --- a/src/sleep/sleep.c +++ b/src/sleep/sleep.c @@ -227,7 +227,7 @@ static int execute_s2h(usec_t hibernate_delay_sec) { r = read_wakealarm(&orig_time); if (r < 0) - return log_error_errno(errno, "Failed to read time: %d", r); + return log_error_errno(r, "Failed to read time: %d", r); orig_time += hibernate_delay_sec / USEC_PER_SEC; xsprintf(time_str, "%" PRIu64, orig_time); @@ -244,7 +244,7 @@ static int execute_s2h(usec_t hibernate_delay_sec) { r = read_wakealarm(&cmp_time); if (r < 0) - return log_error_errno(errno, "Failed to read time: %d", r); + return log_error_errno(r, "Failed to read time: %d", r); /* reset RTC */ r = write_wakealarm("0"); From c16669a3094fc37e8d4fc815fcb6180a094d8ceb Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 24 Oct 2018 12:40:07 +0200 Subject: [PATCH 04/11] =?UTF-8?q?sleep:=20rename=20read=5Fwakealarm()=20?= =?UTF-8?q?=E2=86=92=20rtc=5Fread=5Ftime()?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It doesn't read wakealarm, but the current time of the RTC. Hence, let's rename this to make it less misleading. --- src/sleep/sleep.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/sleep/sleep.c b/src/sleep/sleep.c index a651870641a..f9b822bd93d 100644 --- a/src/sleep/sleep.c +++ b/src/sleep/sleep.c @@ -188,7 +188,7 @@ static int execute(char **modes, char **states) { return r; } -static int read_wakealarm(uint64_t *result) { +static int rtc_read_time(uint64_t *ret_sec) { _cleanup_free_ char *t = NULL; int r; @@ -196,7 +196,7 @@ static int read_wakealarm(uint64_t *result) { if (r < 0) return r; - return safe_atou64(t, result); + return safe_atou64(t, ret_sec); } static int write_wakealarm(const char *str) { @@ -225,7 +225,7 @@ static int execute_s2h(usec_t hibernate_delay_sec) { if (r < 0) return r; - r = read_wakealarm(&orig_time); + r = rtc_read_time(&orig_time); if (r < 0) return log_error_errno(r, "Failed to read time: %d", r); @@ -242,7 +242,7 @@ static int execute_s2h(usec_t hibernate_delay_sec) { if (r < 0) return r; - r = read_wakealarm(&cmp_time); + r = rtc_read_time(&cmp_time); if (r < 0) return log_error_errno(r, "Failed to read time: %d", r); From f780e438fa9552809757b77b949b2a3204b65d9a Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 24 Oct 2018 12:45:06 +0200 Subject: [PATCH 05/11] sleep: rework write_wakealarm() to take a numeric parameter Also, let's rename it to rtc_write_wake_alarm(). Both changes together make sure rtc_write_wake_alarm() and rtc_read_time() are more alike in their naming and semantics. --- src/sleep/sleep.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/sleep/sleep.c b/src/sleep/sleep.c index f9b822bd93d..f3098a69478 100644 --- a/src/sleep/sleep.c +++ b/src/sleep/sleep.c @@ -199,12 +199,15 @@ static int rtc_read_time(uint64_t *ret_sec) { return safe_atou64(t, ret_sec); } -static int write_wakealarm(const char *str) { +static int rtc_write_wake_alarm(uint64_t sec) { + char buf[DECIMAL_STR_MAX(uint64_t)]; int r; - r = write_string_file("/sys/class/rtc/rtc0/wakealarm", str, 0); + xsprintf(buf, "%" PRIu64, sec); + + r = write_string_file("/sys/class/rtc/rtc0/wakealarm", buf, 0); if (r < 0) - return log_error_errno(r, "Failed to write '%s' to /sys/class/rtc/rtc0/wakealarm: %m", str); + return log_error_errno(r, "Failed to write '%s' to /sys/class/rtc/rtc0/wakealarm: %m", buf); return 0; } @@ -213,8 +216,7 @@ static int execute_s2h(usec_t hibernate_delay_sec) { _cleanup_strv_free_ char **hibernate_modes = NULL, **hibernate_states = NULL, **suspend_modes = NULL, **suspend_states = NULL; - usec_t orig_time, cmp_time; - char time_str[DECIMAL_STR_MAX(uint64_t)]; + usec_t original_time, wake_time, cmp_time; int r; r = parse_sleep_config("suspend", NULL, &suspend_modes, &suspend_states, NULL); @@ -225,18 +227,16 @@ static int execute_s2h(usec_t hibernate_delay_sec) { if (r < 0) return r; - r = rtc_read_time(&orig_time); + r = rtc_read_time(&original_time); if (r < 0) return log_error_errno(r, "Failed to read time: %d", r); - orig_time += hibernate_delay_sec / USEC_PER_SEC; - xsprintf(time_str, "%" PRIu64, orig_time); - - r = write_wakealarm(time_str); + wake_time = original_time + (hibernate_delay_sec / USEC_PER_SEC); + r = rtc_write_wake_alarm(wake_time); if (r < 0) return r; - log_debug("Set RTC wake alarm for %s", time_str); + log_debug("Set RTC wake alarm for %" PRIu64, wake_time); r = execute(suspend_modes, suspend_states); if (r < 0) @@ -247,14 +247,14 @@ static int execute_s2h(usec_t hibernate_delay_sec) { return log_error_errno(r, "Failed to read time: %d", r); /* reset RTC */ - r = write_wakealarm("0"); + r = rtc_write_wake_alarm(0); if (r < 0) return r; log_debug("Woke up at %"PRIu64, cmp_time); /* if woken up after alarm time, hibernate */ - if (cmp_time >= orig_time) + if (cmp_time >= wake_time) r = execute(hibernate_modes, hibernate_states); return r; From 033cea5c9d51cc33095432aa69932bc4bbc08354 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 24 Oct 2018 12:47:46 +0200 Subject: [PATCH 06/11] sleep: move log message generation for rtc_read_time() into the function itself The log messages were petty borked anyway, and generated at two separate locations. Let's fix that. --- src/sleep/sleep.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/sleep/sleep.c b/src/sleep/sleep.c index f3098a69478..91b52445e29 100644 --- a/src/sleep/sleep.c +++ b/src/sleep/sleep.c @@ -194,9 +194,13 @@ static int rtc_read_time(uint64_t *ret_sec) { r = read_one_line_file("/sys/class/rtc/rtc0/since_epoch", &t); if (r < 0) - return r; + return log_error_errno(r, "Failed to read RTC time: %m"); - return safe_atou64(t, ret_sec); + r = safe_atou64(t, ret_sec); + if (r < 0) + return log_error_errno(r, "Failed to parse RTC time '%s': %m", t); + + return 0; } static int rtc_write_wake_alarm(uint64_t sec) { @@ -229,7 +233,7 @@ static int execute_s2h(usec_t hibernate_delay_sec) { r = rtc_read_time(&original_time); if (r < 0) - return log_error_errno(r, "Failed to read time: %d", r); + return r; wake_time = original_time + (hibernate_delay_sec / USEC_PER_SEC); r = rtc_write_wake_alarm(wake_time); @@ -244,7 +248,7 @@ static int execute_s2h(usec_t hibernate_delay_sec) { r = rtc_read_time(&cmp_time); if (r < 0) - return log_error_errno(r, "Failed to read time: %d", r); + return r; /* reset RTC */ r = rtc_write_wake_alarm(0); From d029a3a8ca9bdbf6ad6649a379c3d6757dd178ff Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 24 Oct 2018 12:48:56 +0200 Subject: [PATCH 07/11] sleep: round up when calculating RTC sleep time Paranoia: this way we know that when we wake up all timers are definitely equal or ahead of what we expect them to be. --- src/sleep/sleep.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sleep/sleep.c b/src/sleep/sleep.c index 91b52445e29..bd4c104ad07 100644 --- a/src/sleep/sleep.c +++ b/src/sleep/sleep.c @@ -235,7 +235,7 @@ static int execute_s2h(usec_t hibernate_delay_sec) { if (r < 0) return r; - wake_time = original_time + (hibernate_delay_sec / USEC_PER_SEC); + wake_time = original_time + DIV_ROUND_UP(hibernate_delay_sec, USEC_PER_SEC); r = rtc_write_wake_alarm(wake_time); if (r < 0) return r; From eabcf200f7ab271f3f684f380464ad2f8e53fb06 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 24 Oct 2018 12:52:11 +0200 Subject: [PATCH 08/11] sleep: let's turn off the RTC alarm time ASAP Let's be a tiny bit more careful here. Also, let's rearrange things to simplify them a bit, and to not use "r" outside of its immediate scope of validity. --- src/sleep/sleep.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/sleep/sleep.c b/src/sleep/sleep.c index bd4c104ad07..771a8cd48bd 100644 --- a/src/sleep/sleep.c +++ b/src/sleep/sleep.c @@ -246,22 +246,22 @@ static int execute_s2h(usec_t hibernate_delay_sec) { if (r < 0) return r; - r = rtc_read_time(&cmp_time); + /* Reset RTC right-away */ + r = rtc_write_wake_alarm(0); if (r < 0) return r; - /* reset RTC */ - r = rtc_write_wake_alarm(0); + r = rtc_read_time(&cmp_time); if (r < 0) return r; log_debug("Woke up at %"PRIu64, cmp_time); - /* if woken up after alarm time, hibernate */ - if (cmp_time >= wake_time) - r = execute(hibernate_modes, hibernate_states); + if (cmp_time < wake_time) /* We woke up before the alarm time, we are done. */ + return 0; - return r; + /* If woken up after alarm time, hibernate */ + return execute(hibernate_modes, hibernate_states); } static int help(void) { From c695101f47824a6d2142621b439a69f4373f2abf Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 24 Oct 2018 12:57:37 +0200 Subject: [PATCH 09/11] sleep: no need to check for resume_offset twice The W_OK check already checks for existance hence let's remove the F_OK check. --- src/sleep/sleep.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/sleep/sleep.c b/src/sleep/sleep.c index 771a8cd48bd..eeaf8d8972d 100644 --- a/src/sleep/sleep.c +++ b/src/sleep/sleep.c @@ -54,15 +54,14 @@ static int write_hibernate_location_info(void) { } /* Only available in 4.17+ */ - if (access("/sys/power/resume_offset", F_OK) < 0) { - if (errno == ENOENT) + if (access("/sys/power/resume_offset", W_OK) < 0) { + if (errno == ENOENT) { + log_debug("Kernel too old, can't configure resume offset, ignoring."); return 0; + } - return log_debug_errno(errno, "/sys/power/resume_offset unavailable: %m"); - } - - if (access("/sys/power/resume_offset", W_OK) < 0) return log_debug_errno(errno, "/sys/power/resume_offset not writeable: %m"); + } fd = open(device, O_RDONLY | O_CLOEXEC | O_NONBLOCK); if (fd < 0) From 14250f0942b0c1122a35d678e73945d7adf9cfa0 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 24 Oct 2018 13:04:30 +0200 Subject: [PATCH 10/11] sleep: rework what we do if a suspend fails. First of all, let's fix logging: let's simply log the same message as we do on success, so that there's always the same pair of these messages around, regardless if the suspend was successful or not. To distuingish a successful suspend from a failed one, check the ERRNO= field of the structured message. In most ways a failed suspend cycle is not distuingishable from a successful one that took no time, hence let's treat it this way, and always pair the success message with a failure message. This also changes a more important concept: the post-suspend callouts are now called also called on failure, following the same logic: let's always run them in pairs: for every pre callout a post callout has to follow. --- src/sleep/sleep.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/sleep/sleep.c b/src/sleep/sleep.c index eeaf8d8972d..e3b72a768d5 100644 --- a/src/sleep/sleep.c +++ b/src/sleep/sleep.c @@ -174,12 +174,15 @@ static int execute(char **modes, char **states) { r = write_state(&f, states); if (r < 0) - return log_error_errno(r, "Failed to write /sys/power/state: %m"); - - log_struct(LOG_INFO, - "MESSAGE_ID=" SD_MESSAGE_SLEEP_STOP_STR, - LOG_MESSAGE("System resumed."), - "SLEEP=%s", arg_verb); + log_struct_errno(LOG_ERR, r, + "MESSAGE_ID=" SD_MESSAGE_SLEEP_STOP_STR, + LOG_MESSAGE("Failed to suspend system. System resumed again: %m"), + "SLEEP=%s", arg_verb); + else + log_struct(LOG_INFO, + "MESSAGE_ID=" SD_MESSAGE_SLEEP_STOP_STR, + LOG_MESSAGE("System resumed."), + "SLEEP=%s", arg_verb); arguments[1] = (char*) "post"; execute_directories(dirs, DEFAULT_TIMEOUT_USEC, NULL, NULL, arguments, NULL); From f05e1ae66601d4b2c8666c8fa91b46287ae93d86 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 24 Oct 2018 13:15:24 +0200 Subject: [PATCH 11/11] sleep: when we can't hibernate on suspend-then-hibernate, fall back to suspend again Let's make this a bit safer, and try hard to return to sleep, if we can at all. Fixes: #10212 --- src/sleep/sleep.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/sleep/sleep.c b/src/sleep/sleep.c index e3b72a768d5..a4eba598514 100644 --- a/src/sleep/sleep.c +++ b/src/sleep/sleep.c @@ -263,7 +263,17 @@ static int execute_s2h(usec_t hibernate_delay_sec) { return 0; /* If woken up after alarm time, hibernate */ - return execute(hibernate_modes, hibernate_states); + r = execute(hibernate_modes, hibernate_states); + if (r < 0) { + log_notice("Couldn't hibernate, will try to suspend again."); + r = execute(suspend_modes, suspend_states); + if (r < 0) { + log_notice("Could neither hibernate nor suspend again, giving up."); + return r; + } + } + + return 0; } static int help(void) {