From 792dd6f46c285a0d3434c5caa4a9a80a8823cc3b Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Wed, 27 Mar 2024 00:30:06 +0800 Subject: [PATCH 1/7] sleep-config: make sleep_operation_is_hibernation uppercase --- src/shared/sleep-config.c | 2 +- src/shared/sleep-config.h | 2 +- src/sleep/sleep.c | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/shared/sleep-config.c b/src/shared/sleep-config.c index c96f8485dd..5f51ab45de 100644 --- a/src/shared/sleep-config.c +++ b/src/shared/sleep-config.c @@ -341,7 +341,7 @@ static int sleep_supported_internal( return false; } - if (sleep_operation_is_hibernation(operation)) { + 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 bc5aeb91bd..93811c63a2 100644 --- a/src/shared/sleep-config.h +++ b/src/shared/sleep-config.h @@ -20,7 +20,7 @@ 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) { +static inline bool SLEEP_OPERATION_IS_HIBERNATION(SleepOperation operation) { return IN_SET(operation, SLEEP_HIBERNATE, SLEEP_HYBRID_SLEEP); } diff --git a/src/sleep/sleep.c b/src/sleep/sleep.c index 0f619ce0f5..bdba4284bf 100644 --- a/src/sleep/sleep.c +++ b/src/sleep/sleep.c @@ -238,7 +238,7 @@ static int execute( return log_error_errno(errno, "Failed to open /sys/power/state: %m"); /* Configure hibernation settings if we are supposed to hibernate */ - if (sleep_operation_is_hibernation(operation)) { + if (SLEEP_OPERATION_IS_HIBERNATION(operation)) { _cleanup_(hibernation_device_done) HibernationDevice hibernation_device = {}; bool resume_set; @@ -301,7 +301,7 @@ static int execute( return 0; fail: - if (sleep_operation_is_hibernation(operation)) + if (SLEEP_OPERATION_IS_HIBERNATION(operation)) clear_efi_hibernate_location_and_warn(); return r; From 0a3fd5ce432905b221448f4e8a2a9512bf5252f9 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Wed, 27 Mar 2024 19:59:51 +0800 Subject: [PATCH 2/7] man/systemd-sleep: reorder options --- man/systemd-sleep.conf.xml | 42 +++++++++++++++++++------------------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/man/systemd-sleep.conf.xml b/man/systemd-sleep.conf.xml index 411577d519..b4db11a6b3 100644 --- a/man/systemd-sleep.conf.xml +++ b/man/systemd-sleep.conf.xml @@ -153,27 +153,6 @@ - - HibernateMode= - - The string to be written to /sys/power/disk by - systemd-hibernate.service8. - More than one value can be specified by separating multiple values with whitespace. They will be - tried in turn, until one is written without error. If none of the writes succeed, the operation will - be aborted. - - The allowed set of values is determined by the kernel and is shown in the file itself (use - cat /sys/power/disk to display). See the kernel documentation page - - Basic sysfs Interfaces for System Suspend and Hibernation for more details. - - - systemd-suspend-then-hibernate.service8 - uses the value of HibernateMode= when hibernating. - - - - SuspendState= @@ -195,6 +174,27 @@ + + HibernateMode= + + The string to be written to /sys/power/disk by + systemd-hibernate.service8. + More than one value can be specified by separating multiple values with whitespace. They will be + tried in turn, until one is written without error. If none of the writes succeed, the operation will + be aborted. + + The allowed set of values is determined by the kernel and is shown in the file itself (use + cat /sys/power/disk to display). See the kernel documentation page + + Basic sysfs Interfaces for System Suspend and Hibernation for more details. + + + systemd-suspend-then-hibernate.service8 + uses the value of HibernateMode= when hibernating. + + + + HibernateDelaySec= From a6fe020ab81b1396fa4c994cbeaab34719a2192c Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Wed, 27 Mar 2024 21:03:30 +0800 Subject: [PATCH 3/7] sleep-config: fix potential memory leak Follow-up for 77bd39385eae7b8e3182e66292b9f94bb54e235d --- src/shared/sleep-config.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/shared/sleep-config.c b/src/shared/sleep-config.c index 5f51ab45de..f99c38bc79 100644 --- a/src/shared/sleep-config.c +++ b/src/shared/sleep-config.c @@ -69,8 +69,8 @@ static int config_parse_sleep_mode( void *data, void *userdata) { - _cleanup_strv_free_ char **modes = NULL; char ***sv = ASSERT_PTR(data); + _cleanup_strv_free_ char **modes = NULL; int r; assert(filename); @@ -87,7 +87,7 @@ static int config_parse_sleep_mode( return log_oom(); } - return free_and_replace(*sv, modes); + return strv_free_and_replace(*sv, modes); } static void sleep_config_validate_state_and_mode(SleepConfig *sc) { From f8c700791e7a11d7a9aab0112da31725421d43fe Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Wed, 27 Mar 2024 00:15:40 +0800 Subject: [PATCH 4/7] string-util: modernize string_contains_word_strv a bit Also correct the comment on flags. --- src/basic/string-util.c | 12 +++++------- src/basic/string-util.h | 2 +- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/src/basic/string-util.c b/src/basic/string-util.c index 7f6c87b461..b50f45f7f4 100644 --- a/src/basic/string-util.c +++ b/src/basic/string-util.c @@ -1268,18 +1268,16 @@ int string_extract_line(const char *s, size_t i, char **ret) { } } -int string_contains_word_strv(const char *string, const char *separators, char **words, const char **ret_word) { - /* In the default mode with no separators specified, we split on whitespace and - * don't coalesce separators. */ +int string_contains_word_strv(const char *string, const char *separators, char * const *words, const char **ret_word) { + /* In the default mode with no separators specified, we split on whitespace and coalesce separators. */ const ExtractFlags flags = separators ? EXTRACT_DONT_COALESCE_SEPARATORS : 0; - const char *found = NULL; + int r; - for (const char *p = string;;) { + for (;;) { _cleanup_free_ char *w = NULL; - int r; - r = extract_first_word(&p, &w, separators, flags); + r = extract_first_word(&string, &w, separators, flags); if (r < 0) return r; if (r == 0) diff --git a/src/basic/string-util.h b/src/basic/string-util.h index a68f3c94ab..055ab66668 100644 --- a/src/basic/string-util.h +++ b/src/basic/string-util.h @@ -271,7 +271,7 @@ char* string_erase(char *x); int string_truncate_lines(const char *s, size_t n_lines, char **ret); int string_extract_line(const char *s, size_t i, char **ret); -int string_contains_word_strv(const char *string, const char *separators, char **words, const char **ret_word); +int string_contains_word_strv(const char *string, const char *separators, char * const *words, const char **ret_word); static inline int string_contains_word(const char *string, const char *separators, const char *word) { return string_contains_word_strv(string, separators, STRV_MAKE(word), NULL); } From b0e3b85a7c8a2b9a1a977caa3c47aaae3defc288 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Wed, 27 Mar 2024 00:24:58 +0800 Subject: [PATCH 5/7] sleep-config: make sleep_mode_supported take a path Preparation for later commits. --- src/shared/sleep-config.c | 25 ++++++++++++++----------- src/shared/sleep-config.h | 4 ++-- src/test/test-sleep-config.c | 8 ++++---- 3 files changed, 20 insertions(+), 17 deletions(-) diff --git a/src/shared/sleep-config.c b/src/shared/sleep-config.c index f99c38bc79..be20f0aae2 100644 --- a/src/shared/sleep-config.c +++ b/src/shared/sleep-config.c @@ -183,7 +183,7 @@ int parse_sleep_config(SleepConfig **ret) { return 0; } -int sleep_state_supported(char **states) { +int sleep_state_supported(char * const *states) { _cleanup_free_ char *supported_sysfs = NULL; const char *found; int r; @@ -213,22 +213,24 @@ int sleep_state_supported(char **states) { return false; } -int sleep_mode_supported(char **modes) { +int sleep_mode_supported(const char *path, char * const *modes) { _cleanup_free_ char *supported_sysfs = NULL; int r; + assert(path); + /* Unlike state, kernel has its own default choice if not configured */ if (strv_isempty(modes)) { - log_debug("No sleep mode configured, using kernel default."); + log_debug("No sleep mode configured, using kernel default for %s.", path); return true; } - if (access("/sys/power/disk", W_OK) < 0) - return log_debug_errno(errno, "/sys/power/disk is not writable: %m"); + if (access(path, W_OK) < 0) + return log_debug_errno(errno, "%s is not writable: %m", path); - r = read_one_line_file("/sys/power/disk", &supported_sysfs); + r = read_one_line_file(path, &supported_sysfs); if (r < 0) - return log_debug_errno(r, "Failed to read /sys/power/disk: %m"); + return log_debug_errno(r, "Failed to read %s: %m", path); for (const char *p = supported_sysfs;;) { _cleanup_free_ char *word = NULL; @@ -237,7 +239,7 @@ int sleep_mode_supported(char **modes) { r = extract_first_word(&p, &word, NULL, 0); if (r < 0) - return log_debug_errno(r, "Failed to parse /sys/power/disk: %m"); + return log_debug_errno(r, "Failed to parse %s: %m", path); if (r == 0) break; @@ -250,14 +252,15 @@ int sleep_mode_supported(char **modes) { } if (strv_contains(modes, mode)) { - log_debug("Disk sleep mode '%s' is supported by kernel.", mode); + log_debug("Sleep mode '%s' is supported by kernel (%s).", mode, path); return true; } } if (DEBUG_LOGGING) { _cleanup_free_ char *joined = strv_join(modes, " "); - log_debug("None of the configured hibernation power modes are supported by kernel: %s", strnull(joined)); + log_debug("None of the configured modes are supported by kernel (%s): %s", + path, strnull(joined)); } return false; } @@ -342,7 +345,7 @@ static int sleep_supported_internal( } if (SLEEP_OPERATION_IS_HIBERNATION(operation)) { - r = sleep_mode_supported(sleep_config->modes[operation]); + r = sleep_mode_supported("/sys/power/disk", sleep_config->modes[operation]); if (r < 0) return r; if (r == 0) { diff --git a/src/shared/sleep-config.h b/src/shared/sleep-config.h index 93811c63a2..12239a8a01 100644 --- a/src/shared/sleep-config.h +++ b/src/shared/sleep-config.h @@ -55,5 +55,5 @@ static inline int sleep_supported(SleepOperation operation) { } /* Only for test-sleep-config */ -int sleep_state_supported(char **states); -int sleep_mode_supported(char **modes); +int sleep_state_supported(char * const *states); +int sleep_mode_supported(const char *path, char * const *modes); diff --git a/src/test/test-sleep-config.c b/src/test/test-sleep-config.c index 112fec6346..dc1a8fb71c 100644 --- a/src/test/test-sleep-config.c +++ b/src/test/test-sleep-config.c @@ -52,10 +52,10 @@ TEST(sleep_supported) { log_info("Standby configured: %s", yes_no(sleep_state_supported(standby) > 0)); log_info("Suspend configured: %s", yes_no(sleep_state_supported(mem) > 0)); log_info("Hibernate configured: %s", yes_no(sleep_state_supported(disk) > 0)); - log_info("Hibernate+Suspend (Hybrid-Sleep) configured: %s", yes_no(sleep_mode_supported(suspend) > 0)); - log_info("Hibernate+Reboot configured: %s", yes_no(sleep_mode_supported(reboot) > 0)); - log_info("Hibernate+Platform configured: %s", yes_no(sleep_mode_supported(platform) > 0)); - log_info("Hibernate+Shutdown configured: %s", yes_no(sleep_mode_supported(shutdown) > 0)); + log_info("Hibernate+Suspend (Hybrid-Sleep) configured: %s", yes_no(sleep_mode_supported("/sys/power/disk", suspend) > 0)); + log_info("Hibernate+Reboot configured: %s", yes_no(sleep_mode_supported("/sys/power/disk", reboot) > 0)); + log_info("Hibernate+Platform configured: %s", yes_no(sleep_mode_supported("/sys/power/disk", platform) > 0)); + log_info("Hibernate+Shutdown configured: %s", yes_no(sleep_mode_supported("/sys/power/disk", shutdown) > 0)); log_info("Freeze configured: %s", yes_no(sleep_state_supported(freeze) > 0)); log_info("/= high-level sleep verbs =/"); From b78612832206f1ca337195d83ca00cfd0e6c7f25 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Wed, 27 Mar 2024 19:45:08 +0800 Subject: [PATCH 6/7] sleep: make write_mode take a path Similar to the previous commit, preparation for adding support for /sys/power/mem_sleep. --- src/sleep/sleep.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/sleep/sleep.c b/src/sleep/sleep.c index bdba4284bf..72489b31a0 100644 --- a/src/sleep/sleep.c +++ b/src/sleep/sleep.c @@ -151,22 +151,22 @@ static int write_state(int fd, char * const *states) { return r; } -static int write_mode(char * const *modes) { - int r = 0; +static int write_mode(const char *path, char * const *modes) { + int r, ret = 0; + + assert(path); STRV_FOREACH(mode, modes) { - int k; - - k = write_string_file("/sys/power/disk", *mode, WRITE_STRING_FILE_DISABLE_BUFFER); - if (k >= 0) { - log_debug("Using sleep disk mode '%s'.", *mode); + r = write_string_file(path, *mode, WRITE_STRING_FILE_DISABLE_BUFFER); + if (r >= 0) { + log_debug("Using sleep mode '%s' for %s.", *mode, path); return 0; } - RET_GATHER(r, log_debug_errno(k, "Failed to write '%s' to /sys/power/disk: %m", *mode)); + RET_GATHER(ret, log_debug_errno(r, "Failed to write '%s' to %s: %m", *mode, path)); } - return r; + return ret; } static int lock_all_homes(void) { @@ -259,7 +259,7 @@ static int execute( goto fail; } - r = write_mode(sleep_config->modes[operation]); + r = write_mode("/sys/power/disk", sleep_config->modes[operation]); if (r < 0) { log_error_errno(r, "Failed to write mode to /sys/power/disk: %m"); goto fail; From a2124b35e92178fd028d7c29ecdd5e80ea7f7b8f Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Wed, 27 Mar 2024 19:45:34 +0800 Subject: [PATCH 7/7] sleep: add SleepMemMode= setting for configuring /sys/power/mem_sleep The setting is used when /sys/power/state is set to 'mem' (common for suspend) or /sys/power/disk is set to 'suspend' (hybrid-sleep). We default to kernel choice here, i.e. respect what's set through 'mem_sleep_default=' kernel cmdline option. --- man/systemd-sleep.conf.xml | 18 ++++++++++++++++++ src/shared/sleep-config.c | 14 ++++++++++++++ src/shared/sleep-config.h | 16 +++++++++++++++- src/sleep/sleep.c | 6 ++++++ src/sleep/sleep.conf | 1 + 5 files changed, 54 insertions(+), 1 deletion(-) diff --git a/man/systemd-sleep.conf.xml b/man/systemd-sleep.conf.xml index b4db11a6b3..7a343975d7 100644 --- a/man/systemd-sleep.conf.xml +++ b/man/systemd-sleep.conf.xml @@ -195,6 +195,24 @@ + + SleepMemMode= + + The string to be written to /sys/power/mem_sleep + when or hybrid-sleep is used. + More than one value can be specified by separating multiple values with whitespace. They will be + tried in turn, until one is written without error. If none of the writes succeed, the operation will + be aborted. Defaults to empty, i.e. the kernel default or kernel command line option + mem_sleep_default= is respected. + + The allowed set of values is determined by the kernel and is shown in the file itself (use + cat /sys/power/mem_sleep to display). See the kernel documentation page + + Basic sysfs Interfaces for System Suspend and Hibernation for more details. + + + + HibernateDelaySec= diff --git a/src/shared/sleep-config.c b/src/shared/sleep-config.c index be20f0aae2..4a90ad67bd 100644 --- a/src/shared/sleep-config.c +++ b/src/shared/sleep-config.c @@ -54,6 +54,8 @@ SleepConfig* sleep_config_free(SleepConfig *sc) { strv_free(sc->modes[i]); } + strv_free(sc->mem_modes); + return mfree(sc); } @@ -140,6 +142,8 @@ int parse_sleep_config(SleepConfig **ret) { { "Sleep", "HybridSleepState", config_parse_warn_compat, DISABLED_LEGACY, NULL }, { "Sleep", "HybridSleepMode", config_parse_warn_compat, DISABLED_LEGACY, NULL }, + { "Sleep", "SleepMemMode", config_parse_sleep_mode, 0, &sc->mem_modes }, + { "Sleep", "HibernateDelaySec", config_parse_sec, 0, &sc->hibernate_delay_usec }, { "Sleep", "SuspendEstimationSec", config_parse_sec, 0, &sc->suspend_estimation_usec }, {} @@ -344,6 +348,16 @@ static int sleep_supported_internal( return false; } + if (SLEEP_NEEDS_MEM_SLEEP(sleep_config, operation)) { + r = sleep_mode_supported("/sys/power/mem_sleep", sleep_config->mem_modes); + if (r < 0) + return r; + if (r == 0) { + *ret_support = SLEEP_STATE_OR_MODE_NOT_SUPPORTED; + return false; + } + } + if (SLEEP_OPERATION_IS_HIBERNATION(operation)) { r = sleep_mode_supported("/sys/power/disk", sleep_config->modes[operation]); if (r < 0) diff --git a/src/shared/sleep-config.h b/src/shared/sleep-config.h index 12239a8a01..75d9c4a622 100644 --- a/src/shared/sleep-config.h +++ b/src/shared/sleep-config.h @@ -1,6 +1,7 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ #pragma once +#include "strv.h" #include "time-util.h" typedef enum SleepOperation { @@ -28,7 +29,8 @@ typedef struct SleepConfig { bool allow[_SLEEP_OPERATION_MAX]; char **states[_SLEEP_OPERATION_CONFIG_MAX]; - char **modes[_SLEEP_OPERATION_CONFIG_MAX]; /* Power mode after writing hibernation image */ + char **modes[_SLEEP_OPERATION_CONFIG_MAX]; /* Power mode after writing hibernation image (/sys/power/disk) */ + char **mem_modes; /* /sys/power/mem_sleep */ usec_t hibernate_delay_usec; usec_t suspend_estimation_usec; @@ -39,6 +41,18 @@ DEFINE_TRIVIAL_CLEANUP_FUNC(SleepConfig*, sleep_config_free); int parse_sleep_config(SleepConfig **sleep_config); +static inline bool SLEEP_NEEDS_MEM_SLEEP(const SleepConfig *sc, SleepOperation operation) { + assert(sc); + assert(operation >= 0 && operation < _SLEEP_OPERATION_CONFIG_MAX); + + /* As per https://docs.kernel.org/admin-guide/pm/sleep-states.html#basic-sysfs-interfaces-for-system-suspend-and-hibernation, + * /sys/power/mem_sleep is honored if /sys/power/state is set to "mem" (common for suspend) + * or /sys/power/disk is set to "suspend" (hybrid-sleep). */ + + return strv_contains(sc->states[operation], "mem") || + strv_contains(sc->modes[operation], "suspend"); +} + typedef enum SleepSupport { SLEEP_SUPPORTED, SLEEP_DISABLED, /* Disabled in SleepConfig.allow */ diff --git a/src/sleep/sleep.c b/src/sleep/sleep.c index 72489b31a0..693484e886 100644 --- a/src/sleep/sleep.c +++ b/src/sleep/sleep.c @@ -237,6 +237,12 @@ static int execute( if (state_fd < 0) return log_error_errno(errno, "Failed to open /sys/power/state: %m"); + if (SLEEP_NEEDS_MEM_SLEEP(sleep_config, operation)) { + r = write_mode("/sys/power/mem_sleep", sleep_config->mem_modes); + if (r < 0) + return log_error_errno(r, "Failed to write mode to /sys/power/mem_sleep: %m"); + } + /* Configure hibernation settings if we are supposed to hibernate */ if (SLEEP_OPERATION_IS_HIBERNATION(operation)) { _cleanup_(hibernation_device_done) HibernationDevice hibernation_device = {}; diff --git a/src/sleep/sleep.conf b/src/sleep/sleep.conf index fad95b3897..d12f97099a 100644 --- a/src/sleep/sleep.conf +++ b/src/sleep/sleep.conf @@ -23,5 +23,6 @@ #AllowHybridSleep=yes #SuspendState=mem standby freeze #HibernateMode=platform shutdown +#SleepMemMode= #HibernateDelaySec= #SuspendEstimationSec=60min