Merge pull request #25374 from yuwata/sleep-fixlets

sleep: several fixlets

src/shared/sleep-config.c

@@ -22,6 +22,7 @@
 #include "btrfs-util.h"
 #include "conf-parser.h"
 #include "constants.h"
+#include "device-private.h"
 #include "device-util.h"
 #include "devnum-util.h"
 #include "env-util.h"
@@ -65,10 +66,14 @@ int parse_sleep_config(SleepConfig **ret_sleep_config) {
         int allow_suspend = -1, allow_hibernate = -1,
             allow_s2h = -1, allow_hybrid_sleep = -1;
 
-        sc = new0(SleepConfig, 1);
+        sc = new(SleepConfig, 1);
         if (!sc)
                 return log_oom();
 
+        *sc = (SleepConfig) {
+                .hibernate_delay_usec = USEC_INFINITY,
+        };
+
         const ConfigTableItem items[] = {
                 { "Sleep", "AllowSuspend",              config_parse_tristate, 0, &allow_suspend                  },
                 { "Sleep", "AllowHibernation",          config_parse_tristate, 0, &allow_hibernate                },
@@ -82,7 +87,8 @@ int parse_sleep_config(SleepConfig **ret_sleep_config) {
                 { "Sleep", "HybridSleepMode",           config_parse_strv,     0, sc->modes + SLEEP_HYBRID_SLEEP  },
                 { "Sleep", "HybridSleepState",          config_parse_strv,     0, sc->states + SLEEP_HYBRID_SLEEP },
 
-                { "Sleep", "HibernateDelaySec",         config_parse_sec,      0, &sc->hibernate_delay_sec        },
+                { "Sleep", "HibernateDelaySec",         config_parse_sec,      0, &sc->hibernate_delay_usec       },
+                { "Sleep", "SuspendEstimationSec",      config_parse_sec,      0, &sc->suspend_estimation_usec    },
                 {}
         };
 
@@ -113,8 +119,8 @@ int parse_sleep_config(SleepConfig **ret_sleep_config) {
                 sc->modes[SLEEP_HYBRID_SLEEP] = strv_new("suspend", "platform", "shutdown");
         if (!sc->states[SLEEP_HYBRID_SLEEP])
                 sc->states[SLEEP_HYBRID_SLEEP] = strv_new("disk");
-        if (sc->hibernate_delay_sec == 0)
-                sc->hibernate_delay_sec = 2 * USEC_PER_HOUR;
+        if (sc->suspend_estimation_usec == 0)
+                sc->suspend_estimation_usec = DEFAULT_SUSPEND_ESTIMATION_USEC;
 
         /* Ensure values set for all required fields */
         if (!sc->states[SLEEP_SUSPEND] || !sc->modes[SLEEP_HIBERNATE]
@@ -137,16 +143,27 @@ static int battery_enumerator_new(sd_device_enumerator **ret) {
         if (r < 0)
                 return r;
 
-        r = sd_device_enumerator_add_match_subsystem(e, "power_supply", /* match= */ true);
+        r = sd_device_enumerator_add_match_subsystem(e, "power_supply", /* match = */ true);
         if (r < 0)
                 return r;
 
-        r = sd_device_enumerator_add_match_property(e, "POWER_SUPPLY_TYPE", "Battery");
+        r = sd_device_enumerator_allow_uninitialized(e);
+        if (r < 0)
+                return r;
+
+        r = sd_device_enumerator_add_match_sysattr(e, "type", "Battery", /* match = */ true);
+        if (r < 0)
+                return r;
+
+        r = sd_device_enumerator_add_match_sysattr(e, "present", "1", /* match = */ true);
+        if (r < 0)
+                return r;
+
+        r = sd_device_enumerator_add_match_sysattr(e, "scope", "Device", /* match = */ false);
         if (r < 0)
                 return r;
 
         *ret = TAKE_PTR(e);
-
         return 0;
 }
 
@@ -165,18 +182,13 @@ int get_capacity_by_name(Hashmap *capacities_by_name, const char *name) {
 
 /* Battery percentage capacity fetched from capacity file and if in range 0-100 then returned */
 static int read_battery_capacity_percentage(sd_device *dev) {
-        const char *power_supply_capacity;
         int battery_capacity, r;
 
         assert(dev);
 
-        r = sd_device_get_property_value(dev, "POWER_SUPPLY_CAPACITY", &power_supply_capacity);
+        r = device_get_sysattr_int(dev, "capacity", &battery_capacity);
         if (r < 0)
-                return log_device_debug_errno(dev, r, "Failed to get property POWER_SUPPLY_CAPACITY: %m");
-
-        r = safe_atoi(power_supply_capacity, &battery_capacity);
-        if (r < 0)
-                return log_device_debug_errno(dev, r, "Failed to parse property POWER_SUPPLY_CAPACITY: %m");
+                return log_device_debug_errno(dev, r, "Failed to read/parse POWER_SUPPLY_CAPACITY: %m");
 
         if (battery_capacity < 0 || battery_capacity > 100)
                 return log_device_debug_errno(dev, SYNTHETIC_ERRNO(ERANGE), "Invalid battery capacity");
@@ -198,15 +210,9 @@ int battery_is_low(void) {
         if (r < 0)
                 return log_debug_errno(r, "Failed to initialize battery enumerator: %m");
 
-        FOREACH_DEVICE(e, dev) {
-                r = read_battery_capacity_percentage(dev);
-                if (r < 0) {
-                        log_device_debug_errno(dev, r, "Failed to get battery capacity, ignoring: %m");
-                        continue;
-                }
-                if (r > BATTERY_LOW_CAPACITY_LEVEL)
+        FOREACH_DEVICE(e, dev)
+                if (read_battery_capacity_percentage(dev) > BATTERY_LOW_CAPACITY_LEVEL)
                         return false;
-        }
 
         return true;
 }
@@ -259,61 +265,55 @@ int fetch_batteries_capacity_by_name(Hashmap **ret) {
         return 0;
 }
 
-/* Read file path and return hash of value in that file */
-static int get_battery_identifier(sd_device *dev, const char *property, struct siphash *state) {
+static int siphash24_compress_device_sysattr(sd_device *dev, const char *attr, struct siphash *state) {
         const char *x;
         int r;
 
         assert(dev);
-        assert(property);
+        assert(attr);
         assert(state);
 
-        r = sd_device_get_property_value(dev, property, &x);
-        if (r == -ENOENT)
-               log_device_debug_errno(dev, r, "Battery device property %s is unavailable, ignoring: %m", property);
-        else if (r < 0)
-               return log_device_debug_errno(dev, r, "Failed to get battery device property %s: %m", property);
-        else if (isempty(x))
-               log_device_debug(dev, "Battery device property '%s' is empty.", property);
-        else
-               siphash24_compress_string(x, state);
+        r = sd_device_get_sysattr_value(dev, attr, &x);
+        if (r < 0)
+                return log_device_debug_errno(dev, r, "Failed to read '%s' attribute: %m", attr);
 
+        if (!isempty(x))
+                siphash24_compress_string(x, state);
+
+        return 0;
+}
+
+static int siphash24_compress_id128(int (*getter)(sd_id128_t*), const char *name, struct siphash *state) {
+        sd_id128_t id;
+        int r;
+
+        assert(getter);
+        assert(state);
+
+        r = getter(&id);
+        if (r < 0)
+                return log_debug_errno(r, "Failed to get %s ID: %m", name);
+
+        siphash24_compress(&id, sizeof(sd_id128_t), state);
         return 0;
 }
 
 /* Read system and battery identifier from specific location and generate hash of it */
 static int get_system_battery_identifier_hash(sd_device *dev, uint64_t *ret) {
         struct siphash state;
-        sd_id128_t machine_id, product_id;
-        int r;
 
         assert(ret);
         assert(dev);
 
         siphash24_init(&state, BATTERY_DISCHARGE_RATE_HASH_KEY.bytes);
 
-        get_battery_identifier(dev, "POWER_SUPPLY_MANUFACTURER", &state);
-        get_battery_identifier(dev, "POWER_SUPPLY_MODEL_NAME", &state);
-        get_battery_identifier(dev, "POWER_SUPPLY_SERIAL_NUMBER", &state);
-
-        r = sd_id128_get_machine(&machine_id);
-        if (r == -ENOENT)
-               log_debug_errno(r, "machine ID is unavailable: %m");
-        else if (r < 0)
-               return log_debug_errno(r, "Failed to get machine ID: %m");
-        else
-               siphash24_compress(&machine_id, sizeof(sd_id128_t), &state);
-
-        r = id128_get_product(&product_id);
-        if (r == -ENOENT)
-               log_debug_errno(r, "product_id does not exist: %m");
-        else if (r < 0)
-               return log_debug_errno(r, "Failed to get product ID: %m");
-        else
-               siphash24_compress(&product_id, sizeof(sd_id128_t), &state);
+        (void) siphash24_compress_device_sysattr(dev, "manufacturer", &state);
+        (void) siphash24_compress_device_sysattr(dev, "model_name", &state);
+        (void) siphash24_compress_device_sysattr(dev, "serial_number", &state);
+        (void) siphash24_compress_id128(sd_id128_get_machine, "machine", &state);
+        (void) siphash24_compress_id128(id128_get_product, "product", &state);
 
         *ret = siphash24_finalize(&state);
-
         return 0;
 }
 
@@ -391,11 +391,11 @@ static int put_battery_discharge_rate(int estimated_battery_discharge_rate, uint
                                         estimated_battery_discharge_rate);
 
         r = write_string_filef(
-                DISCHARGE_RATE_FILEPATH,
-                WRITE_STRING_FILE_CREATE | WRITE_STRING_FILE_MKDIR_0755 | (trunc ? WRITE_STRING_FILE_TRUNCATE : 0),
-                "%"PRIu64" %d",
-                system_hash_id,
-                estimated_battery_discharge_rate);
+                        DISCHARGE_RATE_FILEPATH,
+                        WRITE_STRING_FILE_CREATE | WRITE_STRING_FILE_MKDIR_0755 | (trunc ? WRITE_STRING_FILE_TRUNCATE : 0),
+                        "%"PRIu64" %d",
+                        system_hash_id,
+                        estimated_battery_discharge_rate);
         if (r < 0)
                 return log_debug_errno(r, "Failed to update %s: %m", DISCHARGE_RATE_FILEPATH);
 

src/shared/sleep-config.h

@@ -6,6 +6,8 @@
 #include "hashmap.h"
 #include "time-util.h"
 
+#define DEFAULT_SUSPEND_ESTIMATION_USEC (1 * USEC_PER_HOUR)
+
 typedef enum SleepOperation {
         SLEEP_SUSPEND,
         SLEEP_HIBERNATE,
@@ -19,7 +21,8 @@ typedef struct SleepConfig {
         bool allow[_SLEEP_OPERATION_MAX];
         char **modes[_SLEEP_OPERATION_MAX];
         char **states[_SLEEP_OPERATION_MAX];
-        usec_t hibernate_delay_sec;
+        usec_t hibernate_delay_usec;
+        usec_t suspend_estimation_usec;
 } SleepConfig;
 
 SleepConfig* free_sleep_config(SleepConfig *sc);

src/sleep/sleep.c

@@ -267,36 +267,48 @@ static int execute(
 }
 
 static int custom_timer_suspend(const SleepConfig *sleep_config) {
+        usec_t hibernate_timestamp;
         int r;
 
         assert(sleep_config);
 
+        hibernate_timestamp = usec_add(now(CLOCK_BOOTTIME), sleep_config->hibernate_delay_usec);
+
         while (battery_is_low() == 0) {
                 _cleanup_hashmap_free_ Hashmap *last_capacity = NULL, *current_capacity = NULL;
                 _cleanup_close_ int tfd = -EBADF;
                 struct itimerspec ts = {};
-                usec_t suspend_interval = sleep_config->hibernate_delay_sec, before_timestamp = 0, after_timestamp = 0, total_suspend_interval;
+                usec_t suspend_interval;
                 bool woken_by_timer;
 
                 tfd = timerfd_create(CLOCK_BOOTTIME_ALARM, TFD_NONBLOCK|TFD_CLOEXEC);
                 if (tfd < 0)
                         return log_error_errno(errno, "Error creating timerfd: %m");
 
-                /* Store current battery capacity and current time before suspension */
+                /* Store current battery capacity before suspension */
                 r = fetch_batteries_capacity_by_name(&last_capacity);
-                if (r >= 0)
-                        before_timestamp = now(CLOCK_BOOTTIME);
-                else if (r == -ENOENT)
-                        /* In case of no battery, system suspend interval will be set to HibernateDelaySec=. */
-                        log_debug_errno(r, "Suspend Interval value set to %s: %m", FORMAT_TIMESPAN(suspend_interval, USEC_PER_SEC));
-                else
+                if (r < 0)
                         return log_error_errno(r, "Error fetching battery capacity percentage: %m");
 
-                r = get_total_suspend_interval(last_capacity, &total_suspend_interval);
-                if (r < 0)
-                        log_debug_errno(r, "Failed to estimate suspend interval using previous discharge rate, ignoring: %m");
-                else
-                        suspend_interval = total_suspend_interval;
+                if (hashmap_isempty(last_capacity))
+                        /* In case of no battery, system suspend interval will be set to HibernateDelaySec= or 2 hours. */
+                        suspend_interval = timestamp_is_set(hibernate_timestamp) ? sleep_config->hibernate_delay_usec : DEFAULT_SUSPEND_ESTIMATION_USEC;
+                else {
+                        r = get_total_suspend_interval(last_capacity, &suspend_interval);
+                        if (r < 0) {
+                                log_debug_errno(r, "Failed to estimate suspend interval using previous discharge rate, ignoring: %m");
+                                /* In case of any errors, especially when we do not know the battery
+                                 * discharging rate, system suspend interval will be set to
+                                 * SuspendEstimationSec=. */
+                                suspend_interval = sleep_config->suspend_estimation_usec;
+                        }
+                }
+
+                /* Do not suspend more than HibernateDelaySec= */
+                usec_t before_timestamp = now(CLOCK_BOOTTIME);
+                suspend_interval = MIN(suspend_interval, usec_sub_unsigned(hibernate_timestamp, before_timestamp));
+                if (suspend_interval <= 0)
+                        break; /* system should hibernate */
 
                 log_debug("Set timerfd wake alarm for %s", FORMAT_TIMESPAN(suspend_interval, USEC_PER_SEC));
                 /* Wake alarm for system with or without battery to hibernate or estimate discharge rate whichever is applicable */
@@ -316,17 +328,20 @@ static int custom_timer_suspend(const SleepConfig *sleep_config) {
                 woken_by_timer = FLAGS_SET(r, POLLIN);
 
                 r = fetch_batteries_capacity_by_name(&current_capacity);
-                if (r < 0) {
+                if (r < 0 || hashmap_isempty(current_capacity)) {
                         /* In case of no battery or error while getting charge level, no need to measure
-                         * discharge rate. Instead system should wakeup if it is manual wakeup or
-                         * hibernate if this is a timer wakeup.   */
-                        log_debug_errno(r, "Battery capacity percentage unavailable, cannot estimate discharge rate: %m");
+                         * discharge rate. Instead the system should wake up if it is manual wakeup or
+                         * hibernate if this is a timer wakeup. */
+                        if (r < 0)
+                                log_debug_errno(r, "Battery capacity percentage unavailable, cannot estimate discharge rate: %m");
+                        else
+                                log_debug("No battery found.");
                         if (!woken_by_timer)
                                 return 0;
                         break;
                 }
 
-                after_timestamp = now(CLOCK_BOOTTIME);
+                usec_t after_timestamp = now(CLOCK_BOOTTIME);
                 log_debug("Attempting to estimate battery discharge rate after wakeup from %s sleep",
                           FORMAT_TIMESPAN(after_timestamp - before_timestamp, USEC_PER_HOUR));
 
@@ -379,7 +394,7 @@ static int freeze_thaw_user_slice(const char **method) {
 
 static int execute_s2h(const SleepConfig *sleep_config) {
         _unused_ _cleanup_(freeze_thaw_user_slice) const char *auto_method_thaw = "ThawUnit";
-        int r, k;
+        int r;
 
         assert(sleep_config);
 
@@ -387,15 +402,21 @@ static int execute_s2h(const SleepConfig *sleep_config) {
         if (r < 0)
                 log_debug_errno(r, "Failed to freeze unit user.slice, ignoring: %m");
 
-        r = check_wakeup_type();
-        if (r < 0)
-                log_debug_errno(r, "Failed to check hardware wakeup type, ignoring: %m");
+        /* Only check if we have automated battery alarms if HibernateDelaySec= is not set, as in that case
+         * we'll busy poll for the configured interval instead */
+        if (!timestamp_is_set(sleep_config->hibernate_delay_usec)) {
+                r = check_wakeup_type();
+                if (r < 0)
+                        log_debug_errno(r, "Failed to check hardware wakeup type, ignoring: %m");
+                else {
+                        r = battery_trip_point_alarm_exists();
+                        if (r < 0)
+                                log_debug_errno(r, "Failed to check whether acpi_btp support is enabled or not, ignoring: %m");
+                }
+        } else
+                r = 0;  /* Force fallback path */
 
-        k = battery_trip_point_alarm_exists();
-        if (k < 0)
-                log_debug_errno(k, "Failed to check whether acpi_btp support is enabled or not, ignoring: %m");
-
-        if (r >= 0 && k > 0) {
+        if (r > 0) { /* If we have both wakeup alarms and battery trip point support, use them */
                 log_debug("Attempting to suspend...");
                 r = execute(sleep_config, SLEEP_SUSPEND, NULL);
                 if (r < 0)
@@ -410,9 +431,9 @@ static int execute_s2h(const SleepConfig *sleep_config) {
                         return 0;
         } else {
                 r = custom_timer_suspend(sleep_config);
-                if(r < 0)
+                if (r < 0)
                         return log_debug_errno(r, "Suspend cycle with manual battery discharge rate estimation failed: %m");
-                if(r == 0)
+                if (r == 0)
                         /* manual wakeup */
                         return 0;
         }

src/sleep/sleep.conf

@@ -23,4 +23,5 @@
 #HibernateState=disk
 #HybridSleepMode=suspend platform shutdown
 #HybridSleepState=disk
-#HibernateDelaySec=120min
+#HibernateDelaySec=
+#SuspendEstimationSec=60min