From e024cdd270469371013a8f821aea30e5ab5a1065 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Thu, 26 Oct 2023 20:27:12 +0800 Subject: [PATCH 1/7] sleep: introduce sleep_operation_is_hibernation --- src/shared/sleep-config.c | 2 +- src/shared/sleep-config.h | 4 ++++ src/sleep/sleep.c | 2 +- 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/shared/sleep-config.c b/src/shared/sleep-config.c index ab09530355..77b5ee5b84 100644 --- a/src/shared/sleep-config.c +++ b/src/shared/sleep-config.c @@ -305,7 +305,7 @@ static int sleep_supported_internal( return false; } - if (IN_SET(operation, SLEEP_HIBERNATE, SLEEP_HYBRID_SLEEP)) { + if (sleep_operation_is_hibernation(operation)) { r = sleep_mode_supported(sleep_config->modes[operation]); if (r < 0) return r; diff --git a/src/shared/sleep-config.h b/src/shared/sleep-config.h index bc10a5b39b..bc5aeb91bd 100644 --- a/src/shared/sleep-config.h +++ b/src/shared/sleep-config.h @@ -20,6 +20,10 @@ typedef enum SleepOperation { const char* sleep_operation_to_string(SleepOperation s) _const_; SleepOperation sleep_operation_from_string(const char *s) _pure_; +static inline bool sleep_operation_is_hibernation(SleepOperation operation) { + return IN_SET(operation, SLEEP_HIBERNATE, SLEEP_HYBRID_SLEEP); +} + typedef struct SleepConfig { bool allow[_SLEEP_OPERATION_MAX]; diff --git a/src/sleep/sleep.c b/src/sleep/sleep.c index 8bfc9ee6cc..354044c67c 100644 --- a/src/sleep/sleep.c +++ b/src/sleep/sleep.c @@ -284,7 +284,7 @@ static int execute( setvbuf(f, NULL, _IONBF, 0); /* Configure hibernation settings if we are supposed to hibernate */ - if (!strv_isempty(modes)) { + if (sleep_operation_is_hibernation(operation)) { bool resume_set; r = find_suitable_hibernation_device(&hibernation_device); From 37f80890b29ca28a23a1e36ecf03324f4f8d8e47 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Thu, 26 Oct 2023 22:17:12 +0800 Subject: [PATCH 2/7] sleep: drop unneeded includes --- src/sleep/sleep.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/sleep/sleep.c b/src/sleep/sleep.c index 354044c67c..05d6e38ae1 100644 --- a/src/sleep/sleep.c +++ b/src/sleep/sleep.c @@ -7,11 +7,9 @@ #include #include #include -#include #include -#include -#include #include +#include #include #include @@ -22,7 +20,6 @@ #include "battery-capacity.h" #include "battery-util.h" -#include "btrfs-util.h" #include "build.h" #include "bus-error.h" #include "bus-locator.h" @@ -560,6 +557,7 @@ static int help(void) { } static int parse_argv(int argc, char *argv[]) { + enum { ARG_VERSION = 0x100, }; @@ -577,6 +575,7 @@ static int parse_argv(int argc, char *argv[]) { while ((c = getopt_long(argc, argv, "h", options, NULL)) >= 0) switch (c) { + case 'h': return help(); @@ -588,6 +587,7 @@ static int parse_argv(int argc, char *argv[]) { default: assert_not_reached(); + } if (argc - optind != 1) @@ -644,6 +644,7 @@ static int run(int argc, char *argv[]) { default: r = execute(sleep_config, arg_operation, NULL); break; + } return r; From 4706c3ec2ba5059db13545153aaff4b948ad516a Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Thu, 26 Oct 2023 22:18:57 +0800 Subject: [PATCH 3/7] sleep: log about errno --- 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 05d6e38ae1..cea57d8565 100644 --- a/src/sleep/sleep.c +++ b/src/sleep/sleep.c @@ -634,7 +634,7 @@ static int run(int argc, char *argv[]) { * asked us to do both: suspend + hibernate, and it's almost certainly the * hibernation that failed, hence still do the other thing, the suspend. */ - log_notice("Couldn't hybrid sleep, will try to suspend instead."); + log_notice_errno(r, "Couldn't hybrid sleep, will try to suspend instead: %m"); r = execute(sleep_config, SLEEP_SUSPEND, "suspend-after-failed-hybrid-sleep"); } From 0374cbd668697a0c832cabe0da5a1a3d4e44575b Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Thu, 26 Oct 2023 22:20:36 +0800 Subject: [PATCH 4/7] sleep: update help text for suspend-then-hibernate --- src/sleep/sleep.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/sleep/sleep.c b/src/sleep/sleep.c index cea57d8565..c5ec63e4f7 100644 --- a/src/sleep/sleep.c +++ b/src/sleep/sleep.c @@ -548,7 +548,8 @@ static int help(void) { " hibernate Hibernate the system\n" " hybrid-sleep Both hibernate and suspend the system\n" " suspend-then-hibernate Initially suspend and then hibernate\n" - " the system after a fixed period of time\n" + " the system after a fixed period of time or\n" + " when battery is low\n" "\nSee the %s for details.\n", program_invocation_short_name, link); From 78c21009bfcf0758f9c85dc70ac896c8aab6b535 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Thu, 26 Oct 2023 23:22:11 +0800 Subject: [PATCH 5/7] sleep: rework write_state and write_mode --- src/sleep/sleep.c | 76 +++++++++++++++++++++++------------------------ 1 file changed, 37 insertions(+), 39 deletions(-) diff --git a/src/sleep/sleep.c b/src/sleep/sleep.c index c5ec63e4f7..79d7a49b99 100644 --- a/src/sleep/sleep.c +++ b/src/sleep/sleep.c @@ -123,44 +123,45 @@ static int write_efi_hibernate_location(const HibernationDevice *hibernation_dev #endif } -static int write_mode(char **modes) { +static int write_state(int fd, char * const *states) { + int r = 0; + + assert(fd >= 0); + assert(states); + + STRV_FOREACH(state, states) { + _cleanup_fclose_ FILE *f = NULL; + int k; + + k = fdopen_independent(fd, "we", &f); + if (k < 0) + return RET_GATHER(r, k); + + k = write_string_stream(f, *state, WRITE_STRING_FILE_DISABLE_BUFFER); + if (k >= 0) { + log_debug("Using sleep state '%s'.", *state); + return 0; + } + + RET_GATHER(r, log_debug_errno(k, "Failed to write '%s' to /sys/power/state: %m", *state)); + } + + return r; +} + +static int write_mode(char * const *modes) { int r = 0; STRV_FOREACH(mode, modes) { int k; k = write_string_file("/sys/power/disk", *mode, WRITE_STRING_FILE_DISABLE_BUFFER); - if (k >= 0) + if (k >= 0) { + log_debug("Using sleep disk mode '%s'.", *mode); return 0; + } - log_debug_errno(k, "Failed to write '%s' to /sys/power/disk: %m", *mode); - if (r >= 0) - r = k; - } - - return r; -} - -static int write_state(FILE **f, char **states) { - int r = 0; - - assert(f); - assert(*f); - - STRV_FOREACH(state, states) { - int k; - - k = write_string_stream(*f, *state, WRITE_STRING_FILE_DISABLE_BUFFER); - if (k >= 0) - return 0; - log_debug_errno(k, "Failed to write '%s' to /sys/power/state: %m", *state); - if (r >= 0) - r = k; - - fclose(*f); - *f = fopen("/sys/power/state", "we"); - if (!*f) - return -errno; + RET_GATHER(r, log_debug_errno(k, "Failed to write '%s' to /sys/power/disk: %m", *mode)); } return r; @@ -255,7 +256,7 @@ static int execute( }; _cleanup_(hibernation_device_done) HibernationDevice hibernation_device = {}; - _cleanup_fclose_ FILE *f = NULL; + _cleanup_close_ int state_fd = -EBADF; char **modes, **states; int r; @@ -272,13 +273,10 @@ static int execute( "No sleep states configured for sleep operation %s, can't sleep.", sleep_operation_to_string(operation)); - /* This file is opened first, so that if we hit an error, - * we can abort before modifying any state. */ - f = fopen("/sys/power/state", "we"); - if (!f) - return log_error_errno(errno, "Failed to open /sys/power/state: %m"); - - setvbuf(f, NULL, _IONBF, 0); + /* This file is opened first, so that if we hit an error, we can abort before modifying any state. */ + state_fd = open("/sys/power/state", O_WRONLY|O_CLOEXEC); + if (state_fd < 0) + return -errno; /* Configure hibernation settings if we are supposed to hibernate */ if (sleep_operation_is_hibernation(operation)) { @@ -327,7 +325,7 @@ static int execute( LOG_MESSAGE("Entering sleep state '%s'...", sleep_operation_to_string(operation)), "SLEEP=%s", sleep_operation_to_string(arg_operation)); - r = write_state(&f, states); + r = write_state(state_fd, states); if (r < 0) log_struct_errno(LOG_ERR, r, "MESSAGE_ID=" SD_MESSAGE_SLEEP_STOP_STR, From 122f6f1eaa4447449c7054793f6497eb9e4d03c6 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Thu, 26 Oct 2023 23:50:50 +0800 Subject: [PATCH 6/7] sleep: minor modernization for lock_all_homes --- 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 79d7a49b99..8dd14e5546 100644 --- a/src/sleep/sleep.c +++ b/src/sleep/sleep.c @@ -213,9 +213,9 @@ static int lock_all_homes(void) { /* Let's synchronously lock all home directories managed by homed that have been marked for it. This * way the key material required to access these volumes is hopefully removed from memory. */ - r = sd_bus_open_system(&bus); + r = bus_connect_system_systemd(&bus); if (r < 0) - return log_warning_errno(r, "Failed to connect to system bus, ignoring: %m"); + return log_error_errno(r, "Failed to connect to system bus: %m"); r = bus_message_new_method_call(bus, &m, bus_home_mgr, "LockAllHomes"); if (r < 0) From cc1c8d129f9b89fa033fdd907e704f93a98ab8e0 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Thu, 26 Oct 2023 23:39:32 +0800 Subject: [PATCH 7/7] sleep: make sure we clear HibernateLocation on all error paths Also, let's say "sleep operation" rather than "sleep state", the latter of which creates ambiguity with /sys/power/state. --- src/sleep/sleep.c | 127 +++++++++++++++++++++++----------------------- 1 file changed, 64 insertions(+), 63 deletions(-) diff --git a/src/sleep/sleep.c b/src/sleep/sleep.c index 8dd14e5546..21af3e9e52 100644 --- a/src/sleep/sleep.c +++ b/src/sleep/sleep.c @@ -167,43 +167,6 @@ static int write_mode(char * const *modes) { return r; } -/* Return true if wakeup type is APM timer */ -static int check_wakeup_type(void) { - static const char dmi_object_path[] = "/sys/firmware/dmi/entries/1-0/raw"; - uint8_t wakeup_type_byte, tablesize; - _cleanup_free_ char *buf = NULL; - size_t bufsize; - int r; - - /* implementation via dmi/entries */ - r = read_full_virtual_file(dmi_object_path, &buf, &bufsize); - if (r < 0) - return log_debug_errno(r, "Unable to read %s: %m", dmi_object_path); - if (bufsize < 25) - return log_debug_errno(SYNTHETIC_ERRNO(EINVAL), - "Only read %zu bytes from %s (expected 25)", - bufsize, dmi_object_path); - - /* index 1 stores the size of table */ - tablesize = (uint8_t) buf[1]; - if (tablesize < 25) - return log_debug_errno(SYNTHETIC_ERRNO(EINVAL), - "Table size less than the index[0x18] where waketype byte is available."); - - wakeup_type_byte = (uint8_t) buf[24]; - /* 0 is Reserved and 8 is AC Power Restored. As per table 12 in - * https://www.dmtf.org/sites/default/files/standards/documents/DSP0134_3.4.0.pdf */ - if (wakeup_type_byte >= 128) - return log_debug_errno(SYNTHETIC_ERRNO(EINVAL), "Expected value in range 0-127"); - - if (wakeup_type_byte == 3) { - log_debug("DMI BIOS System Information indicates wakeup type is APM Timer"); - return true; - } - - return false; -} - static int lock_all_homes(void) { _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; _cleanup_(sd_bus_message_unrefp) sd_bus_message *m = NULL; @@ -242,12 +205,12 @@ static int execute( SleepOperation operation, const char *action) { - char *arguments[] = { + const char *arguments[] = { NULL, - (char*) "pre", + "pre", /* NB: we use 'arg_operation' instead of 'operation' here, as we want to communicate the overall * operation here, not the specific one, in case of s2h. */ - (char*) sleep_operation_to_string(arg_operation), + sleep_operation_to_string(arg_operation), NULL }; static const char* const dirs[] = { @@ -257,18 +220,13 @@ static int execute( _cleanup_(hibernation_device_done) HibernationDevice hibernation_device = {}; _cleanup_close_ int state_fd = -EBADF; - char **modes, **states; int r; assert(sleep_config); assert(operation >= 0); - assert(operation < _SLEEP_OPERATION_MAX); - assert(operation != SLEEP_SUSPEND_THEN_HIBERNATE); /* Handled by execute_s2h() instead */ + assert(operation < _SLEEP_OPERATION_CONFIG_MAX); /* Others are handled by execute_s2h() instead */ - states = sleep_config->states[operation]; - modes = sleep_config->modes[operation]; - - if (strv_isempty(states)) + if (strv_isempty(sleep_config->states[operation])) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "No sleep states configured for sleep operation %s, can't sleep.", sleep_operation_to_string(operation)); @@ -296,16 +254,16 @@ static int execute( r = write_resume_config(hibernation_device.devno, hibernation_device.offset, hibernation_device.path); if (r < 0) { - if (is_efi_boot()) - (void) efi_set_variable(EFI_SYSTEMD_VARIABLE(HibernateLocation), NULL, 0); - - return log_error_errno(r, "Failed to prepare for hibernation: %m"); + log_error_errno(r, "Failed to write hibernation device to /sys/power/resume or /sys/power/resume_offset: %m"); + goto fail; } } - r = write_mode(modes); - if (r < 0) - return log_error_errno(r, "Failed to write mode to /sys/power/disk: %m"); + r = write_mode(sleep_config->modes[operation]); + if (r < 0) { + log_error_errno(r, "Failed to write mode to /sys/power/disk: %m"); + goto fail; + } } /* Pass an action string to the call-outs. This is mostly our operation string, except if the @@ -313,19 +271,18 @@ static int execute( if (!action) action = sleep_operation_to_string(operation); - r = setenv("SYSTEMD_SLEEP_ACTION", action, 1); - if (r != 0) - log_warning_errno(errno, "Error setting SYSTEMD_SLEEP_ACTION=%s, ignoring: %m", action); + if (setenv("SYSTEMD_SLEEP_ACTION", action, /* overwrite = */ 1) < 0) + log_warning_errno(errno, "Failed to set SYSTEMD_SLEEP_ACTION=%s, ignoring: %m", action); - (void) execute_directories(dirs, DEFAULT_TIMEOUT_USEC, NULL, NULL, arguments, NULL, EXEC_DIR_PARALLEL | EXEC_DIR_IGNORE_ERRORS); + (void) execute_directories(dirs, DEFAULT_TIMEOUT_USEC, NULL, NULL, (char **) arguments, NULL, EXEC_DIR_PARALLEL | EXEC_DIR_IGNORE_ERRORS); (void) lock_all_homes(); log_struct(LOG_INFO, "MESSAGE_ID=" SD_MESSAGE_SLEEP_START_STR, - LOG_MESSAGE("Entering sleep state '%s'...", sleep_operation_to_string(operation)), + LOG_MESSAGE("Performing sleep operation '%s'...", sleep_operation_to_string(operation)), "SLEEP=%s", sleep_operation_to_string(arg_operation)); - r = write_state(state_fd, states); + r = write_state(state_fd, sleep_config->states[operation]); if (r < 0) log_struct_errno(LOG_ERR, r, "MESSAGE_ID=" SD_MESSAGE_SLEEP_STOP_STR, @@ -334,15 +291,59 @@ static int execute( else log_struct(LOG_INFO, "MESSAGE_ID=" SD_MESSAGE_SLEEP_STOP_STR, - LOG_MESSAGE("System returned from sleep state."), + LOG_MESSAGE("System returned from sleep operation '%s'.", sleep_operation_to_string(arg_operation)), "SLEEP=%s", sleep_operation_to_string(arg_operation)); - arguments[1] = (char*) "post"; - (void) execute_directories(dirs, DEFAULT_TIMEOUT_USEC, NULL, NULL, arguments, NULL, EXEC_DIR_PARALLEL | EXEC_DIR_IGNORE_ERRORS); + arguments[1] = "post"; + (void) execute_directories(dirs, DEFAULT_TIMEOUT_USEC, NULL, NULL, (char **) arguments, NULL, EXEC_DIR_PARALLEL | EXEC_DIR_IGNORE_ERRORS); + + if (r >= 0) + return 0; + +fail: + if (sleep_operation_is_hibernation(operation) && is_efi_boot()) + (void) efi_set_variable(EFI_SYSTEMD_VARIABLE(HibernateLocation), NULL, 0); return r; } +/* Return true if wakeup type is APM timer */ +static int check_wakeup_type(void) { + static const char dmi_object_path[] = "/sys/firmware/dmi/entries/1-0/raw"; + uint8_t wakeup_type_byte, tablesize; + _cleanup_free_ char *buf = NULL; + size_t bufsize; + int r; + + /* implementation via dmi/entries */ + r = read_full_virtual_file(dmi_object_path, &buf, &bufsize); + if (r < 0) + return log_debug_errno(r, "Unable to read %s: %m", dmi_object_path); + if (bufsize < 25) + return log_debug_errno(SYNTHETIC_ERRNO(EINVAL), + "Only read %zu bytes from %s (expected 25)", + bufsize, dmi_object_path); + + /* index 1 stores the size of table */ + tablesize = (uint8_t) buf[1]; + if (tablesize < 25) + return log_debug_errno(SYNTHETIC_ERRNO(EINVAL), + "Table size less than the index[0x18] where waketype byte is available."); + + wakeup_type_byte = (uint8_t) buf[24]; + /* 0 is Reserved and 8 is AC Power Restored. As per table 12 in + * https://www.dmtf.org/sites/default/files/standards/documents/DSP0134_3.4.0.pdf */ + if (wakeup_type_byte >= 128) + return log_debug_errno(SYNTHETIC_ERRNO(EINVAL), "Expected value in range 0-127"); + + if (wakeup_type_byte == 3) { + log_debug("DMI BIOS System Information indicates wakeup type is APM Timer"); + return true; + } + + return false; +} + static int custom_timer_suspend(const SleepConfig *sleep_config) { usec_t hibernate_timestamp; int r;