[systemd-devel] [PATCH 1/4] cgroups: support for MemoryAndSwapLimit= setting
Lennart Poettering
lennart at poettering.net
Thu Oct 10 07:03:20 PDT 2013
On Thu, 10.10.13 02:18, Mika Eloranta (mel at 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.
More information about the systemd-devel
mailing list