From 7c9d2b79935d413389a603918a711df75acd3f48 Mon Sep 17 00:00:00 2001 From: Chris Down Date: Mon, 30 Sep 2019 18:24:26 +0100 Subject: [PATCH 1/3] cgroup: Check ancestor memory min for unified memory config Otherwise we might not enable it when we should, ie. DefaultMemoryMin is set in a parent, but not MemoryMin in the current unit. --- src/core/cgroup.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/cgroup.c b/src/core/cgroup.c index 423742969e..bcf06b0e60 100644 --- a/src/core/cgroup.c +++ b/src/core/cgroup.c @@ -955,7 +955,7 @@ static bool unit_has_unified_memory_config(Unit *u) { c = unit_get_cgroup_context(u); assert(c); - return c->memory_min > 0 || unit_get_ancestor_memory_low(u) > 0 || + return unit_get_ancestor_memory_min(u) > 0 || unit_get_ancestor_memory_low(u) > 0 || c->memory_high != CGROUP_LIMIT_MAX || c->memory_max != CGROUP_LIMIT_MAX || c->memory_swap_max != CGROUP_LIMIT_MAX; } From 64fe532e90b3e99bf7821ded8a1107c239099e40 Mon Sep 17 00:00:00 2001 From: Chris Down Date: Mon, 30 Sep 2019 18:25:09 +0100 Subject: [PATCH 2/3] cgroup: Respect DefaultMemoryMin when setting memory.min This is an oversight from https://github.com/systemd/systemd/pull/12332. Sadly the tests didn't catch it since it requires a real cgroup hierarchy to see, and it wasn't seen in prod since we're only currently using DefaultMemoryLow, not DefaultMemoryMin. :-( --- src/core/cgroup.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/cgroup.c b/src/core/cgroup.c index bcf06b0e60..74579a0a19 100644 --- a/src/core/cgroup.c +++ b/src/core/cgroup.c @@ -1227,7 +1227,7 @@ static void cgroup_context_apply( log_cgroup_compat(u, "Applying MemoryLimit=%" PRIu64 " as MemoryMax=", max); } - cgroup_apply_unified_memory_limit(u, "memory.min", c->memory_min); + cgroup_apply_unified_memory_limit(u, "memory.min", unit_get_ancestor_memory_min(u)); cgroup_apply_unified_memory_limit(u, "memory.low", unit_get_ancestor_memory_low(u)); cgroup_apply_unified_memory_limit(u, "memory.high", c->memory_high); cgroup_apply_unified_memory_limit(u, "memory.max", max); From 184e989d7da4648bd36511ffa28a9f2b469589d1 Mon Sep 17 00:00:00 2001 From: Chris Down Date: Mon, 30 Sep 2019 18:36:13 +0100 Subject: [PATCH 3/3] cgroup: Mark memory protections as explicitly set in transient units A later version of the DefaultMemory{Low,Min} patch changed these to require explicitly setting memory_foo_set, but we only set that in load-fragment, not dbus-cgroup. Without these, we may fall back to either DefaultMemoryFoo or CGROUP_LIMIT_MIN when we really shouldn't. --- src/core/dbus-cgroup.c | 64 +++++++++++++++++++++++++++++++----------- 1 file changed, 48 insertions(+), 16 deletions(-) diff --git a/src/core/dbus-cgroup.c b/src/core/dbus-cgroup.c index bd0e73befc..00b71298dc 100644 --- a/src/core/dbus-cgroup.c +++ b/src/core/dbus-cgroup.c @@ -772,17 +772,33 @@ int bus_cgroup_set_property( if (streq(name, "MemoryAccounting")) return bus_cgroup_set_boolean(u, name, &c->memory_accounting, CGROUP_MASK_MEMORY, message, flags, error); - if (streq(name, "MemoryMin")) - return bus_cgroup_set_memory_protection(u, name, &c->memory_min, message, flags, error); + if (streq(name, "MemoryMin")) { + r = bus_cgroup_set_memory_protection(u, name, &c->memory_min, message, flags, error); + if (r > 0) + c->memory_min_set = true; + return r; + } - if (streq(name, "MemoryLow")) - return bus_cgroup_set_memory_protection(u, name, &c->memory_low, message, flags, error); + if (streq(name, "MemoryLow")) { + r = bus_cgroup_set_memory_protection(u, name, &c->memory_low, message, flags, error); + if (r > 0) + c->memory_low_set = true; + return r; + } - if (streq(name, "DefaultMemoryMin")) - return bus_cgroup_set_memory_protection(u, name, &c->default_memory_min, message, flags, error); + if (streq(name, "DefaultMemoryMin")) { + r = bus_cgroup_set_memory_protection(u, name, &c->default_memory_min, message, flags, error); + if (r > 0) + c->default_memory_min_set = true; + return r; + } - if (streq(name, "DefaultMemoryLow")) - return bus_cgroup_set_memory_protection(u, name, &c->default_memory_low, message, flags, error); + if (streq(name, "DefaultMemoryLow")) { + r = bus_cgroup_set_memory_protection(u, name, &c->default_memory_low, message, flags, error); + if (r > 0) + c->default_memory_low_set = true; + return r; + } if (streq(name, "MemoryHigh")) return bus_cgroup_set_memory(u, name, &c->memory_high, message, flags, error); @@ -796,17 +812,33 @@ int bus_cgroup_set_property( if (streq(name, "MemoryLimit")) return bus_cgroup_set_memory(u, name, &c->memory_limit, message, flags, error); - if (streq(name, "MemoryMinScale")) - return bus_cgroup_set_memory_protection_scale(u, name, &c->memory_min, message, flags, error); + if (streq(name, "MemoryMinScale")) { + r = bus_cgroup_set_memory_protection_scale(u, name, &c->memory_min, message, flags, error); + if (r > 0) + c->memory_min_set = true; + return r; + } - if (streq(name, "MemoryLowScale")) - return bus_cgroup_set_memory_protection_scale(u, name, &c->memory_low, message, flags, error); + if (streq(name, "MemoryLowScale")) { + r = bus_cgroup_set_memory_protection_scale(u, name, &c->memory_low, message, flags, error); + if (r > 0) + c->memory_low_set = true; + return r; + } - if (streq(name, "DefaultMemoryMinScale")) - return bus_cgroup_set_memory_protection_scale(u, name, &c->default_memory_min, message, flags, error); + if (streq(name, "DefaultMemoryMinScale")) { + r = bus_cgroup_set_memory_protection_scale(u, name, &c->default_memory_min, message, flags, error); + if (r > 0) + c->default_memory_min_set = true; + return r; + } - if (streq(name, "DefaultMemoryLowScale")) - return bus_cgroup_set_memory_protection_scale(u, name, &c->default_memory_low, message, flags, error); + if (streq(name, "DefaultMemoryLowScale")) { + r = bus_cgroup_set_memory_protection_scale(u, name, &c->default_memory_low, message, flags, error); + if (r > 0) + c->default_memory_low_set = true; + return r; + } if (streq(name, "MemoryHighScale")) return bus_cgroup_set_memory_scale(u, name, &c->memory_high, message, flags, error);