On Thu, 10.10.13 02:18, Mika Eloranta (m...@ohmu.fi) wrote: Mika,
so before we add properties for these settings we need to make sure they actually have a future in the kernel and are attributes that are going to stay supported. For example MemorySoftLimit is something we supported previously, but which I recently removed because Tejun Heo (the kernel cgroup maintainer, added to CC) suggested that the attribute wouldn't continue to exist on the kernel side or at least not in this form. Tejun, Mika sent patches to wrap memory.memsw.limit_in_bytes, memory.kmem.limit_in_bytes, memory.soft_limit_in_bytes, memory.kmem.tcp.limit_in_bytes in high-level systemd attributes. Could you comment on the future of these attributes in the kernel? Should we expose them in systemd? At the systemd hack fest in New Orleans we already discussed memory.soft_limit_in_bytes and memory.memsw.limit_in_bytes and you suggested not to expose them. What about the other two? (I have the suspicion though that if we want to expose something we probably want to expose a single knob that puts a limit on all kinds of memory, regardless of "RAM", "swap", "kernel" or "tcp"...) Thanks, Lennart > Add a MemoryAndSwapLimit setting that behaves the same way > as MemoryLimit, except that it controls the > memory.memsw.limit_in_bytes cgroup attribute. > --- > man/systemd.resource-control.xml | 9 +++++++-- > src/core/cgroup.c | 21 ++++++++++++++++++--- > src/core/cgroup.h | 1 + > src/core/dbus-cgroup.c | 1 + > src/core/load-fragment-gperf.gperf.m4 | 1 + > src/core/load-fragment.c | 34 ++++++++++++++++++++++++++++++++++ > src/core/load-fragment.h | 1 + > src/systemctl/systemctl.c | 2 +- > 8 files changed, 64 insertions(+), 6 deletions(-) > > diff --git a/man/systemd.resource-control.xml > b/man/systemd.resource-control.xml > index 8688905..606e078 100644 > --- a/man/systemd.resource-control.xml > +++ b/man/systemd.resource-control.xml > @@ -138,7 +138,7 @@ along with systemd; If not, see > <http://www.gnu.org/licenses/>. > </varlistentry> > > <varlistentry> > - > <term><varname>MemoryLimit=<replaceable>bytes</replaceable></varname></term> > + <term><varname>MemoryLimit=, > MemoryAndSwapLimit=<replaceable>bytes</replaceable></varname></term> > > <listitem> > <para>Specify the limit on maximum memory usage of the > @@ -149,7 +149,12 @@ along with systemd; If not, see > <http://www.gnu.org/licenses/>. > Megabytes, Gigabytes, or Terabytes (with the base 1024), > respectively. This controls the > <literal>memory.limit_in_bytes</literal> control group > - attribute. For details about this control group attribute, > + attribute. > + <literal>MemoryAndSwapLimit</literal> controls the > + <literal>memory.limit_in_bytes</literal> control group > + attribute, which sets the limit for the sum of the used > + memory and used swap space. > + For details about these control group attributes, > see <ulink > > url="https://www.kernel.org/doc/Documentation/cgroups/memory.txt";>memory.txt</ulink>.</para> > > diff --git a/src/core/cgroup.c b/src/core/cgroup.c > index 8bf4d89..3b465cc 100644 > --- a/src/core/cgroup.c > +++ b/src/core/cgroup.c > @@ -34,6 +34,7 @@ void cgroup_context_init(CGroupContext *c) { > > c->cpu_shares = 1024; > c->memory_limit = (uint64_t) -1; > + c->memory_and_swap_limit = (uint64_t) -1; > c->blockio_weight = 1000; > } > > @@ -94,6 +95,7 @@ void cgroup_context_dump(CGroupContext *c, FILE* f, const > char *prefix) { > "%sCPUShares=%lu\n" > "%sBlockIOWeight=%lu\n" > "%sMemoryLimit=%" PRIu64 "\n" > + "%sMemoryAndSwapLimit=%" PRIu64 "\n" > "%sDevicePolicy=%s\n", > prefix, yes_no(c->cpu_accounting), > prefix, yes_no(c->blockio_accounting), > @@ -101,6 +103,7 @@ void cgroup_context_dump(CGroupContext *c, FILE* f, const > char *prefix) { > prefix, c->cpu_shares, > prefix, c->blockio_weight, > prefix, c->memory_limit, > + prefix, c->memory_and_swap_limit, > prefix, cgroup_device_policy_to_string(c->device_policy)); > > LIST_FOREACH(device_allow, a, c->device_allow) > @@ -254,9 +257,8 @@ void cgroup_context_apply(CGroupContext *c, > CGroupControllerMask mask, const cha > } > > if (mask & CGROUP_MEMORY) { > + char buf[DECIMAL_STR_MAX(uint64_t) + 1]; > if (c->memory_limit != (uint64_t) -1) { > - char buf[DECIMAL_STR_MAX(uint64_t) + 1]; > - > sprintf(buf, "%" PRIu64 "\n", c->memory_limit); > r = cg_set_attribute("memory", path, > "memory.limit_in_bytes", buf); > } else > @@ -264,6 +266,18 @@ void cgroup_context_apply(CGroupContext *c, > CGroupControllerMask mask, const cha > > if (r < 0) > log_error("Failed to set memory.limit_in_bytes on > %s: %s", path, strerror(-r)); > + > + if (c->memory_and_swap_limit != (uint64_t) -1) { > + sprintf(buf, "%" PRIu64 "\n", > c->memory_and_swap_limit); > + r = cg_set_attribute("memory", path, > "memory.memsw.limit_in_bytes", buf); > + } else { > + r = cg_set_attribute("memory", path, > "memory.memsw.limit_in_bytes", "-1"); > + if (r == -EACCES) /* CONFIG_MEMCG_SWAP not built > into kernel, ignore */ > + r = 0; > + } > + > + if (r < 0) > + log_error("Failed to set memory.memsw.limit_in_bytes > on %s: %s", path, strerror(-r)); > } > > if (mask & CGROUP_DEVICE) { > @@ -326,7 +340,8 @@ CGroupControllerMask > cgroup_context_get_mask(CGroupContext *c) { > mask |= CGROUP_BLKIO; > > if (c->memory_accounting || > - c->memory_limit != (uint64_t) -1) > + c->memory_limit != (uint64_t) -1 || > + c->memory_and_swap_limit != (uint64_t) -1) > mask |= CGROUP_MEMORY; > > if (c->device_allow || c->device_policy != CGROUP_AUTO) > diff --git a/src/core/cgroup.h b/src/core/cgroup.h > index 0a079e9..c84eed2 100644 > --- a/src/core/cgroup.h > +++ b/src/core/cgroup.h > @@ -77,6 +77,7 @@ struct CGroupContext { > LIST_HEAD(CGroupBlockIODeviceBandwidth, blockio_device_bandwidths); > > uint64_t memory_limit; > + uint64_t memory_and_swap_limit; > > CGroupDevicePolicy device_policy; > LIST_HEAD(CGroupDeviceAllow, device_allow); > diff --git a/src/core/dbus-cgroup.c b/src/core/dbus-cgroup.c > index 9ebcad9..d86a03a 100644 > --- a/src/core/dbus-cgroup.c > +++ b/src/core/dbus-cgroup.c > @@ -133,6 +133,7 @@ const BusProperty bus_cgroup_context_properties[] = { > { "BlockIOWriteBandwidth", bus_cgroup_append_device_bandwidths, > "a(st)", 0 }, > { "MemoryAccounting", bus_property_append_bool, > "b", offsetof(CGroupContext, memory_accounting) }, > { "MemoryLimit", bus_property_append_uint64, > "t", offsetof(CGroupContext, memory_limit) }, > + { "MemoryAndSwapLimit", bus_property_append_uint64, > "t", offsetof(CGroupContext, memory_and_swap_limit) }, > { "DevicePolicy", bus_cgroup_append_device_policy, > "s", offsetof(CGroupContext, device_policy) }, > { "DeviceAllow", bus_cgroup_append_device_allow, > "a(ss)", 0 }, > {} > diff --git a/src/core/load-fragment-gperf.gperf.m4 > b/src/core/load-fragment-gperf.gperf.m4 > index 31fb7bc..f86352e 100644 > --- a/src/core/load-fragment-gperf.gperf.m4 > +++ b/src/core/load-fragment-gperf.gperf.m4 > @@ -89,6 +89,7 @@ $1.CPUAccounting, config_parse_bool, > 0, > $1.CPUShares, config_parse_cpu_shares, 0, > offsetof($1, cgroup_context) > $1.MemoryAccounting, config_parse_bool, 0, > offsetof($1, cgroup_context.memory_accounting) > $1.MemoryLimit, config_parse_memory_limit, 0, > offsetof($1, cgroup_context) > +$1.MemoryAndSwapLimit, config_parse_memory_and_swap_limit, 0, > offsetof($1, cgroup_context) > $1.DeviceAllow, config_parse_device_allow, 0, > offsetof($1, cgroup_context) > $1.DevicePolicy, config_parse_device_policy, 0, > offsetof($1, cgroup_context.device_policy) > $1.BlockIOAccounting, config_parse_bool, 0, > offsetof($1, cgroup_context.blockio_accounting) > diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c > index 44920d6..6dabf4c 100644 > --- a/src/core/load-fragment.c > +++ b/src/core/load-fragment.c > @@ -2072,6 +2072,39 @@ int config_parse_memory_limit( > return 0; > } > > +int config_parse_memory_and_swap_limit( > + const char *unit, > + const char *filename, > + unsigned line, > + const char *section, > + const char *lvalue, > + int ltype, > + const char *rvalue, > + void *data, > + void *userdata) { > + > + CGroupContext *c = data; > + off_t bytes; > + int r; > + > + if (isempty(rvalue)) { > + c->memory_and_swap_limit = (uint64_t) -1; > + return 0; > + } > + > + assert_cc(sizeof(uint64_t) == sizeof(off_t)); > + > + r = parse_bytes(rvalue, &bytes); > + if (r < 0) { > + log_syntax(unit, LOG_ERR, filename, line, EINVAL, > + "Memory and swap limit '%s' invalid. Ignoring.", > rvalue); > + return 0; > + } > + > + c->memory_and_swap_limit = (uint64_t) bytes; > + return 0; > +} > + > int config_parse_device_allow( > const char *unit, > const char *filename, > @@ -2712,6 +2745,7 @@ void unit_dump_config_items(FILE *f) { > { config_parse_syscall_filter, "SYSCALL" }, > { config_parse_cpu_shares, "SHARES" }, > { config_parse_memory_limit, "LIMIT" }, > + { config_parse_memory_and_swap_limit, "LIMIT" }, > { config_parse_device_allow, "DEVICE" }, > { config_parse_device_policy, "POLICY" }, > { config_parse_blockio_bandwidth, "BANDWIDTH" }, > diff --git a/src/core/load-fragment.h b/src/core/load-fragment.h > index 90e5e3a..846825c 100644 > --- a/src/core/load-fragment.h > +++ b/src/core/load-fragment.h > @@ -78,6 +78,7 @@ int config_parse_environ(const char *unit, const char > *filename, unsigned line, > int config_parse_unit_slice(const char *unit, const char *filename, unsigned > line, const char *section, const char *lvalue, int ltype, const char *rvalue, > void *data, void *userdata); > int config_parse_cpu_shares(const char *unit, const char *filename, unsigned > line, const char *section, const char *lvalue, int ltype, const char *rvalue, > void *data, void *userdata); > int config_parse_memory_limit(const char *unit, const char *filename, > unsigned line, const char *section, const char *lvalue, int ltype, const char > *rvalue, void *data, void *userdata); > +int config_parse_memory_and_swap_limit(const char *unit, const char > *filename, unsigned line, const char *section, const char *lvalue, int ltype, > const char *rvalue, void *data, void *userdata); > int config_parse_device_policy(const char *unit, const char *filename, > unsigned line, const char *section, const char *lvalue, int ltype, const char > *rvalue, void *data, void *userdata); > int config_parse_device_allow(const char *unit, const char *filename, > unsigned line, const char *section, const char *lvalue, int ltype, const char > *rvalue, void *data, void *userdata); > int config_parse_blockio_weight(const char *unit, const char *filename, > unsigned line, const char *section, const char *lvalue, int ltype, const char > *rvalue, void *data, void *userdata); > diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c > index d75281f..9e6bb06 100644 > --- a/src/systemctl/systemctl.c > +++ b/src/systemctl/systemctl.c > @@ -3658,7 +3658,7 @@ static int append_assignment(DBusMessageIter *iter, > const char *assignment) { > !dbus_message_iter_append_basic(&sub, DBUS_TYPE_BOOLEAN, > &b)) > return log_oom(); > > - } else if (streq(field, "MemoryLimit")) { > + } else if (streq(field, "MemoryLimit") || streq(field, > "MemoryAndSwapLimit")) { > off_t bytes; > uint64_t u; > Lennart -- Lennart Poettering - Red Hat, Inc. _______________________________________________ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
