From 35c08a56a1cc217e78fc0cbfa69fb30ffb9d43cc Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Fri, 24 Nov 2023 19:12:06 +0800 Subject: [PATCH 1/4] core/dbus-unit: don't log cgroup v1 property name --- src/core/dbus-unit.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/dbus-unit.c b/src/core/dbus-unit.c index 36b1bfa066..1a037b7035 100644 --- a/src/core/dbus-unit.c +++ b/src/core/dbus-unit.c @@ -1075,7 +1075,7 @@ static int property_get_current_memory( r = unit_get_memory_current(u, &sz); if (r < 0 && r != -ENODATA) - log_unit_warning_errno(u, r, "Failed to get memory.usage_in_bytes attribute: %m"); + log_unit_warning_errno(u, r, "Failed to get current memory usage from cgroup: %m"); return sd_bus_message_append(reply, "t", sz); } From 3f362012ce0034dc14d3c1a1c2a3a64a11efa9da Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Fri, 24 Nov 2023 19:15:40 +0800 Subject: [PATCH 2/4] bus-print-properties: ignore CGROUP_LIMIT_MAX for Memory*{Current,Peak} MemoryCurrent and MemoryAvailable are shown as "[not set]" when UINT64_MAX (unset). Let's do the same for the newly-added Memory*{Current,Peak} properties. --- src/shared/bus-print-properties.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/shared/bus-print-properties.c b/src/shared/bus-print-properties.c index 8999a1a4fa..62b9cf1bf4 100644 --- a/src/shared/bus-print-properties.c +++ b/src/shared/bus-print-properties.c @@ -158,6 +158,7 @@ static int bus_print_property(const char *name, const char *expected_value, sd_b (STR_IN_SET(name, "CPUShares", "StartupCPUShares") && u == CGROUP_CPU_SHARES_INVALID) || (STR_IN_SET(name, "BlockIOWeight", "StartupBlockIOWeight") && u == CGROUP_BLKIO_WEIGHT_INVALID) || (STR_IN_SET(name, "MemoryCurrent", "MemoryAvailable", "TasksCurrent") && u == UINT64_MAX) || + (startswith(name, "Memory") && ENDSWITH_SET(name, "Current", "Peak") && u == CGROUP_LIMIT_MAX) || (endswith(name, "NSec") && u == UINT64_MAX)) bus_print_property_value(name, expected_value, flags, "[not set]"); From bfb6b1214a8da947cb82fed2eec3d7f2b1c6175f Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Fri, 24 Nov 2023 23:31:55 +0800 Subject: [PATCH 3/4] bus-print-properties: prettify more unset properties --- src/shared/bus-print-properties.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/shared/bus-print-properties.c b/src/shared/bus-print-properties.c index 62b9cf1bf4..6704e1ef3d 100644 --- a/src/shared/bus-print-properties.c +++ b/src/shared/bus-print-properties.c @@ -159,11 +159,13 @@ static int bus_print_property(const char *name, const char *expected_value, sd_b (STR_IN_SET(name, "BlockIOWeight", "StartupBlockIOWeight") && u == CGROUP_BLKIO_WEIGHT_INVALID) || (STR_IN_SET(name, "MemoryCurrent", "MemoryAvailable", "TasksCurrent") && u == UINT64_MAX) || (startswith(name, "Memory") && ENDSWITH_SET(name, "Current", "Peak") && u == CGROUP_LIMIT_MAX) || + (startswith(name, "IO") && ENDSWITH_SET(name, "Bytes", "Operations") && u == UINT64_MAX) || (endswith(name, "NSec") && u == UINT64_MAX)) bus_print_property_value(name, expected_value, flags, "[not set]"); - else if ((STR_IN_SET(name, "DefaultMemoryLow", "DefaultMemoryMin", "MemoryLow", "MemoryHigh", "MemoryMax", "MemorySwapMax", "MemoryZSwapMax", "MemoryLimit") && u == CGROUP_LIMIT_MAX) || + else if ((ENDSWITH_SET(name, "MemoryLow", "MemoryMin", "MemoryHigh", "MemoryMax", "MemorySwapMax", "MemoryZSwapMax", "MemoryLimit") && + u == CGROUP_LIMIT_MAX) || (STR_IN_SET(name, "TasksMax", "DefaultTasksMax") && u == UINT64_MAX) || (startswith(name, "Limit") && u == UINT64_MAX) || (startswith(name, "DefaultLimit") && u == UINT64_MAX)) From f17b07f4d72238da95312920dcc2ad076568cba3 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Fri, 24 Nov 2023 23:20:41 +0800 Subject: [PATCH 4/4] core/cgroup: use the cached memory accounting value when cgroup is gone Follow-up for 9824ab1f009e99b0b9d273ace4c98cc687a4c1d7 Fixes https://github.com/systemd/systemd/issues/28542#issuecomment-1825413237 --- src/core/cgroup.c | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/src/core/cgroup.c b/src/core/cgroup.c index 5b0cb15c85..10678dc669 100644 --- a/src/core/cgroup.c +++ b/src/core/cgroup.c @@ -4052,6 +4052,7 @@ int unit_get_memory_accounting(Unit *u, CGroupMemoryAccountingMetric metric, uin }; uint64_t bytes; + bool updated = false; int r; assert(u); @@ -4062,7 +4063,8 @@ int unit_get_memory_accounting(Unit *u, CGroupMemoryAccountingMetric metric, uin return -ENODATA; if (!u->cgroup_path) - return -ENODATA; + /* If the cgroup is already gone, we try to find the last cached value. */ + goto cache; /* The root cgroup doesn't expose this information. */ if (unit_has_host_root_cgroup(u)) @@ -4078,19 +4080,22 @@ int unit_get_memory_accounting(Unit *u, CGroupMemoryAccountingMetric metric, uin 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)) + if (r < 0 && r != -ENODATA) return r; + updated = r >= 0; - if (metric <= _CGROUP_MEMORY_ACCOUNTING_METRIC_CACHED_LAST) { - uint64_t *last = &u->memory_accounting_last[metric]; +cache: + if (metric > _CGROUP_MEMORY_ACCOUNTING_METRIC_CACHED_LAST) + return -ENODATA; - if (r >= 0) - *last = bytes; - else if (*last != UINT64_MAX) - bytes = *last; - else - return r; - } + uint64_t *last = &u->memory_accounting_last[metric]; + + if (updated) + *last = bytes; + else if (*last != UINT64_MAX) + bytes = *last; + else + return -ENODATA; if (ret) *ret = bytes;