core: generalize memory accounting attribute handling

Follow-up for #29941

Also, support for MemoryCurrent in cgroup v1 is removed, as we're
going to remove that completely anyway.

Fixes #30000

6 files changed, 102 insertions, 140 deletions

diff --git a/src/core/cgroup.c b/src/core/cgroup.c
index 73431fa5db2..4e4b668f62c 100644
--- a/src/core/cgroup.c
+++ b/src/core/cgroup.c
@@ -4017,6 +4017,8 @@ int unit_get_memory_available(Unit *u, uint64_t *ret) {
 int unit_get_memory_current(Unit *u, uint64_t *ret) {
         int r;
 
+        // FIXME: Merge this into unit_get_memory_accounting after support for cgroup v1 is dropped
+
         assert(u);
         assert(ret);
 
@@ -4040,12 +4042,21 @@ int unit_get_memory_current(Unit *u, uint64_t *ret) {
         return cg_get_attribute_as_uint64("memory", u->cgroup_path, r > 0 ? "memory.current" : "memory.usage_in_bytes", ret);
 }
 
-static int unit_get_memory_attr_raw(Unit *u, const char* mem_attribute, uint64_t *ret) {
+int unit_get_memory_accounting(Unit *u, CGroupMemoryAccountingMetric metric, uint64_t *ret) {
+
+        static const char* const attributes_table[_CGROUP_MEMORY_ACCOUNTING_METRIC_MAX] = {
+                [CGROUP_MEMORY_PEAK]          = "memory.peak",
+                [CGROUP_MEMORY_SWAP_CURRENT]  = "memory.swap.current",
+                [CGROUP_MEMORY_SWAP_PEAK]     = "memory.swap.peak",
+                [CGROUP_MEMORY_ZSWAP_CURRENT] = "memory.zswap.current",
+        };
+
+        uint64_t bytes;
         int r;
 
         assert(u);
-        assert(mem_attribute);
-        assert(ret);
+        assert(metric >= 0);
+        assert(metric < _CGROUP_MEMORY_ACCOUNTING_METRIC_MAX);
 
         if (!UNIT_CGROUP_BOOL(u, memory_accounting))
                 return -ENODATA;
@@ -4057,64 +4068,36 @@ static int unit_get_memory_attr_raw(Unit *u, const char* mem_attribute, uint64_t
         if (unit_has_host_root_cgroup(u))
                 return -ENODATA;
 
-        if ((u->cgroup_realized_mask & CGROUP_MASK_MEMORY) == 0)
+        if (!FLAGS_SET(u->cgroup_realized_mask, CGROUP_MASK_MEMORY))
                 return -ENODATA;
 
         r = cg_all_unified();
         if (r < 0)
                 return r;
-        if (!r)
+        if (r == 0)
                 return -ENODATA;
 
-        return cg_get_attribute_as_uint64("memory", u->cgroup_path, mem_attribute, ret);
-}
-
-static int unit_get_memory_attr_cached(Unit *u, const char* mem_attribute, uint64_t* last, uint64_t *ret) {
-        uint64_t bytes;
-        int r;
-
-        assert(u);
-        assert(mem_attribute);
-        assert(last);
-
-        if (!UNIT_CGROUP_BOOL(u, memory_accounting))
-                return -ENODATA;
+        r = cg_get_attribute_as_uint64("memory", u->cgroup_path, attributes_table[metric], &bytes);
+        if (r < 0 && (r != -ENODATA || metric > _CGROUP_MEMORY_ACCOUNTING_METRIC_CACHED_LAST))
+                return r;
 
-        r = unit_get_memory_attr_raw(u, mem_attribute, &bytes);
-        if (r == -ENODATA && *last != UINT64_MAX) {
-                /* If we can't get the memory peak anymore (because the cgroup was already removed, for example),
-                 * use our cached value. */
+        if (metric <= _CGROUP_MEMORY_ACCOUNTING_METRIC_CACHED_LAST) {
+                uint64_t *last = &u->memory_accounting_last[metric];
 
-                if (ret)
-                        *ret = *last;
-                return 0;
+                if (r >= 0)
+                        *last = bytes;
+                else if (*last != UINT64_MAX)
+                        bytes = *last;
+                else
+                        return r;
         }
-        if (r < 0)
-                return r;
 
-        *last = bytes;
         if (ret)
                 *ret = bytes;
 
         return 0;
 }
 
-int unit_get_memory_peak(Unit *u, uint64_t *ret) {
-        return unit_get_memory_attr_cached(u, "memory.peak", &u->memory_peak_last, ret);
-}
-
-int unit_get_memory_swap_current(Unit *u, uint64_t *ret) {
-        return unit_get_memory_attr_raw(u, "memory.swap.current", ret);
-}
-
-int unit_get_memory_swap_peak(Unit *u, uint64_t *ret) {
-        return unit_get_memory_attr_cached(u, "memory.swap.peak", &u->memory_swap_peak_last, ret);
-}
-
-int unit_get_memory_zswap_current(Unit *u, uint64_t *ret) {
-        return unit_get_memory_attr_raw(u, "memory.zswap.current", ret);
-}
-
 int unit_get_tasks_current(Unit *u, uint64_t *ret) {
         assert(u);
         assert(ret);
@@ -4650,3 +4633,12 @@ static const char* const cgroup_io_accounting_metric_table[_CGROUP_IO_ACCOUNTING
 };
 
 DEFINE_STRING_TABLE_LOOKUP(cgroup_io_accounting_metric, CGroupIOAccountingMetric);
+
+static const char* const cgroup_memory_accounting_metric_table[_CGROUP_MEMORY_ACCOUNTING_METRIC_MAX] = {
+        [CGROUP_MEMORY_PEAK]          = "MemoryPeak",
+        [CGROUP_MEMORY_SWAP_CURRENT]  = "MemorySwapCurrent",
+        [CGROUP_MEMORY_SWAP_PEAK]     = "MemorySwapPeak",
+        [CGROUP_MEMORY_ZSWAP_CURRENT] = "MemoryZSwapCurrent",
+};
+
+DEFINE_STRING_TABLE_LOOKUP(cgroup_memory_accounting_metric, CGroupMemoryAccountingMetric);
diff --git a/src/core/cgroup.h b/src/core/cgroup.h
index 203575ca343..157ac7271f6 100644
--- a/src/core/cgroup.h
+++ b/src/core/cgroup.h
@@ -261,6 +261,21 @@ typedef enum CGroupIOAccountingMetric {
         _CGROUP_IO_ACCOUNTING_METRIC_INVALID = -EINVAL,
 } CGroupIOAccountingMetric;
 
+typedef enum CGroupMemoryAccountingMetric {
+        CGROUP_MEMORY_PEAK,
+        CGROUP_MEMORY_SWAP_PEAK,
+        /* We cache the above attributes, so that they can be fetched even after the cgroup is gone, e.g.
+         * when systemd-run exits. */
+        _CGROUP_MEMORY_ACCOUNTING_METRIC_CACHED_LAST = CGROUP_MEMORY_SWAP_PEAK,
+
+        /* These attributes are transient, so no need for caching. */
+        CGROUP_MEMORY_SWAP_CURRENT,
+        CGROUP_MEMORY_ZSWAP_CURRENT,
+
+        _CGROUP_MEMORY_ACCOUNTING_METRIC_MAX,
+        _CGROUP_MEMORY_ACCOUNTING_METRIC_INVALID = -EINVAL,
+} CGroupMemoryAccountingMetric;
+
 typedef struct Unit Unit;
 typedef struct Manager Manager;
 typedef enum ManagerState ManagerState;
@@ -352,12 +367,9 @@ int unit_watch_all_pids(Unit *u);
 
 int unit_synthesize_cgroup_empty_event(Unit *u);
 
-int unit_get_memory_current(Unit *u, uint64_t *ret);
-int unit_get_memory_peak(Unit *u, uint64_t *ret);
-int unit_get_memory_swap_current(Unit *u, uint64_t *ret);
-int unit_get_memory_swap_peak(Unit *u, uint64_t *ret);
-int unit_get_memory_zswap_current(Unit *u, uint64_t *ret);
 int unit_get_memory_available(Unit *u, uint64_t *ret);
+int unit_get_memory_current(Unit *u, uint64_t *ret);
+int unit_get_memory_accounting(Unit *u, CGroupMemoryAccountingMetric metric, uint64_t *ret);
 int unit_get_tasks_current(Unit *u, uint64_t *ret);
 int unit_get_cpu_usage(Unit *u, nsec_t *ret);
 int unit_get_io_accounting(Unit *u, CGroupIOAccountingMetric metric, bool allow_cache, uint64_t *ret);
@@ -410,3 +422,6 @@ CGroupIPAccountingMetric cgroup_ip_accounting_metric_from_string(const char *s)
 
 const char* cgroup_io_accounting_metric_to_string(CGroupIOAccountingMetric m) _const_;
 CGroupIOAccountingMetric cgroup_io_accounting_metric_from_string(const char *s) _pure_;
+
+const char* cgroup_memory_accounting_metric_to_string(CGroupMemoryAccountingMetric m) _const_;
+CGroupMemoryAccountingMetric cgroup_memory_accounting_metric_from_string(const char *s) _pure_;
diff --git a/src/core/dbus-unit.c b/src/core/dbus-unit.c
index e5293269e3b..36b1bfa066b 100644
--- a/src/core/dbus-unit.c
+++ b/src/core/dbus-unit.c
@@ -1080,53 +1080,7 @@ static int property_get_current_memory(
         return sd_bus_message_append(reply, "t", sz);
 }
 
-static int property_get_peak_memory(
-                sd_bus *bus,
-                const char *path,
-                const char *interface,
-                const char *property,
-                sd_bus_message *reply,
-                void *userdata,
-                sd_bus_error *error) {
-
-        uint64_t sz = UINT64_MAX;
-        Unit *u = ASSERT_PTR(userdata);
-        int r;
-
-        assert(bus);
-        assert(reply);
-
-        r = unit_get_memory_peak(u, &sz);
-        if (r < 0 && r != -ENODATA)
-                log_unit_warning_errno(u, r, "Failed to get memory.peak attribute: %m");
-
-        return sd_bus_message_append(reply, "t", sz);
-}
-
-static int property_get_current_swap_memory(
-                sd_bus *bus,
-                const char *path,
-                const char *interface,
-                const char *property,
-                sd_bus_message *reply,
-                void *userdata,
-                sd_bus_error *error) {
-
-        uint64_t sz = UINT64_MAX;
-        Unit *u = ASSERT_PTR(userdata);
-        int r;
-
-        assert(bus);
-        assert(reply);
-
-        r = unit_get_memory_swap_current(u, &sz);
-        if (r < 0 && r != -ENODATA)
-                log_unit_warning_errno(u, r, "Failed to get memory.swap.current attribute: %m");
-
-        return sd_bus_message_append(reply, "t", sz);
-}
-
-static int property_get_peak_swap_memory(
+static int property_get_available_memory(
                 sd_bus *bus,
                 const char *path,
                 const char *interface,
@@ -1142,14 +1096,14 @@ static int property_get_peak_swap_memory(
         assert(bus);
         assert(reply);
 
-        r = unit_get_memory_swap_peak(u, &sz);
+        r = unit_get_memory_available(u, &sz);
         if (r < 0 && r != -ENODATA)
-                log_unit_warning_errno(u, r, "Failed to get memory.swap.peak attribute: %m");
+                log_unit_warning_errno(u, r, "Failed to get total available memory from cgroup: %m");
 
         return sd_bus_message_append(reply, "t", sz);
 }
 
-static int property_get_current_zswap_memory(
+static int property_get_memory_accounting(
                 sd_bus *bus,
                 const char *path,
                 const char *interface,
@@ -1158,40 +1112,15 @@ static int property_get_current_zswap_memory(
                 void *userdata,
                 sd_bus_error *error) {
 
-        uint64_t sz = UINT64_MAX;
         Unit *u = ASSERT_PTR(userdata);
-        int r;
-
-        assert(bus);
-        assert(reply);
-
-        r = unit_get_memory_swap_current(u, &sz);
-        if (r < 0 && r != -ENODATA)
-                log_unit_warning_errno(u, r, "Failed to get memory.zswap.current attribute: %m");
-
-        return sd_bus_message_append(reply, "t", sz);
-}
-
-static int property_get_available_memory(
-                sd_bus *bus,
-                const char *path,
-                const char *interface,
-                const char *property,
-                sd_bus_message *reply,
-                void *userdata,
-                sd_bus_error *error) {
-
+        CGroupMemoryAccountingMetric metric;
         uint64_t sz = UINT64_MAX;
-        Unit *u = ASSERT_PTR(userdata);
-        int r;
 
         assert(bus);
         assert(reply);
 
-        r = unit_get_memory_available(u, &sz);
-        if (r < 0 && r != -ENODATA)
-                log_unit_warning_errno(u, r, "Failed to get total available memory from cgroup: %m");
-
+        assert_se((metric = cgroup_memory_accounting_metric_from_string(property)) >= 0);
+        (void) unit_get_memory_accounting(u, metric, &sz);
         return sd_bus_message_append(reply, "t", sz);
 }
 
@@ -1628,10 +1557,10 @@ const sd_bus_vtable bus_unit_cgroup_vtable[] = {
         SD_BUS_PROPERTY("ControlGroup", "s", property_get_cgroup, 0, 0),
         SD_BUS_PROPERTY("ControlGroupId", "t", NULL, offsetof(Unit, cgroup_id), 0),
         SD_BUS_PROPERTY("MemoryCurrent", "t", property_get_current_memory, 0, 0),
-        SD_BUS_PROPERTY("MemoryPeak", "t", property_get_peak_memory, 0, 0),
-        SD_BUS_PROPERTY("MemorySwapCurrent", "t", property_get_current_swap_memory, 0, 0),
-        SD_BUS_PROPERTY("MemorySwapPeak", "t", property_get_peak_swap_memory, 0, 0),
-        SD_BUS_PROPERTY("MemoryZSwapCurrent", "t", property_get_current_zswap_memory, 0, 0),
+        SD_BUS_PROPERTY("MemoryPeak", "t", property_get_memory_accounting, 0, 0),
+        SD_BUS_PROPERTY("MemorySwapCurrent", "t", property_get_memory_accounting, 0, 0),
+        SD_BUS_PROPERTY("MemorySwapPeak", "t", property_get_memory_accounting, 0, 0),
+        SD_BUS_PROPERTY("MemoryZSwapCurrent", "t", property_get_memory_accounting, 0, 0),
         SD_BUS_PROPERTY("MemoryAvailable", "t", property_get_available_memory, 0, 0),
         SD_BUS_PROPERTY("CPUUsageNSec", "t", property_get_cpu_usage, 0, 0),
         SD_BUS_PROPERTY("EffectiveCPUs", "ay", property_get_cpuset_cpus, 0, 0),
diff --git a/src/core/unit-serialize.c b/src/core/unit-serialize.c
index d5361905b83..af3e1c20899 100644
--- a/src/core/unit-serialize.c
+++ b/src/core/unit-serialize.c
@@ -96,6 +96,13 @@ static const char* const io_accounting_metric_field_last_table[_CGROUP_IO_ACCOUN
 
 DEFINE_PRIVATE_STRING_TABLE_LOOKUP(io_accounting_metric_field_last, CGroupIOAccountingMetric);
 
+static const char* const memory_accounting_metric_field_last_table[_CGROUP_MEMORY_ACCOUNTING_METRIC_CACHED_LAST + 1] = {
+        [CGROUP_MEMORY_PEAK]      = "memory-accounting-peak",
+        [CGROUP_MEMORY_SWAP_PEAK] = "memory-accounting-swap-peak",
+};
+
+DEFINE_PRIVATE_STRING_TABLE_LOOKUP(memory_accounting_metric_field_last, CGroupMemoryAccountingMetric);
+
 int unit_serialize_state(Unit *u, FILE *f, FDSet *fds, bool switching_root) {
         int r;
 
@@ -165,6 +172,14 @@ int unit_serialize_state(Unit *u, FILE *f, FDSet *fds, bool switching_root) {
                         (void) serialize_item_format(f, io_accounting_metric_field_last_to_string(im), "%" PRIu64, u->io_accounting_last[im]);
         }
 
+        for (CGroupMemoryAccountingMetric metric = 0; metric <= _CGROUP_MEMORY_ACCOUNTING_METRIC_CACHED_LAST; metric++) {
+                uint64_t v;
+
+                r = unit_get_memory_accounting(u, metric, &v);
+                if (r >= 0)
+                        (void) serialize_item_format(f, memory_accounting_metric_field_last_to_string(metric), "%" PRIu64, v);
+        }
+
         if (u->cgroup_path)
                 (void) serialize_item(f, "cgroup", u->cgroup_path);
 
@@ -474,6 +489,18 @@ int unit_deserialize_state(Unit *u, FILE *f, FDSet *fds) {
                         continue;
                 }
 
+                m = memory_accounting_metric_field_last_from_string(l);
+                if (m >= 0) {
+                        uint64_t c;
+
+                        r = safe_atou64(v, &c);
+                        if (r < 0)
+                                log_unit_debug(u, "Failed to parse memory accounting last value %s, ignoring.", v);
+                        else
+                                u->memory_accounting_last[m] = c;
+                        continue;
+                }
+
                 /* Check if this is an IP accounting metric serialization field */
                 m = ip_accounting_metric_field_from_string(l);
                 if (m >= 0) {
diff --git a/src/core/unit.c b/src/core/unit.c
index 381241f02de..b88e28d77ed 100644
--- a/src/core/unit.c
+++ b/src/core/unit.c
@@ -114,8 +114,10 @@ Unit* unit_new(Manager *m, size_t size) {
         u->ref_uid = UID_INVALID;
         u->ref_gid = GID_INVALID;
         u->cpu_usage_last = NSEC_INFINITY;
-        u->memory_peak_last = UINT64_MAX;
-        u->memory_swap_peak_last = UINT64_MAX;
+
+        FOREACH_ARRAY(i, u->memory_accounting_last, ELEMENTSOF(u->memory_accounting_last))
+                *i = UINT64_MAX;
+
         u->cgroup_invalidated_mask |= CGROUP_MASK_BPF_FIREWALL;
         u->failure_action_exit_status = u->success_action_exit_status = -1;
 
@@ -2372,7 +2374,7 @@ static int unit_log_resources(Unit *u) {
                                         nsec > NOTICEWORTHY_CPU_NSEC);
         }
 
-        (void) unit_get_memory_peak(u, &memory_peak);
+        (void) unit_get_memory_accounting(u, CGROUP_MEMORY_PEAK, &memory_peak);
         if (memory_peak != UINT64_MAX) {
                 /* Format peak memory for inclusion in the structured log message */
                 if (asprintf(&t, "MEMORY_PEAK=%" PRIu64, memory_peak) < 0) {
@@ -2390,7 +2392,7 @@ static int unit_log_resources(Unit *u) {
                 message_parts[n_message_parts++] = t;
         }
 
-        (void) unit_get_memory_swap_peak(u, &memory_swap_peak);
+        (void) unit_get_memory_accounting(u, CGROUP_MEMORY_SWAP_PEAK, &memory_swap_peak);
         if (memory_swap_peak != UINT64_MAX) {
                 /* Format peak swap memory for inclusion in the structured log message */
                 if (asprintf(&t, "MEMORY_SWAP_PEAK=%" PRIu64, memory_swap_peak) < 0) {
diff --git a/src/core/unit.h b/src/core/unit.h
index ded2f9d7d54..60bc2e3d350 100644
--- a/src/core/unit.h
+++ b/src/core/unit.h
@@ -365,11 +365,8 @@ typedef struct Unit {
         nsec_t cpu_usage_base;
         nsec_t cpu_usage_last; /* the most recently read value */
 
-        /* Most recently read value of memory.peak */
-        uint64_t memory_peak_last;
-
-        /* Most recently read value of memory.swap.peak */
-        uint64_t memory_swap_peak_last;
+        /* Most recently read value of memory accounting metrics */
+        uint64_t memory_accounting_last[_CGROUP_MEMORY_ACCOUNTING_METRIC_CACHED_LAST + 1];
 
         /* The current counter of OOM kills initiated by systemd-oomd */
         uint64_t managed_oom_kill_last;