From d9ab2bcf0591b496f1a4750c7ff790b33f9c7e59 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 8 Jun 2016 18:56:20 +0200 Subject: [PATCH 01/11] util: when determining the amount of memory on this system, take cgroup limit into account When determining the amount of RAM in the system, let's make sure we also read the root-level cgroup memory limit into account. This isn't particularly useful on the host, but in containers it makes sure that whatever memory the container got assigned is actually used for RAM size calculations. --- src/basic/util.c | 35 +++++++++++++++++++++++++++++------ src/test/test-util.c | 14 ++++++++++++++ 2 files changed, 43 insertions(+), 6 deletions(-) diff --git a/src/basic/util.c b/src/basic/util.c index f2f92fb3b7..88d58cd94a 100644 --- a/src/basic/util.c +++ b/src/basic/util.c @@ -36,6 +36,7 @@ #include "alloc-util.h" #include "build.h" +#include "cgroup-util.h" #include "def.h" #include "dirent-util.h" #include "fd-util.h" @@ -771,15 +772,37 @@ int namespace_enter(int pidns_fd, int mntns_fd, int netns_fd, int userns_fd, int } uint64_t physical_memory(void) { - long mem; + _cleanup_free_ char *root = NULL, *value = NULL; + uint64_t mem, lim; + size_t ps; + long sc; - /* We return this as uint64_t in case we are running as 32bit - * process on a 64bit kernel with huge amounts of memory */ + /* We return this as uint64_t in case we are running as 32bit process on a 64bit kernel with huge amounts of + * memory. + * + * In order to support containers nicely that have a configured memory limit we'll take the minimum of the + * physically reported amount of memory and the limit configured for the root cgroup, if there is any. */ - mem = sysconf(_SC_PHYS_PAGES); - assert(mem > 0); + sc = sysconf(_SC_PHYS_PAGES); + assert(sc > 0); - return (uint64_t) mem * (uint64_t) page_size(); + ps = page_size(); + mem = (uint64_t) sc * (uint64_t) ps; + + if (cg_get_root_path(&root) < 0) + return mem; + + if (cg_get_attribute("memory", root, "memory.limit_in_bytes", &value)) + return mem; + + if (safe_atou64(value, &lim) < 0) + return mem; + + /* Make sure the limit is a multiple of our own page size */ + lim /= ps; + lim *= ps; + + return MIN(mem, lim); } int update_reboot_parameter_and_warn(const char *param) { diff --git a/src/test/test-util.c b/src/test/test-util.c index 9b6d2a7968..5b3fbcff53 100644 --- a/src/test/test-util.c +++ b/src/test/test-util.c @@ -26,6 +26,7 @@ #include "def.h" #include "fileio.h" #include "fs-util.h" +#include "parse-util.h" #include "raw-clone.h" #include "rm-rf.h" #include "string-util.h" @@ -263,6 +264,18 @@ static void test_raw_clone(void) { } } +static void test_physical_memory(void) { + uint64_t p; + char buf[FORMAT_BYTES_MAX]; + + p = physical_memory(); + assert_se(p > 0); + assert_se(p < UINT64_MAX); + assert_se(p % page_size() == 0); + + log_info("Memory: %s", format_bytes(buf, sizeof(buf), p)); +} + int main(int argc, char *argv[]) { log_parse_environment(); log_open(); @@ -277,6 +290,7 @@ int main(int argc, char *argv[]) { test_log2i(); test_execute_directory(); test_raw_clone(); + test_physical_memory(); return 0; } From 9184ca48ea43e009cd6f379f319926109a30926c Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 8 Jun 2016 19:25:38 +0200 Subject: [PATCH 02/11] util-lib: introduce parse_percent() for parsing percent specifications And port a couple of users over to it. --- src/basic/parse-util.c | 19 +++++++++++++++++++ src/basic/parse-util.h | 2 ++ src/core/load-fragment.c | 15 +++++---------- src/login/logind-user.c | 29 ++++++++--------------------- src/shared/bus-unit-util.c | 15 ++++++--------- src/test/test-parse-util.c | 19 +++++++++++++++++++ 6 files changed, 59 insertions(+), 40 deletions(-) diff --git a/src/basic/parse-util.c b/src/basic/parse-util.c index 6c11b605a9..503a895731 100644 --- a/src/basic/parse-util.c +++ b/src/basic/parse-util.c @@ -532,3 +532,22 @@ int parse_fractional_part_u(const char **p, size_t digits, unsigned *res) { return 0; } + +int parse_percent(const char *p) { + const char *pc, *n; + unsigned v; + int r; + + pc = endswith(p, "%"); + if (!pc) + return -EINVAL; + + n = strndupa(p, pc - p); + r = safe_atou(n, &v); + if (r < 0) + return r; + if (v > 100) + return -ERANGE; + + return (int) v; +} diff --git a/src/basic/parse-util.h b/src/basic/parse-util.h index 7dc579a159..73441bb6fd 100644 --- a/src/basic/parse-util.h +++ b/src/basic/parse-util.h @@ -105,3 +105,5 @@ static inline int safe_atozu(const char *s, size_t *ret_u) { int safe_atod(const char *s, double *ret_d); int parse_fractional_part_u(const char **s, size_t digits, unsigned *res); + +int parse_percent(const char *p); diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c index 17c72aed88..fe60bee789 100644 --- a/src/core/load-fragment.c +++ b/src/core/load-fragment.c @@ -2774,7 +2774,7 @@ int config_parse_cpu_quota( void *userdata) { CGroupContext *c = data; - double percent; + int r; assert(filename); assert(lvalue); @@ -2785,18 +2785,13 @@ int config_parse_cpu_quota( return 0; } - if (!endswith(rvalue, "%")) { - log_syntax(unit, LOG_ERR, filename, line, 0, "CPU quota '%s' not ending in '%%'. Ignoring.", rvalue); + r = parse_percent(rvalue); + if (r <= 0) { + log_syntax(unit, LOG_ERR, filename, line, r, "CPU quota '%s' invalid. Ignoring.", rvalue); return 0; } - if (sscanf(rvalue, "%lf%%", &percent) != 1 || percent <= 0) { - log_syntax(unit, LOG_ERR, filename, line, 0, "CPU quota '%s' invalid. Ignoring.", rvalue); - return 0; - } - - c->cpu_quota_per_sec_usec = (usec_t) (percent * USEC_PER_SEC / 100); - + c->cpu_quota_per_sec_usec = ((usec_t) r * USEC_PER_SEC) / 100U; return 0; } diff --git a/src/login/logind-user.c b/src/login/logind-user.c index a826321bf0..6d0904f5ca 100644 --- a/src/login/logind-user.c +++ b/src/login/logind-user.c @@ -843,7 +843,6 @@ int config_parse_tmpfs_size( void *userdata) { size_t *sz = data; - const char *e; int r; assert(filename); @@ -851,29 +850,17 @@ int config_parse_tmpfs_size( assert(rvalue); assert(data); - e = endswith(rvalue, "%"); - if (e) { - unsigned long ul; - char *f; - - errno = 0; - ul = strtoul(rvalue, &f, 10); - if (errno > 0 || f != e) { - log_syntax(unit, LOG_ERR, filename, line, errno, "Failed to parse percentage value, ignoring: %s", rvalue); - return 0; - } - - if (ul <= 0 || ul >= 100) { - log_syntax(unit, LOG_ERR, filename, line, 0, "Percentage value out of range, ignoring: %s", rvalue); - return 0; - } - - *sz = PAGE_ALIGN((size_t) ((physical_memory() * (uint64_t) ul) / (uint64_t) 100)); - } else { + /* First, try to parse as percentage */ + r = parse_percent(rvalue); + if (r > 0 && r < 100) + *sz = PAGE_ALIGN((size_t) ((physical_memory() * (uint64_t) r) / 100U)); + else { uint64_t k; + /* If the passed argument was not a percentage, or out of range, parse as byte size */ + r = parse_size(rvalue, 1024, &k); - if (r < 0 || (uint64_t) (size_t) k != k) { + if (r < 0 || k <= 0 || (uint64_t) (size_t) k != k) { log_syntax(unit, LOG_ERR, filename, line, r, "Failed to parse size value, ignoring: %s", rvalue); return 0; } diff --git a/src/shared/bus-unit-util.c b/src/shared/bus-unit-util.c index 8f4f93ee0c..778c79b3cf 100644 --- a/src/shared/bus-unit-util.c +++ b/src/shared/bus-unit-util.c @@ -83,18 +83,14 @@ int bus_append_unit_property_assignment(sd_bus_message *m, const char *assignmen if (isempty(eq)) r = sd_bus_message_append(m, "sv", "CPUQuotaPerSecUSec", "t", USEC_INFINITY); - else if (endswith(eq, "%")) { - double percent; - - if (sscanf(eq, "%lf%%", &percent) != 1 || percent <= 0) { - log_error("CPU quota '%s' invalid.", eq); + else { + r = parse_percent(eq); + if (r <= 0) { + log_error_errno(r, "CPU quota '%s' invalid.", eq); return -EINVAL; } - r = sd_bus_message_append(m, "sv", "CPUQuotaPerSecUSec", "t", (usec_t) percent * USEC_PER_SEC / 100); - } else { - log_error("CPU quota needs to be in percent."); - return -EINVAL; + r = sd_bus_message_append(m, "sv", "CPUQuotaPerSecUSec", "t", (usec_t) r * USEC_PER_SEC / 100U); } goto finish; @@ -110,6 +106,7 @@ int bus_append_unit_property_assignment(sd_bus_message *m, const char *assignmen char *n; usec_t t; size_t l; + r = parse_sec(eq, &t); if (r < 0) return log_error_errno(r, "Failed to parse %s= parameter: %s", field, eq); diff --git a/src/test/test-parse-util.c b/src/test/test-parse-util.c index 7d8677e17c..0a76308f72 100644 --- a/src/test/test-parse-util.c +++ b/src/test/test-parse-util.c @@ -475,6 +475,24 @@ static void test_safe_atod(void) { assert_se(*e == ','); } +static void test_parse_percent(void) { + assert_se(parse_percent("") == -EINVAL); + assert_se(parse_percent("foo") == -EINVAL); + assert_se(parse_percent("0") == -EINVAL); + assert_se(parse_percent("50") == -EINVAL); + assert_se(parse_percent("100") == -EINVAL); + assert_se(parse_percent("-1") == -EINVAL); + assert_se(parse_percent("0%") == 0); + assert_se(parse_percent("55%") == 55); + assert_se(parse_percent("100%") == 100); + assert_se(parse_percent("-7%") == -ERANGE); + assert_se(parse_percent("107%") == -ERANGE); + assert_se(parse_percent("%") == -EINVAL); + assert_se(parse_percent("%%") == -EINVAL); + assert_se(parse_percent("%1") == -EINVAL); + assert_se(parse_percent("1%%") == -EINVAL); +} + int main(int argc, char *argv[]) { log_parse_environment(); log_open(); @@ -488,6 +506,7 @@ int main(int argc, char *argv[]) { test_safe_atou16(); test_safe_atoi16(); test_safe_atod(); + test_parse_percent(); return 0; } From 875ae5661a011b757f0eaa468dea6ba91cbe5437 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 8 Jun 2016 19:36:09 +0200 Subject: [PATCH 03/11] core: optionally, accept a percentage value for MemoryLimit= and related settings If a percentage is used, it is taken relative to the installed RAM size. This should make it easier to write generic unit files that adapt to the local system. --- man/systemd.resource-control.xml | 36 +++++++++++++++++--------------- src/core/load-fragment.c | 16 +++++++++++--- 2 files changed, 32 insertions(+), 20 deletions(-) diff --git a/man/systemd.resource-control.xml b/man/systemd.resource-control.xml index d4c8fa7091..0551d75026 100644 --- a/man/systemd.resource-control.xml +++ b/man/systemd.resource-control.xml @@ -228,9 +228,11 @@ reclaimed as long as memory can be reclaimed from unprotected units. Takes a memory size in bytes. If the value is suffixed with K, M, G or T, the specified memory size is - parsed as Kilobytes, Megabytes, Gigabytes, or Terabytes (with the base 1024), respectively. This controls the - memory.low control group attribute. For details about this control group attribute, see - cgroup-v2.txt. + parsed as Kilobytes, Megabytes, Gigabytes, or Terabytes (with the base 1024), respectively. Alternatively, a + percentage value may be specified, which is taken relative to the installed physical memory on the + system. This controls the memory.low control group attribute. For details about this + control group attribute, see cgroup-v2.txt. Implies MemoryAccounting=true. @@ -247,7 +249,9 @@ aggressively in such cases. This is the main mechanism to control memory usage of a unit. Takes a memory size in bytes. If the value is suffixed with K, M, G or T, the specified memory size is - parsed as Kilobytes, Megabytes, Gigabytes, or Terabytes (with the base 1024), respectively. If assigned the + parsed as Kilobytes, Megabytes, Gigabytes, or Terabytes (with the base 1024), respectively. Alternatively, a + percentage value may be specified, which is taken relative to the installed physical memory on the + system. If assigned the special value infinity, no memory limit is applied. This controls the memory.high control group attribute. For details about this control group attribute, see cgroup-v2.txt. @@ -268,8 +272,9 @@ last line of defense. Takes a memory size in bytes. If the value is suffixed with K, M, G or T, the specified memory size is - parsed as Kilobytes, Megabytes, Gigabytes, or Terabytes (with the base 1024), respectively. If assigned the - special value infinity, no memory limit is applied. This controls the + parsed as Kilobytes, Megabytes, Gigabytes, or Terabytes (with the base 1024), respectively. Alternatively, a + percentage value may be specified, which is taken relative to the installed physical memory on the system. If + assigned the special value infinity, no memory limit is applied. This controls the memory.max control group attribute. For details about this control group attribute, see cgroup-v2.txt. @@ -284,17 +289,14 @@ MemoryLimit=bytes - Specify the limit on maximum memory usage of the - executed processes. The limit specifies how much process and - kernel memory can be used by tasks in this unit. Takes a - memory size in bytes. If the value is suffixed with K, M, G - or T, the specified memory size is parsed as Kilobytes, - Megabytes, Gigabytes, or Terabytes (with the base 1024), - respectively. If assigned the special value - infinity, no memory limit is applied. This - controls the memory.limit_in_bytes - control group attribute. For details about this control - group attribute, see Specify the limit on maximum memory usage of the executed processes. The limit specifies how much + process and kernel memory can be used by tasks in this unit. Takes a memory size in bytes. If the value is + suffixed with K, M, G or T, the specified memory size is parsed as Kilobytes, Megabytes, Gigabytes, or + Terabytes (with the base 1024), respectively. Alternatively, a percentage value may be specified, which is + taken relative to the installed physical memory on the system. If assigned the special value + infinity, no memory limit is applied. This controls the + memory.limit_in_bytes control group attribute. For details about this control group + attribute, see memory.txt. Implies MemoryAccounting=true. diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c index fe60bee789..05852cc7e3 100644 --- a/src/core/load-fragment.c +++ b/src/core/load-fragment.c @@ -2812,9 +2812,19 @@ int config_parse_memory_limit( int r; if (!isempty(rvalue) && !streq(rvalue, "infinity")) { - r = parse_size(rvalue, 1024, &bytes); - if (r < 0 || bytes < 1) { - log_syntax(unit, LOG_ERR, filename, line, r, "Memory limit '%s' invalid. Ignoring.", rvalue); + + r = parse_percent(rvalue); + if (r < 0) { + r = parse_size(rvalue, 1024, &bytes); + if (r < 0) { + log_syntax(unit, LOG_ERR, filename, line, r, "Memory limit '%s' invalid. Ignoring.", rvalue); + return 0; + } + } else + bytes = (((physical_memory() / page_size()) * (uint64_t) r) / 100) * page_size(); + + if (bytes < 1) { + log_syntax(unit, LOG_ERR, filename, line, 0, "Memory limit '%s' too small. Ignoring.", rvalue); return 0; } } From 328583dbc3621271dbdac4a279865ea7243f8ddf Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 8 Jun 2016 19:37:42 +0200 Subject: [PATCH 04/11] man: minor fixes --- man/systemd.resource-control.xml | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/man/systemd.resource-control.xml b/man/systemd.resource-control.xml index 0551d75026..7263c0b329 100644 --- a/man/systemd.resource-control.xml +++ b/man/systemd.resource-control.xml @@ -92,16 +92,14 @@ Automatic Dependencies - Units with the Slice= setting set get - automatic Requires= and - After= dependencies on the specified slice - unit. + Units with the Slice= setting set automatically acquire Requires= and + After= dependencies on the specified slice unit. Unified and Legacy Control Group Hierarchies - Unified control group hierarchy is the new version of kernel control group interface. Depending on the + The unified control group hierarchy is the new version of kernel control group interface. Depending on the resource type, there are differences in resource control capabilities. Also, because of interface changes, some resource types have a separate set of options on the unified hierarchy. @@ -117,8 +115,8 @@ - MemoryMax replaces MemoryLimit. MemoryLow - and MemoryHigh are effective only on unified hierarchy. + MemoryMax= replaces MemoryLimit=. MemoryLow= + and MemoryHigh= are effective only on unified hierarchy. From cd0a7a8e58cf7e9f273bf446bb50bf982a8cf29c Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 8 Jun 2016 20:04:22 +0200 Subject: [PATCH 05/11] core: when receiving a memory limit via the bus, refuse 0 When parsing unit files we already refuse unit memory limits of zero, let's also refuse it when the value is set via the bus. --- src/core/dbus-cgroup.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/core/dbus-cgroup.c b/src/core/dbus-cgroup.c index 8525fa1bf1..5b94b5e575 100644 --- a/src/core/dbus-cgroup.c +++ b/src/core/dbus-cgroup.c @@ -641,7 +641,7 @@ int bus_cgroup_set_property( return 1; - } else if (streq(name, "BlockIOReadBandwidth") || streq(name, "BlockIOWriteBandwidth")) { + } else if (STR_IN_SET(name, "BlockIOReadBandwidth", "BlockIOWriteBandwidth")) { const char *path; bool read = true; unsigned n = 0; @@ -835,6 +835,8 @@ int bus_cgroup_set_property( r = sd_bus_message_read(message, "t", &v); if (r < 0) return r; + if (v <= 0) + return sd_bus_error_set_errnof(error, EINVAL, "%s= is too small", name); if (mode != UNIT_CHECK) { if (streq(name, "MemoryLow")) @@ -860,6 +862,8 @@ int bus_cgroup_set_property( r = sd_bus_message_read(message, "t", &limit); if (r < 0) return r; + if (limit <= 0) + return sd_bus_error_set_errnof(error, EINVAL, "%s= is too small", name); if (mode != UNIT_CHECK) { c->memory_limit = limit; From 799ec13412e2f6649fd69ce4f1ca1674a40039b0 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 8 Jun 2016 20:05:14 +0200 Subject: [PATCH 06/11] core: make sure to use "infinity" in unit files, not "max" THe latter is a kernelism, we only understand "infinity". --- src/core/dbus-cgroup.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/dbus-cgroup.c b/src/core/dbus-cgroup.c index 5b94b5e575..fe035109e3 100644 --- a/src/core/dbus-cgroup.c +++ b/src/core/dbus-cgroup.c @@ -849,7 +849,7 @@ int bus_cgroup_set_property( unit_invalidate_cgroup(u, CGROUP_MASK_MEMORY); if (v == CGROUP_LIMIT_MAX) - unit_write_drop_in_private_format(u, mode, name, "%s=max", name); + unit_write_drop_in_private_format(u, mode, name, "%s=infinity", name); else unit_write_drop_in_private_format(u, mode, name, "%s=%" PRIu64, name, v); } From d8cf2ac79b524d7784bccb428295ebc9c5e8548c Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 8 Jun 2016 20:45:32 +0200 Subject: [PATCH 07/11] util: introduce physical_memory_scale() to unify how we scale by physical memory The various bits of code did the scaling all different, let's unify this, given that the code is not trivial. --- src/basic/util.c | 27 +++++++++++++++++++++++++++ src/basic/util.h | 1 + src/core/load-fragment.c | 2 +- src/login/logind-user.c | 2 +- src/login/logind.c | 2 +- src/test/test-util.c | 38 +++++++++++++++++++++++++++++++++++++- 6 files changed, 68 insertions(+), 4 deletions(-) diff --git a/src/basic/util.c b/src/basic/util.c index 88d58cd94a..09d16697b7 100644 --- a/src/basic/util.c +++ b/src/basic/util.c @@ -805,6 +805,33 @@ uint64_t physical_memory(void) { return MIN(mem, lim); } +uint64_t physical_memory_scale(uint64_t v, uint64_t max) { + uint64_t p, m, ps, r; + + assert(max > 0); + + /* Returns the physical memory size, multiplied by v divided by max. Returns UINT64_MAX on overflow. On success + * the result is a multiple of the page size (rounds down). */ + + ps = page_size(); + assert(ps > 0); + + p = physical_memory() / ps; + assert(p > 0); + + m = p * v; + if (m / p != v) + return UINT64_MAX; + + m /= max; + + r = m * ps; + if (r / ps != m) + return UINT64_MAX; + + return r; +} + int update_reboot_parameter_and_warn(const char *param) { int r; diff --git a/src/basic/util.h b/src/basic/util.h index 9e6df19ef1..db105197e8 100644 --- a/src/basic/util.h +++ b/src/basic/util.h @@ -184,6 +184,7 @@ int namespace_open(pid_t pid, int *pidns_fd, int *mntns_fd, int *netns_fd, int * int namespace_enter(int pidns_fd, int mntns_fd, int netns_fd, int userns_fd, int root_fd); uint64_t physical_memory(void); +uint64_t physical_memory_scale(uint64_t v, uint64_t max); int update_reboot_parameter_and_warn(const char *param); diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c index 05852cc7e3..58d7275a96 100644 --- a/src/core/load-fragment.c +++ b/src/core/load-fragment.c @@ -2821,7 +2821,7 @@ int config_parse_memory_limit( return 0; } } else - bytes = (((physical_memory() / page_size()) * (uint64_t) r) / 100) * page_size(); + bytes = physical_memory_scale(r, 100U); if (bytes < 1) { log_syntax(unit, LOG_ERR, filename, line, 0, "Memory limit '%s' too small. Ignoring.", rvalue); diff --git a/src/login/logind-user.c b/src/login/logind-user.c index 6d0904f5ca..de44d369cf 100644 --- a/src/login/logind-user.c +++ b/src/login/logind-user.c @@ -853,7 +853,7 @@ int config_parse_tmpfs_size( /* First, try to parse as percentage */ r = parse_percent(rvalue); if (r > 0 && r < 100) - *sz = PAGE_ALIGN((size_t) ((physical_memory() * (uint64_t) r) / 100U)); + *sz = physical_memory_scale(r, 100U); else { uint64_t k; diff --git a/src/login/logind.c b/src/login/logind.c index caf149cfb7..d01dd110ea 100644 --- a/src/login/logind.c +++ b/src/login/logind.c @@ -61,7 +61,7 @@ static void manager_reset_config(Manager *m) { m->idle_action_usec = 30 * USEC_PER_MINUTE; m->idle_action = HANDLE_IGNORE; - m->runtime_dir_size = PAGE_ALIGN((size_t) (physical_memory() / 10)); /* 10% */ + m->runtime_dir_size = physical_memory_scale(10U, 100U); /* 10% */ m->user_tasks_max = 12288; m->sessions_max = 8192; m->inhibitors_max = 8192; diff --git a/src/test/test-util.c b/src/test/test-util.c index 5b3fbcff53..e177612a9f 100644 --- a/src/test/test-util.c +++ b/src/test/test-util.c @@ -273,7 +273,42 @@ static void test_physical_memory(void) { assert_se(p < UINT64_MAX); assert_se(p % page_size() == 0); - log_info("Memory: %s", format_bytes(buf, sizeof(buf), p)); + log_info("Memory: %s (%" PRIu64 ")", format_bytes(buf, sizeof(buf), p), p); +} + +static void test_physical_memory_scale(void) { + uint64_t p; + + p = physical_memory(); + + assert_se(physical_memory_scale(0, 100) == 0); + assert_se(physical_memory_scale(100, 100) == p); + + log_info("Memory original: %" PRIu64, physical_memory()); + log_info("Memory scaled by 50%%: %" PRIu64, physical_memory_scale(50, 100)); + log_info("Memory divided by 2: %" PRIu64, physical_memory() / 2); + log_info("Page size: %zu", page_size()); + + /* There might be an uneven number of pages, hence permit these calculations to be half a page off... */ + assert_se(page_size()/2 + physical_memory_scale(50, 100) - p/2 <= page_size()); + assert_se(physical_memory_scale(200, 100) == p*2); + + assert_se(physical_memory_scale(0, 1) == 0); + assert_se(physical_memory_scale(1, 1) == p); + assert_se(physical_memory_scale(2, 1) == p*2); + + assert_se(physical_memory_scale(0, 2) == 0); + + assert_se(page_size()/2 + physical_memory_scale(1, 2) - p/2 <= page_size()); + assert_se(physical_memory_scale(2, 2) == p); + assert_se(physical_memory_scale(4, 2) == p*2); + + assert_se(physical_memory_scale(0, UINT32_MAX) == 0); + assert_se(physical_memory_scale(UINT32_MAX, UINT32_MAX) == p); + + /* overflow */ + assert_se(physical_memory_scale(UINT64_MAX/4, UINT64_MAX) == UINT64_MAX); + } int main(int argc, char *argv[]) { @@ -291,6 +326,7 @@ int main(int argc, char *argv[]) { test_execute_directory(); test_raw_clone(); test_physical_memory(); + test_physical_memory_scale(); return 0; } From d58d600efdcbefa38247861a1df032d1a8fda606 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 8 Jun 2016 20:52:06 +0200 Subject: [PATCH 08/11] systemctl: allow percent-based MemoryLimit= settings via systemctl set-property The unit files already accept relative, percent-based memory limit specification, let's make sure "systemctl set-property" support this too. Since we want the physical memory size of the destination machine to apply we pass the percentage in a new set of properties that only exist for this purpose, and can only be set. --- src/core/dbus-cgroup.c | 52 ++++++++++++++++++++++++++++++++++++++ src/shared/bus-unit-util.c | 43 ++++++++++++++++++++----------- 2 files changed, 80 insertions(+), 15 deletions(-) diff --git a/src/core/dbus-cgroup.c b/src/core/dbus-cgroup.c index fe035109e3..27bbe2d26d 100644 --- a/src/core/dbus-cgroup.c +++ b/src/core/dbus-cgroup.c @@ -856,6 +856,38 @@ int bus_cgroup_set_property( return 1; + } else if (STR_IN_SET(name, "MemoryLowByPhysicalMemory", "MemoryHighByPhysicalMemory", "MemoryMaxByPhysicalMemory")) { + uint32_t raw; + uint64_t v; + + r = sd_bus_message_read(message, "u", &raw); + if (r < 0) + return r; + + v = physical_memory_scale(raw, UINT32_MAX); + if (v <= 0 || v == UINT64_MAX) + return sd_bus_error_set_errnof(error, EINVAL, "%s= is out of range", name); + + if (mode != UNIT_CHECK) { + const char *e; + + /* Chop off suffix */ + assert_se(e = endswith(name, "ByPhysicalMemory")); + name = strndupa(name, e - name); + + if (streq(name, "MemoryLow")) + c->memory_low = v; + else if (streq(name, "MemoryHigh")) + c->memory_high = v; + else + c->memory_max = v; + + unit_invalidate_cgroup(u, CGROUP_MASK_MEMORY); + unit_write_drop_in_private_format(u, mode, name, "%s=%" PRIu32 "%%", name, (uint32_t) (DIV_ROUND_UP((uint64_t) raw * 100, (uint64_t) UINT32_MAX))); + } + + return 1; + } else if (streq(name, "MemoryLimit")) { uint64_t limit; @@ -877,6 +909,26 @@ int bus_cgroup_set_property( return 1; + } else if (streq(name, "MemoryLimitByPhysicalMemory")) { + uint64_t limit; + uint32_t raw; + + r = sd_bus_message_read(message, "u", &raw); + if (r < 0) + return r; + + limit = physical_memory_scale(raw, UINT32_MAX); + if (limit <= 0 || limit == UINT64_MAX) + return sd_bus_error_set_errnof(error, EINVAL, "%s= is out of range", name); + + if (mode != UNIT_CHECK) { + c->memory_limit = limit; + unit_invalidate_cgroup(u, CGROUP_MASK_MEMORY); + unit_write_drop_in_private_format(u, mode, "MemoryLimit", "MemoryLimit=%" PRIu32 "%%", (uint32_t) (DIV_ROUND_UP((uint64_t) raw * 100, (uint64_t) UINT32_MAX))); + } + + return 1; + } else if (streq(name, "DevicePolicy")) { const char *policy; CGroupDevicePolicy p; diff --git a/src/shared/bus-unit-util.c b/src/shared/bus-unit-util.c index 778c79b3cf..6fc201b885 100644 --- a/src/shared/bus-unit-util.c +++ b/src/shared/bus-unit-util.c @@ -120,6 +120,34 @@ int bus_append_unit_property_assignment(sd_bus_message *m, const char *assignmen strcpy(mempcpy(n, field, l - 3), "USec"); r = sd_bus_message_append(m, "sv", n, "t", t); goto finish; + + } else if (STR_IN_SET(field, "MemoryLow", "MemoryHigh", "MemoryMax", "MemoryLimit")) { + uint64_t bytes; + + if (isempty(eq) || streq(eq, "infinity")) + bytes = CGROUP_LIMIT_MAX; + else { + r = parse_percent(eq); + if (r >= 0) { + char *n; + + /* When this is a percentage we'll convert this into a relative value in the range + * 0…UINT32_MAX and pass it in the MemoryLowByPhysicalMemory property (and related + * ones). This way the physical memory size can be determined server-side */ + + n = strjoina(field, "ByPhysicalMemory"); + r = sd_bus_message_append(m, "sv", n, "u", (uint32_t) (((uint64_t) UINT32_MAX * r) / 100U)); + goto finish; + + } else { + r = parse_size(eq, 1024, &bytes); + if (r < 0) + return log_error_errno(r, "Failed to parse bytes specification %s", assignment); + } + } + + r = sd_bus_message_append(m, "sv", field, "t", bytes); + goto finish; } r = sd_bus_message_append_basic(m, SD_BUS_TYPE_STRING, field); @@ -163,21 +191,6 @@ int bus_append_unit_property_assignment(sd_bus_message *m, const char *assignmen r = sd_bus_message_append(m, "v", "b", r); - } else if (STR_IN_SET(field, "MemoryLow", "MemoryHigh", "MemoryMax", "MemoryLimit")) { - uint64_t bytes; - - if (isempty(eq) || streq(eq, "infinity")) - bytes = CGROUP_LIMIT_MAX; - else { - r = parse_size(eq, 1024, &bytes); - if (r < 0) { - log_error("Failed to parse bytes specification %s", assignment); - return -EINVAL; - } - } - - r = sd_bus_message_append(m, "v", "t", bytes); - } else if (streq(field, "TasksMax")) { uint64_t n; From a48072b7032997458c2d3e736a27193633639926 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 9 Jun 2016 19:49:48 +0200 Subject: [PATCH 09/11] sd-ndisc: add missing cast Apparently newer gcc versions are a bit more forgiving when assigning an "unsigned char*" pointer to something of a different type. Let's add the missing cast so that old gcc versions are fine, too. --- src/libsystemd-network/sd-ndisc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libsystemd-network/sd-ndisc.c b/src/libsystemd-network/sd-ndisc.c index ea3fe369ce..07b0d7f704 100644 --- a/src/libsystemd-network/sd-ndisc.c +++ b/src/libsystemd-network/sd-ndisc.c @@ -307,7 +307,7 @@ static int ndisc_recv(sd_event_source *s, int fd, uint32_t revents, void *userda if (cmsg->cmsg_level == SOL_SOCKET && cmsg->cmsg_type == SO_TIMESTAMP && cmsg->cmsg_len == CMSG_LEN(sizeof(struct timeval))) - triple_timestamp_from_realtime(&rt->timestamp, timeval_load(CMSG_DATA(cmsg))); + triple_timestamp_from_realtime(&rt->timestamp, timeval_load((struct timeval*) CMSG_DATA(cmsg))); } if (!triple_timestamp_is_set(&rt->timestamp)) From 3f71dec5d7210e3b21e79bde446fc2ba003486f1 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 9 Jun 2016 19:53:45 +0200 Subject: [PATCH 10/11] unit: properly comment generated comments in unit files Fix-up for 2a9a6f8ac04a69ca36d645f9305a33645f22a22b --- src/core/unit.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/unit.c b/src/core/unit.c index e98086a3f6..581962eba6 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -3375,7 +3375,7 @@ int unit_write_drop_in(Unit *u, UnitSetPropertiesMode mode, const char *name, co return -EINVAL; wrapped = strjoina("# This is a drop-in unit file extension, created via \"systemctl set-property\"\n" - "or an equivalent operation. Do not edit.\n", + "# or an equivalent operation. Do not edit.\n", data, "\n"); From 0e2e03c67a8b9df2ae7c785741b2b5ac7a2a4f2b Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 9 Jun 2016 16:15:07 +0200 Subject: [PATCH 11/11] update TODO --- TODO | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/TODO b/TODO index 929fb96491..5de4901133 100644 --- a/TODO +++ b/TODO @@ -33,7 +33,10 @@ Janitorial Clean-ups: Features: -* use phyical_memory() to allow MemoryLimit= configuration based on available system memory +* resolved: make sure when we get an ip address with ifindex suffix, we handle + it nicely + +* resolved: maybe add a switch to disable any local caching * ProtectKernelLogs= (drops CAP_SYSLOG, add seccomp for syslog() syscall, and DeviceAllow to /dev/kmsg) in service files @@ -227,7 +230,6 @@ Features: - resolved should optionally register additional per-interface LLMNR names, so that for the container case we can establish the same name (maybe "host") for referencing the server, everywhere. - - enable DNSSEC by default - allow clients to request DNSSEC for a single lookup even if DNSSEC is off (?) * refcounting in sd-resolve is borked