From 3565c709f587a3d29d412d81355f8dd9d565a39e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Koutn=C3=BD?= Date: Thu, 7 Sep 2023 18:50:08 +0200 Subject: [PATCH 1/4] cgroup: Fix MemoryAvailable= by considering physical memory Currently, querying a unit's available memory would result in infinity if there are no limits set on the unit or ancestors. That undermines semantics implied by the name, so look at the physical memory if the search propagates up to the -.slice. This makes sense even in systemd user instances, limits of -.slice are still looked at too. Also change printed representation of infinite MemoryAvailable which means we could not figure out a good estimate. --- src/core/cgroup.c | 9 ++++++--- src/shared/bus-print-properties.c | 4 ++-- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/core/cgroup.c b/src/core/cgroup.c index 91ef33b12c9..a5c8687252c 100644 --- a/src/core/cgroup.c +++ b/src/core/cgroup.c @@ -3791,7 +3791,7 @@ int unit_get_memory_available(Unit *u, uint64_t *ret) { available = LESS_BY(MIN(unit_context->memory_max, unit_context->memory_high), unit_current); for (Unit *slice = UNIT_GET_SLICE(u); slice; slice = UNIT_GET_SLICE(slice)) { - uint64_t slice_current, slice_available = UINT64_MAX; + uint64_t slice_current, slice_available, slice_limit = UINT64_MAX; CGroupContext *slice_context; /* No point in continuing if we can't go any lower */ @@ -3805,14 +3805,17 @@ int unit_get_memory_available(Unit *u, uint64_t *ret) { if (!slice_context) continue; - if (slice_context->memory_max == UINT64_MAX && slice_context->memory_high == UINT64_MAX) + if (unit_has_name(slice, SPECIAL_ROOT_SLICE)) + slice_limit = physical_memory(); + else if (slice_context->memory_max == UINT64_MAX && slice_context->memory_high == UINT64_MAX) continue; + slice_limit = MIN3(slice_limit, slice_context->memory_max, slice_context->memory_high); r = cg_get_attribute_as_uint64("memory", slice->cgroup_path, memory_file, &slice_current); if (r < 0) continue; - slice_available = LESS_BY(MIN(slice_context->memory_max, slice_context->memory_high), slice_current); + slice_available = LESS_BY(slice_limit, slice_current); available = MIN(slice_available, available); } diff --git a/src/shared/bus-print-properties.c b/src/shared/bus-print-properties.c index db41ad24957..8999a1a4fad 100644 --- a/src/shared/bus-print-properties.c +++ b/src/shared/bus-print-properties.c @@ -157,12 +157,12 @@ static int bus_print_property(const char *name, const char *expected_value, sd_b else if ((STR_IN_SET(name, "CPUWeight", "StartupCPUWeight", "IOWeight", "StartupIOWeight") && u == CGROUP_WEIGHT_INVALID) || (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", "TasksCurrent") && u == UINT64_MAX) || + (STR_IN_SET(name, "MemoryCurrent", "MemoryAvailable", "TasksCurrent") && 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", "MemoryAvailable") && u == CGROUP_LIMIT_MAX) || + else if ((STR_IN_SET(name, "DefaultMemoryLow", "DefaultMemoryMin", "MemoryLow", "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 727cea7652c978c5ef2cffa682d163be3a9f7768 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Koutn=C3=BD?= Date: Thu, 7 Sep 2023 19:27:52 +0200 Subject: [PATCH 2/4] cgroup: Refactor MemoryAvailable= evaluation unit_get_memory_available() duplicates similar logic contained in unit_get_memory_current(). Instead, it can call it for each unit it needs data for. Additionally, simplify the flow by treating all units from leaf to root uniformly in one loop. Functional change when a queried unit does not have MemoryAccounting=yes (or cgroup_path), we will try getting an estimate from ancestors. --- src/core/cgroup.c | 63 +++++++++++++---------------------------------- 1 file changed, 17 insertions(+), 46 deletions(-) diff --git a/src/core/cgroup.c b/src/core/cgroup.c index a5c8687252c..9fd62b2b5b9 100644 --- a/src/core/cgroup.c +++ b/src/core/cgroup.c @@ -3751,9 +3751,7 @@ int manager_notify_cgroup_empty(Manager *m, const char *cgroup) { } int unit_get_memory_available(Unit *u, uint64_t *ret) { - uint64_t unit_current, available = UINT64_MAX; - CGroupContext *unit_context; - const char *memory_file; + uint64_t available = UINT64_MAX; int r; assert(u); @@ -3763,61 +3761,34 @@ int unit_get_memory_available(Unit *u, uint64_t *ret) { * claim before hitting the configured cgroup limits (if any). Consider both MemoryHigh * and MemoryMax, and also any slice the unit might be nested below. */ - if (!UNIT_CGROUP_BOOL(u, memory_accounting)) - return -ENODATA; - - if (!u->cgroup_path) - return -ENODATA; - - /* The root cgroup doesn't expose this information */ - if (unit_has_host_root_cgroup(u)) - return -ENODATA; - - if ((u->cgroup_realized_mask & CGROUP_MASK_MEMORY) == 0) - return -ENODATA; - - r = cg_all_unified(); - if (r < 0) - return r; - memory_file = r > 0 ? "memory.current" : "memory.usage_in_bytes"; - - r = cg_get_attribute_as_uint64("memory", u->cgroup_path, memory_file, &unit_current); - if (r < 0) - return r; - - assert_se(unit_context = unit_get_cgroup_context(u)); - - if (unit_context->memory_max != UINT64_MAX || unit_context->memory_high != UINT64_MAX) - available = LESS_BY(MIN(unit_context->memory_max, unit_context->memory_high), unit_current); - - for (Unit *slice = UNIT_GET_SLICE(u); slice; slice = UNIT_GET_SLICE(slice)) { - uint64_t slice_current, slice_available, slice_limit = UINT64_MAX; - CGroupContext *slice_context; + do { + uint64_t unit_current, unit_available, unit_limit = UINT64_MAX; + CGroupContext *unit_context; /* No point in continuing if we can't go any lower */ if (available == 0) break; - if (!slice->cgroup_path) + unit_context = unit_get_cgroup_context(u); + if (!unit_context) + return -ENODATA; + + if (!u->cgroup_path) continue; - slice_context = unit_get_cgroup_context(slice); - if (!slice_context) + if (unit_has_name(u, SPECIAL_ROOT_SLICE)) + unit_limit = physical_memory(); + else if (unit_context->memory_max == UINT64_MAX && unit_context->memory_high == UINT64_MAX) continue; + unit_limit = MIN3(unit_limit, unit_context->memory_max, unit_context->memory_high); - if (unit_has_name(slice, SPECIAL_ROOT_SLICE)) - slice_limit = physical_memory(); - else if (slice_context->memory_max == UINT64_MAX && slice_context->memory_high == UINT64_MAX) - continue; - slice_limit = MIN3(slice_limit, slice_context->memory_max, slice_context->memory_high); - - r = cg_get_attribute_as_uint64("memory", slice->cgroup_path, memory_file, &slice_current); + r = unit_get_memory_current(u, &unit_current); if (r < 0) continue; - slice_available = LESS_BY(slice_limit, slice_current); - available = MIN(slice_available, available); - } + unit_available = LESS_BY(unit_limit, unit_current); + available = MIN(unit_available, available); + } while ((u = UNIT_GET_SLICE(u))); *ret = available; From 8db929a1e235794053853a0b0f720e509dde75d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Koutn=C3=BD?= Date: Thu, 7 Sep 2023 19:48:48 +0200 Subject: [PATCH 3/4] cgroup: Estimate MemoryAvailable= when DefaultMemoryAccounting=no Without memory accounting explicitly disabled, we may not obtain current consumption from all units on the ancestry path. Use a descendant value as lower bound estimate for ancestors if ancestor's consumption cannot be directly queried. This makes MemoryAvailable= an upper bound of available values. --- src/core/cgroup.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/core/cgroup.c b/src/core/cgroup.c index 9fd62b2b5b9..b304b39e8c1 100644 --- a/src/core/cgroup.c +++ b/src/core/cgroup.c @@ -3751,8 +3751,7 @@ int manager_notify_cgroup_empty(Manager *m, const char *cgroup) { } int unit_get_memory_available(Unit *u, uint64_t *ret) { - uint64_t available = UINT64_MAX; - int r; + uint64_t available = UINT64_MAX, current = 0; assert(u); assert(ret); @@ -3762,7 +3761,7 @@ int unit_get_memory_available(Unit *u, uint64_t *ret) { * and MemoryMax, and also any slice the unit might be nested below. */ do { - uint64_t unit_current, unit_available, unit_limit = UINT64_MAX; + uint64_t unit_available, unit_limit = UINT64_MAX; CGroupContext *unit_context; /* No point in continuing if we can't go any lower */ @@ -3776,17 +3775,16 @@ int unit_get_memory_available(Unit *u, uint64_t *ret) { if (!u->cgroup_path) continue; + (void) unit_get_memory_current(u, ¤t); + /* in case of error, previous current propagates as lower bound */ + if (unit_has_name(u, SPECIAL_ROOT_SLICE)) unit_limit = physical_memory(); else if (unit_context->memory_max == UINT64_MAX && unit_context->memory_high == UINT64_MAX) continue; unit_limit = MIN3(unit_limit, unit_context->memory_max, unit_context->memory_high); - r = unit_get_memory_current(u, &unit_current); - if (r < 0) - continue; - - unit_available = LESS_BY(unit_limit, unit_current); + unit_available = LESS_BY(unit_limit, current); available = MIN(unit_available, available); } while ((u = UNIT_GET_SLICE(u))); From 055665d596477c39eeb9ba0af8a8c40694a877f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Koutn=C3=BD?= Date: Thu, 7 Sep 2023 20:00:28 +0200 Subject: [PATCH 4/4] dbus: Document org.freedesktop.systemd1.Service.MemoryAvailable property The value is an optimistic estimate, make it clear in the docs. --- man/org.freedesktop.systemd1.xml | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/man/org.freedesktop.systemd1.xml b/man/org.freedesktop.systemd1.xml index 75d3d457bd3..4ca0583d655 100644 --- a/man/org.freedesktop.systemd1.xml +++ b/man/org.freedesktop.systemd1.xml @@ -4603,10 +4603,11 @@ node /org/freedesktop/systemd1/unit/avahi_2ddaemon_2eservice { ExtensionDirectories see systemd.exec(5) for their meaning. - MemoryAvailable indicates how much unused memory is available to the unit before - the MemoryMax or MemoryHigh (whichever is lower) limit set by the cgroup - memory controller is reached. It will take into consideration limits on all parent slices, other than the - limits set on the unit itself. + MemoryAvailable takes into account unit's and parents' MemoryMax + or MemoryHigh or physically available RAM versus given level's memory consumption + and takes minimum. Beware that other units below the tightest parent slice may consume the memory quicker + and less than reported value would remain for own allocation. + It works better in conjunction with MemoryAccounting=yes on involved units. DelegateSubgroup contains the cgroup subgroup to place invoked unit processes in. As configured by the option of the same name in unit files. This is set to the empty string when it