From acdb4b5236f38bbefbcc4a47fdbb9cd558b4b5c5 Mon Sep 17 00:00:00 2001 From: Chris Down Date: Tue, 30 Apr 2019 14:22:04 -0400 Subject: [PATCH 1/4] cgroup: Polish hierarchically aware protection docs a bit I missed adding a section in `systemd.resource-control` about DefaultMemoryMin in #12332. Also, add a NEWS entry going over the general concept. --- NEWS | 5 +++++ docs/TRANSIENT-SETTINGS.md | 1 + man/systemd.resource-control.xml | 8 ++++++-- 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/NEWS b/NEWS index 057484cbae..57ed27a61d 100644 --- a/NEWS +++ b/NEWS @@ -19,6 +19,11 @@ CHANGES WITH 243 in spe: are harder to type, but we believe the change from 5 digit PIDs to 7 digit PIDs is not too hampering for usability. + * MemoryLow and MemoryMin gained hierarchy-aware counterparts, + DefaultMemoryLow and DefaultMemoryMin, which can be used to + hierarchically set default memory protection values for a particular + subtree of the unit hierarchy. + … CHANGES WITH 242: diff --git a/docs/TRANSIENT-SETTINGS.md b/docs/TRANSIENT-SETTINGS.md index 3aa68c0a26..469ef7ce31 100644 --- a/docs/TRANSIENT-SETTINGS.md +++ b/docs/TRANSIENT-SETTINGS.md @@ -227,6 +227,7 @@ All cgroup/resource control settings are available for transient units ✓ CPUQuota= ✓ CPUQuotaPeriodSec= ✓ MemoryAccounting= +✓ DefaultMemoryMin= ✓ MemoryMin= ✓ DefaultMemoryLow= ✓ MemoryLow= diff --git a/man/systemd.resource-control.xml b/man/systemd.resource-control.xml index e7fb46873c..95209a8a6a 100644 --- a/man/systemd.resource-control.xml +++ b/man/systemd.resource-control.xml @@ -245,6 +245,10 @@ This setting is supported only if the unified control group hierarchy is used and disables MemoryLimit=. + + Units may have their children use a default memory.min value by specifying + DefaultMemoryMin=, which has the same semantics as MemoryMin=. This setting + does not affect memory.min in the unit itself. @@ -266,8 +270,8 @@ This setting is supported only if the unified control group hierarchy is used and disables MemoryLimit=. - Units may can have their children use a default memory.low value by specifying - DefaultMemoryLow=, which has the same usage as MemoryLow=. This setting + Units may have their children use a default memory.low value by specifying + DefaultMemoryLow=, which has the same semantics as MemoryLow=. This setting does not affect memory.low in the unit itself. From 7e7223b3d57c950b399352a92e1d817f7c463602 Mon Sep 17 00:00:00 2001 From: Chris Down Date: Fri, 3 May 2019 08:19:05 -0400 Subject: [PATCH 2/4] cgroup: Readd some plumbing for DefaultMemoryMin Somehow these got lost in the previous PR, rendering DefaultMemoryMin not very useful. --- src/core/dbus-cgroup.c | 1 + src/core/load-fragment-gperf.gperf.m4 | 1 + src/shared/bus-unit-util.c | 2 +- src/shared/bus-util.c | 2 +- 4 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/core/dbus-cgroup.c b/src/core/dbus-cgroup.c index c427c3cafe..4d2bd3d5e1 100644 --- a/src/core/dbus-cgroup.c +++ b/src/core/dbus-cgroup.c @@ -348,6 +348,7 @@ const sd_bus_vtable bus_cgroup_vtable[] = { SD_BUS_PROPERTY("BlockIOWriteBandwidth", "a(st)", property_get_blockio_device_bandwidths, 0, 0), SD_BUS_PROPERTY("MemoryAccounting", "b", bus_property_get_bool, offsetof(CGroupContext, memory_accounting), 0), SD_BUS_PROPERTY("DefaultMemoryLow", "t", NULL, offsetof(CGroupContext, default_memory_low), 0), + SD_BUS_PROPERTY("DefaultMemoryMin", "t", NULL, offsetof(CGroupContext, default_memory_min), 0), SD_BUS_PROPERTY("MemoryMin", "t", NULL, offsetof(CGroupContext, memory_min), 0), SD_BUS_PROPERTY("MemoryLow", "t", NULL, offsetof(CGroupContext, memory_low), 0), SD_BUS_PROPERTY("MemoryHigh", "t", NULL, offsetof(CGroupContext, memory_high), 0), diff --git a/src/core/load-fragment-gperf.gperf.m4 b/src/core/load-fragment-gperf.gperf.m4 index 13e9bbbfb1..b868a367f1 100644 --- a/src/core/load-fragment-gperf.gperf.m4 +++ b/src/core/load-fragment-gperf.gperf.m4 @@ -172,6 +172,7 @@ $1.CPUQuota, config_parse_cpu_quota, 0, $1.CPUQuotaPeriodSec, config_parse_sec_def_infinity, 0, offsetof($1, cgroup_context.cpu_quota_period_usec) $1.MemoryAccounting, config_parse_bool, 0, offsetof($1, cgroup_context.memory_accounting) $1.MemoryMin, config_parse_memory_limit, 0, offsetof($1, cgroup_context) +$1.DefaultMemoryMin, config_parse_memory_limit, 0, offsetof($1, cgroup_context) $1.DefaultMemoryLow, config_parse_memory_limit, 0, offsetof($1, cgroup_context) $1.MemoryLow, config_parse_memory_limit, 0, offsetof($1, cgroup_context) $1.MemoryHigh, config_parse_memory_limit, 0, offsetof($1, cgroup_context) diff --git a/src/shared/bus-unit-util.c b/src/shared/bus-unit-util.c index c6cbc9828c..2b425efc9c 100644 --- a/src/shared/bus-unit-util.c +++ b/src/shared/bus-unit-util.c @@ -413,7 +413,7 @@ static int bus_append_cgroup_property(sd_bus_message *m, const char *field, cons return 1; } - if (STR_IN_SET(field, "MemoryMin", "DefaultMemoryLow", "MemoryLow", "MemoryHigh", "MemoryMax", "MemorySwapMax", "MemoryLimit", "TasksMax")) { + if (STR_IN_SET(field, "MemoryMin", "DefaultMemoryLow", "DefaultMemoryMin", "MemoryLow", "MemoryHigh", "MemoryMax", "MemorySwapMax", "MemoryLimit", "TasksMax")) { if (isempty(eq) || streq(eq, "infinity")) { r = sd_bus_message_append(m, "(sv)", field, "t", CGROUP_LIMIT_MAX); diff --git a/src/shared/bus-util.c b/src/shared/bus-util.c index 9c3ce2f712..c2440271fe 100644 --- a/src/shared/bus-util.c +++ b/src/shared/bus-util.c @@ -794,7 +794,7 @@ static int bus_print_property(const char *name, const char *expected_value, sd_b bus_print_property_value(name, expected_value, value, "[not set]"); - else if ((STR_IN_SET(name, "DefaultMemoryLow", "MemoryLow", "MemoryHigh", "MemoryMax", "MemorySwapMax", "MemoryLimit") && u == CGROUP_LIMIT_MAX) || + else if ((STR_IN_SET(name, "DefaultMemoryLow", "DefaultMemoryMin", "MemoryLow", "MemoryHigh", "MemoryMax", "MemorySwapMax", "MemoryLimit") && u == CGROUP_LIMIT_MAX) || (STR_IN_SET(name, "TasksMax", "DefaultTasksMax") && u == (uint64_t) -1) || (startswith(name, "Limit") && u == (uint64_t) -1) || (startswith(name, "DefaultLimit") && u == (uint64_t) -1)) From 22bf131be278b95a4a204514d37a4344cf6365c6 Mon Sep 17 00:00:00 2001 From: Chris Down Date: Fri, 3 May 2019 08:32:41 -0400 Subject: [PATCH 3/4] cgroup: Support 0-value for memory protection directives These make sense to be explicitly set at 0 (which has a different effect than the default, since it can affect processing of `DefaultMemoryXXX`). Without this, it's not easily possible to relinquish memory protection for a subtree, which is not great. --- NEWS | 3 +++ src/core/dbus-cgroup.c | 17 +++++++++-------- src/core/load-fragment.c | 2 +- 3 files changed, 13 insertions(+), 9 deletions(-) diff --git a/NEWS b/NEWS index 57ed27a61d..78c44db4a6 100644 --- a/NEWS +++ b/NEWS @@ -24,6 +24,9 @@ CHANGES WITH 243 in spe: hierarchically set default memory protection values for a particular subtree of the unit hierarchy. + * Memory protection directives can now take a value of zero, allowing + explicit opting out of a default value propagated by an ancestor. + … CHANGES WITH 242: diff --git a/src/core/dbus-cgroup.c b/src/core/dbus-cgroup.c index 4d2bd3d5e1..d75628c663 100644 --- a/src/core/dbus-cgroup.c +++ b/src/core/dbus-cgroup.c @@ -613,6 +613,7 @@ BUS_DEFINE_SET_CGROUP_WEIGHT(cpu_shares, CGROUP_MASK_CPU, CGROUP_CPU_SHARES_IS_O BUS_DEFINE_SET_CGROUP_WEIGHT(io_weight, CGROUP_MASK_IO, CGROUP_WEIGHT_IS_OK, CGROUP_WEIGHT_INVALID); BUS_DEFINE_SET_CGROUP_WEIGHT(blockio_weight, CGROUP_MASK_BLKIO, CGROUP_BLKIO_WEIGHT_IS_OK, CGROUP_BLKIO_WEIGHT_INVALID); BUS_DEFINE_SET_CGROUP_LIMIT(memory, CGROUP_MASK_MEMORY, physical_memory_scale, 1); +BUS_DEFINE_SET_CGROUP_LIMIT(memory_protection, CGROUP_MASK_MEMORY, physical_memory_scale, 0); BUS_DEFINE_SET_CGROUP_LIMIT(swap, CGROUP_MASK_MEMORY, physical_memory_scale, 0); BUS_DEFINE_SET_CGROUP_LIMIT(tasks_max, CGROUP_MASK_PIDS, system_tasks_max_scale, 1); #pragma GCC diagnostic pop @@ -672,16 +673,16 @@ int bus_cgroup_set_property( 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(u, name, &c->memory_min, message, flags, error); + return bus_cgroup_set_memory_protection(u, name, &c->memory_min, message, flags, error); if (streq(name, "MemoryLow")) - return bus_cgroup_set_memory(u, name, &c->memory_low, message, flags, error); + return bus_cgroup_set_memory_protection(u, name, &c->memory_low, message, flags, error); if (streq(name, "DefaultMemoryMin")) - return bus_cgroup_set_memory(u, name, &c->default_memory_min, message, flags, error); + return bus_cgroup_set_memory_protection(u, name, &c->default_memory_min, message, flags, error); if (streq(name, "DefaultMemoryLow")) - return bus_cgroup_set_memory(u, name, &c->default_memory_low, message, flags, error); + return bus_cgroup_set_memory_protection(u, name, &c->default_memory_low, message, flags, error); if (streq(name, "MemoryHigh")) return bus_cgroup_set_memory(u, name, &c->memory_high, message, flags, error); @@ -696,16 +697,16 @@ int bus_cgroup_set_property( return bus_cgroup_set_memory(u, name, &c->memory_limit, message, flags, error); if (streq(name, "MemoryMinScale")) - return bus_cgroup_set_memory_scale(u, name, &c->memory_min, message, flags, error); + return bus_cgroup_set_memory_protection_scale(u, name, &c->memory_min, message, flags, error); if (streq(name, "MemoryLowScale")) - return bus_cgroup_set_memory_scale(u, name, &c->memory_low, message, flags, error); + return bus_cgroup_set_memory_protection_scale(u, name, &c->memory_low, message, flags, error); if (streq(name, "DefaultMemoryMinScale")) - return bus_cgroup_set_memory_scale(u, name, &c->default_memory_min, message, flags, error); + return bus_cgroup_set_memory_protection_scale(u, name, &c->default_memory_min, message, flags, error); if (streq(name, "DefaultMemoryLowScale")) - return bus_cgroup_set_memory_scale(u, name, &c->default_memory_low, message, flags, error); + return bus_cgroup_set_memory_protection_scale(u, name, &c->default_memory_low, message, flags, error); if (streq(name, "MemoryHighScale")) return bus_cgroup_set_memory_scale(u, name, &c->memory_high, message, flags, error); diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c index bb302fb46b..11a9a7bdeb 100644 --- a/src/core/load-fragment.c +++ b/src/core/load-fragment.c @@ -3137,7 +3137,7 @@ int config_parse_memory_limit( bytes = physical_memory_scale(r, 1000U); if (bytes >= UINT64_MAX || - (bytes <= 0 && !streq(lvalue, "MemorySwapMax"))) { + (bytes <= 0 && !STR_IN_SET(lvalue, "MemorySwapMax", "MemoryLow", "MemoryMin", "DefaultMemoryLow", "DefaultMemoryMin"))) { log_syntax(unit, LOG_ERR, filename, line, 0, "Memory limit '%s' out of range, ignoring.", rvalue); return 0; } From 465ace74d9820824968ab5e82c81e42c2f1894b0 Mon Sep 17 00:00:00 2001 From: Chris Down Date: Fri, 3 May 2019 08:40:11 -0400 Subject: [PATCH 4/4] cgroup: Test that it's possible to set memory protection to 0 again The previous commit fixes this up, and this should prevent it regressing. --- src/test/test-cgroup-unit-default.c | 6 +++--- test/dml-passthrough-set-ml.service | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/test/test-cgroup-unit-default.c b/src/test/test-cgroup-unit-default.c index 7372f97a1c..4fb629217f 100644 --- a/src/test/test-cgroup-unit-default.c +++ b/src/test/test-cgroup-unit-default.c @@ -39,7 +39,7 @@ static int test_default_memory_low(void) { * 1. dml-passthrough.slice sets MemoryLow=100. This should not affect its children, as only * DefaultMemoryLow is propagated, not MemoryLow. As such, all leaf services should end up with * memory.low as 50, inherited from dml.slice, *except* for dml-passthrough-set-ml.service, which - * should have the value of 25, as it has MemoryLow explicitly set. + * should have the value of 0, as it has MemoryLow explicitly set. * * ┌───────────┐ * │ dml.slice │ @@ -49,7 +49,7 @@ static int test_default_memory_low(void) { * │ dml-passthrough.slice │ * └───────────┬───────────┘ * ┌───────────────────────────────────┼───────────────────────────────────┐ - * no new settings DefaultMemoryLow=15 MemoryLow=25 + * no new settings DefaultMemoryLow=15 MemoryLow=0 * ┌───────────────┴───────────────┐ ┌────────────────┴────────────────┐ ┌───────────────┴────────────────┐ * │ dml-passthrough-empty.service │ │ dml-passthrough-set-dml.service │ │ dml-passthrough-set-ml.service │ * └───────────────────────────────┘ └─────────────────────────────────┘ └────────────────────────────────┘ @@ -122,7 +122,7 @@ static int test_default_memory_low(void) { assert_se(unit_get_ancestor_memory_low(dml_passthrough) == 100); assert_se(unit_get_ancestor_memory_low(dml_passthrough_empty) == dml_tree_default); assert_se(unit_get_ancestor_memory_low(dml_passthrough_set_dml) == 50); - assert_se(unit_get_ancestor_memory_low(dml_passthrough_set_ml) == 25); + assert_se(unit_get_ancestor_memory_low(dml_passthrough_set_ml) == 0); assert_se(unit_get_ancestor_memory_low(dml_override) == dml_tree_default); assert_se(unit_get_ancestor_memory_low(dml_override_empty) == 10); diff --git a/test/dml-passthrough-set-ml.service b/test/dml-passthrough-set-ml.service index 2abd591389..2e568b5deb 100644 --- a/test/dml-passthrough-set-ml.service +++ b/test/dml-passthrough-set-ml.service @@ -5,4 +5,4 @@ Description=DML passthrough set ML service Slice=dml-passthrough.slice Type=oneshot ExecStart=/bin/true -MemoryLow=25 +MemoryLow=0