Merge pull request #3481 from poettering/relative-memcg

various changes, most importantly regarding memory metrics

13 files changed, 268 insertions, 69 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/basic/util.c b/src/basic/util.c
index f2f92fb3b7..09d16697b7 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,64 @@ 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. */
+
+        sc = sysconf(_SC_PHYS_PAGES);
+        assert(sc > 0);
+
+        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);
+}
+
+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;
 
-        mem = sysconf(_SC_PHYS_PAGES);
-        assert(mem > 0);
+        r = m * ps;
+        if (r / ps != m)
+                return UINT64_MAX;
 
-        return (uint64_t) mem * (uint64_t) page_size();
+        return r;
 }
 
 int update_reboot_parameter_and_warn(const char *param) {
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/dbus-cgroup.c b/src/core/dbus-cgroup.c
index 8525fa1bf1..27bbe2d26d 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"))
@@ -847,19 +849,53 @@ 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);
                 }
 
                 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;
 
                 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;
@@ -873,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/core/load-fragment.c b/src/core/load-fragment.c
index d42f517354..8295cf45a6 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);
-                return 0;
-        }
-
-        if (sscanf(rvalue, "%lf%%", &percent) != 1 || percent <= 0) {
-                log_syntax(unit, LOG_ERR, filename, line, 0, "CPU quota '%s' invalid. 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;
         }
 
-        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;
 }
 
@@ -2817,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_scale(r, 100U);
+
+                if (bytes < 1) {
+                        log_syntax(unit, LOG_ERR, filename, line, 0, "Memory limit '%s' too small. Ignoring.", rvalue);
                         return 0;
                 }
         }
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");
 
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))
diff --git a/src/login/logind-user.c b/src/login/logind-user.c
index a826321bf0..de44d369cf 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 = physical_memory_scale(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/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/shared/bus-unit-util.c b/src/shared/bus-unit-util.c
index 8f4f93ee0c..6fc201b885 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);
@@ -123,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);
@@ -166,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;
 
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;
 }
diff --git a/src/test/test-util.c b/src/test/test-util.c
index 9b6d2a7968..e177612a9f 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,53 @@ 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 (%" 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[]) {
         log_parse_environment();
         log_open();
@@ -277,6 +325,8 @@ int main(int argc, char *argv[]) {
         test_log2i();
         test_execute_directory();
         test_raw_clone();
+        test_physical_memory();
+        test_physical_memory_scale();
 
         return 0;
 }