From 9a0549093332880df47c4218209ce126b8586835 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 22 May 2014 11:53:12 +0900 Subject: [PATCH] cgroups: simplify CPUQuota= logic Only accept cpu quota values in percentages, get rid of period definition. It's not clear whether the CFS period controllable per-cgroup even has a future in the kernel, hence let's simplify all this, hardcode the period to 100ms and only accept percentage based quota values. --- man/systemd-system.conf.xml | 12 ------ man/systemd.resource-control.xml | 27 +++--------- src/core/cgroup.c | 47 +++------------------ src/core/cgroup.h | 5 --- src/core/dbus-cgroup.c | 61 +-------------------------- src/core/load-fragment-gperf.gperf.m4 | 3 +- src/core/load-fragment.c | 28 +++++------- src/core/main.c | 3 -- src/core/manager.c | 1 - src/core/manager.h | 1 - src/core/system.conf | 1 - src/core/unit.c | 1 - src/libsystemd/sd-bus/bus-util.c | 32 +------------- 13 files changed, 25 insertions(+), 197 deletions(-) diff --git a/man/systemd-system.conf.xml b/man/systemd-system.conf.xml index 3814bd2ad..e2b2bd8b6 100644 --- a/man/systemd-system.conf.xml +++ b/man/systemd-system.conf.xml @@ -301,18 +301,6 @@ above. - - DefaultCPUQuotaPeriodSec= - - Sets the default CPU - quota period. Defaults to 100ms. This - controls th global default for the - CPUQuotaPeriodSec= - setting of units, see - systemd.resource-control5 - for details. - - DefaultTimeoutStartSec= DefaultTimeoutStopSec= diff --git a/man/systemd.resource-control.xml b/man/systemd.resource-control.xml index a1f63dd66..e0e49db7d 100644 --- a/man/systemd.resource-control.xml +++ b/man/systemd.resource-control.xml @@ -143,14 +143,11 @@ along with systemd; If not, see . Assign the specified CPU time quota to the processes - executed. Takes a percentage value (suffixed with "%") or an - absolute time (suffixed by one of the common time units, us, - ms, s, ...). The percentage specifies how much CPU time the - unit shall get at maximum, relative to the total CPU time - available on one CPU. Use values > 100% for alloting CPU - time on more than one CPU. If an absolute time is specified - the processes of this unit will get this much absolute time - within each quota period, at maximum. This controls the + executed. Takes a percentage value, suffixed with "%". The + percentage specifies how much CPU time the unit shall get at + maximum, relative to the total CPU time available on one + CPU. Use values > 100% for alloting CPU time on more than + one CPU. This controls the cpu.cfs_quota_us control group attribute. For details about this control group attribute, see . - - CPUQuotaPeriodSec= - - - Specify the CPU quota period to use. Defaults to - 100ms. This controls the cpu.cfs_period_us - control group attribute. For details about this control - group attribute, see sched-design-CFS.txt. - - Implies CPUAccounting=true. - - - MemoryAccounting= diff --git a/src/core/cgroup.c b/src/core/cgroup.c index 36e9f53db..cd6796383 100644 --- a/src/core/cgroup.c +++ b/src/core/cgroup.c @@ -27,6 +27,8 @@ #include "cgroup-util.h" #include "cgroup.h" +#define CGROUP_CPU_QUOTA_PERIOD_USEC ((usec_t) 100 * USEC_PER_MSEC) + void cgroup_context_init(CGroupContext *c) { assert(c); @@ -40,8 +42,6 @@ void cgroup_context_init(CGroupContext *c) { c->startup_blockio_weight = (unsigned long) -1; c->cpu_quota_per_sec_usec = (usec_t) -1; - c->cpu_quota_usec = (usec_t) -1; - c->cpu_quota_period_usec = 100*USEC_PER_MSEC; } void cgroup_context_free_device_allow(CGroupContext *c, CGroupDeviceAllow *a) { @@ -84,37 +84,11 @@ void cgroup_context_done(CGroupContext *c) { cgroup_context_free_device_allow(c, c->device_allow); } -usec_t cgroup_context_get_cpu_quota_usec(CGroupContext *c) { - assert(c); - - /* Returns the absolute CPU quota */ - - if (c->cpu_quota_usec != (usec_t) -1) - return c->cpu_quota_usec; - else if (c->cpu_quota_per_sec_usec != (usec_t) -1) - return c->cpu_quota_per_sec_usec*c->cpu_quota_period_usec/USEC_PER_SEC; - else - return (usec_t) -1; -} - -usec_t cgroup_context_get_cpu_quota_per_sec_usec(CGroupContext *c) { - assert(c); - - /* Returns the CPU quota relative to 1s */ - - if (c->cpu_quota_usec != (usec_t) -1) - return c->cpu_quota_usec*USEC_PER_SEC/c->cpu_quota_period_usec; - else if (c->cpu_quota_per_sec_usec != (usec_t) -1) - return c->cpu_quota_per_sec_usec; - else - return (usec_t) -1; -} - void cgroup_context_dump(CGroupContext *c, FILE* f, const char *prefix) { CGroupBlockIODeviceBandwidth *b; CGroupBlockIODeviceWeight *w; CGroupDeviceAllow *a; - char t[FORMAT_TIMESPAN_MAX], s[FORMAT_TIMESPAN_MAX], u[FORMAT_TIMESPAN_MAX]; + char u[FORMAT_TIMESPAN_MAX]; assert(c); assert(f); @@ -127,9 +101,7 @@ void cgroup_context_dump(CGroupContext *c, FILE* f, const char *prefix) { "%sMemoryAccounting=%s\n" "%sCPUShares=%lu\n" "%sStartupCPUShares=%lu\n" - "%sCPUQuota=%s\n" "%sCPUQuotaPerSecSec=%s\n" - "%sCPUQuotaPeriodSec=%s\n" "%sBlockIOWeight=%lu\n" "%sStartupBlockIOWeight=%lu\n" "%sMemoryLimit=%" PRIu64 "\n" @@ -139,9 +111,7 @@ void cgroup_context_dump(CGroupContext *c, FILE* f, const char *prefix) { prefix, yes_no(c->memory_accounting), prefix, c->cpu_shares, prefix, c->startup_cpu_shares, - prefix, strna(format_timespan(u, sizeof(u), cgroup_context_get_cpu_quota_usec(c), 1)), - prefix, strna(format_timespan(t, sizeof(t), cgroup_context_get_cpu_quota_per_sec_usec(c), 1)), - prefix, strna(format_timespan(s, sizeof(s), c->cpu_quota_period_usec, 1)), + prefix, strna(format_timespan(u, sizeof(u), c->cpu_quota_per_sec_usec, 1)), prefix, c->blockio_weight, prefix, c->startup_blockio_weight, prefix, c->memory_limit, @@ -328,7 +298,6 @@ void cgroup_context_apply(CGroupContext *c, CGroupControllerMask mask, const cha if ((mask & CGROUP_CPU) && !is_root) { char buf[MAX(DECIMAL_STR_MAX(unsigned long), DECIMAL_STR_MAX(usec_t)) + 1]; - usec_t q; sprintf(buf, "%lu\n", state == MANAGER_STARTING && c->startup_cpu_shares != (unsigned long) -1 ? c->startup_cpu_shares : @@ -337,14 +306,13 @@ void cgroup_context_apply(CGroupContext *c, CGroupControllerMask mask, const cha if (r < 0) log_warning("Failed to set cpu.shares on %s: %s", path, strerror(-r)); - sprintf(buf, USEC_FMT "\n", c->cpu_quota_period_usec); + sprintf(buf, USEC_FMT "\n", CGROUP_CPU_QUOTA_PERIOD_USEC); r = cg_set_attribute("cpu", path, "cpu.cfs_period_us", buf); if (r < 0) log_warning("Failed to set cpu.cfs_period_us on %s: %s", path, strerror(-r)); - q = cgroup_context_get_cpu_quota_usec(c); - if (q != (usec_t) -1) { - sprintf(buf, USEC_FMT "\n", q); + if (c->cpu_quota_per_sec_usec != (usec_t) -1) { + sprintf(buf, USEC_FMT "\n", c->cpu_quota_per_sec_usec * CGROUP_CPU_QUOTA_PERIOD_USEC / USEC_PER_SEC); r = cg_set_attribute("cpu", path, "cpu.cfs_quota_us", buf); } else r = cg_set_attribute("cpu", path, "cpu.cfs_quota_us", "-1"); @@ -479,7 +447,6 @@ CGroupControllerMask cgroup_context_get_mask(CGroupContext *c) { if (c->cpu_accounting || c->cpu_shares != (unsigned long) -1 || c->startup_cpu_shares != (unsigned long) -1 || - c->cpu_quota_usec != (usec_t) -1 || c->cpu_quota_per_sec_usec != (usec_t) -1) mask |= CGROUP_CPUACCT | CGROUP_CPU; diff --git a/src/core/cgroup.h b/src/core/cgroup.h index 44482c508..d299872b1 100644 --- a/src/core/cgroup.h +++ b/src/core/cgroup.h @@ -73,8 +73,6 @@ struct CGroupContext { unsigned long cpu_shares; unsigned long startup_cpu_shares; usec_t cpu_quota_per_sec_usec; - usec_t cpu_quota_usec; - usec_t cpu_quota_period_usec; unsigned long blockio_weight; unsigned long startup_blockio_weight; @@ -102,9 +100,6 @@ void cgroup_context_free_device_allow(CGroupContext *c, CGroupDeviceAllow *a); void cgroup_context_free_blockio_device_weight(CGroupContext *c, CGroupBlockIODeviceWeight *w); void cgroup_context_free_blockio_device_bandwidth(CGroupContext *c, CGroupBlockIODeviceBandwidth *b); -usec_t cgroup_context_get_cpu_quota_usec(CGroupContext *c); -usec_t cgroup_context_get_cpu_quota_per_sec_usec(CGroupContext *c); - CGroupControllerMask unit_get_cgroup_mask(Unit *u); CGroupControllerMask unit_get_siblings_mask(Unit *u); CGroupControllerMask unit_get_members_mask(Unit *u); diff --git a/src/core/dbus-cgroup.c b/src/core/dbus-cgroup.c index ce7e55cb7..eb7c2b9d2 100644 --- a/src/core/dbus-cgroup.c +++ b/src/core/dbus-cgroup.c @@ -133,24 +133,6 @@ static int property_get_device_allow( return sd_bus_message_close_container(reply); } -static int property_get_cpu_quota_usec( - sd_bus *bus, - const char *path, - const char *interface, - const char *property, - sd_bus_message *reply, - void *userdata, - sd_bus_error *error) { - - CGroupContext *c = userdata; - - assert(bus); - assert(reply); - assert(c); - - return sd_bus_message_append(reply, "t", cgroup_context_get_cpu_quota_usec(c)); -} - static int property_get_cpu_quota_per_sec_usec( sd_bus *bus, const char *path, @@ -166,7 +148,7 @@ static int property_get_cpu_quota_per_sec_usec( assert(reply); assert(c); - return sd_bus_message_append(reply, "t", cgroup_context_get_cpu_quota_per_sec_usec(c)); + return sd_bus_message_append(reply, "t", c->cpu_quota_per_sec_usec); } static int property_get_ulong_as_u64( @@ -193,8 +175,6 @@ const sd_bus_vtable bus_cgroup_vtable[] = { SD_BUS_PROPERTY("CPUShares", "t", property_get_ulong_as_u64, offsetof(CGroupContext, cpu_shares), 0), SD_BUS_PROPERTY("StartupCPUShares", "t", property_get_ulong_as_u64, offsetof(CGroupContext, startup_cpu_shares), 0), SD_BUS_PROPERTY("CPUQuotaPerSecUSec", "t", property_get_cpu_quota_per_sec_usec, 0, 0), - SD_BUS_PROPERTY("CPUQuotaUSec", "t", property_get_cpu_quota_usec, 0, 0), - SD_BUS_PROPERTY("CPUQuotaPeriodUSec", "t", bus_property_get_usec, offsetof(CGroupContext, cpu_quota_period_usec), 0), SD_BUS_PROPERTY("BlockIOAccounting", "b", bus_property_get_bool, offsetof(CGroupContext, blockio_accounting), 0), SD_BUS_PROPERTY("BlockIOWeight", "t", property_get_ulong_as_u64, offsetof(CGroupContext, blockio_weight), 0), SD_BUS_PROPERTY("StartupBlockIOWeight", "t", property_get_ulong_as_u64, offsetof(CGroupContext, startup_blockio_weight), 0), @@ -298,51 +278,12 @@ int bus_cgroup_set_property( if (mode != UNIT_CHECK) { c->cpu_quota_per_sec_usec = u64; - c->cpu_quota_usec = (uint64_t) -1; u->cgroup_realized_mask &= ~CGROUP_CPU; unit_write_drop_in_private_format(u, mode, "CPUQuota", "CPUQuota=%0.f%%", (double) (c->cpu_quota_per_sec_usec / 10000)); } return 1; - } else if (streq(name, "CPUQuotaUSec")) { - uint64_t u64; - - r = sd_bus_message_read(message, "t", &u64); - if (r < 0) - return r; - - if (u64 <= 0) - return sd_bus_error_set_errnof(error, EINVAL, "CPUQuotaUSec value out of range"); - - if (mode != UNIT_CHECK) { - c->cpu_quota_usec = u64; - c->cpu_quota_per_sec_usec = (uint64_t) -1; - u->cgroup_realized_mask &= ~CGROUP_CPU; - unit_write_drop_in_private_format(u, mode, "CPUQuota", "CPUQuota=%" PRIu64 "us", u64); - } - - return 1; - - } else if (streq(name, "CPUQuotaPeriodUSec")) { - - uint64_t u64; - - r = sd_bus_message_read(message, "t", &u64); - if (r < 0) - return r; - - if (u64 <= 0 || u64 >= (usec_t) -1) - return sd_bus_error_set_errnof(error, EINVAL, "CPUQuotaPeriodUSec value out of range"); - - if (mode != UNIT_CHECK) { - c->cpu_quota_period_usec = u64; - u->cgroup_realized_mask &= ~CGROUP_CPU; - unit_write_drop_in_private_format(u, mode, name, "CPUQuotaPeriodSec=%" PRIu64 "us", c->cpu_quota_period_usec); - } - - return 1; - } else if (streq(name, "BlockIOAccounting")) { int b; diff --git a/src/core/load-fragment-gperf.gperf.m4 b/src/core/load-fragment-gperf.gperf.m4 index 524e5c76d..4c066a8b9 100644 --- a/src/core/load-fragment-gperf.gperf.m4 +++ b/src/core/load-fragment-gperf.gperf.m4 @@ -108,14 +108,13 @@ $1.CPUAccounting, config_parse_bool, 0, $1.CPUShares, config_parse_cpu_shares, 0, offsetof($1, cgroup_context.cpu_shares) $1.StartupCPUShares, config_parse_cpu_shares, 0, offsetof($1, cgroup_context.startup_cpu_shares) $1.CPUQuota, config_parse_cpu_quota, 0, offsetof($1, cgroup_context) -$1.CPUQuotaPeriodSec, config_parse_sec, 0, offsetof($1, cgroup_context.cpu_quota_period_usec) $1.MemoryAccounting, config_parse_bool, 0, offsetof($1, cgroup_context.memory_accounting) $1.MemoryLimit, config_parse_memory_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) $1.BlockIOWeight, config_parse_blockio_weight, 0, offsetof($1, cgroup_context.blockio_weight) -$1.StartupBlockIOWeight, config_parse_blockio_weight, 0, offsetof($1, cgroup_context.startup_blockio_weight) +$1.StartupBlockIOWeight, config_parse_blockio_weight, 0, offsetof($1, cgroup_context.startup_blockio_weight) $1.BlockIODeviceWeight, config_parse_blockio_device_weight, 0, offsetof($1, cgroup_context) $1.BlockIOReadBandwidth, config_parse_blockio_bandwidth, 0, offsetof($1, cgroup_context) $1.BlockIOWriteBandwidth, config_parse_blockio_bandwidth, 0, offsetof($1, cgroup_context)' diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c index 7c5b49c85..6403e4111 100644 --- a/src/core/load-fragment.c +++ b/src/core/load-fragment.c @@ -2465,7 +2465,7 @@ int config_parse_cpu_quota( void *userdata) { CGroupContext *c = data; - int r; + double percent; assert(filename); assert(lvalue); @@ -2473,30 +2473,22 @@ int config_parse_cpu_quota( if (isempty(rvalue)) { c->cpu_quota_per_sec_usec = (usec_t) -1; - c->cpu_quota_usec = (usec_t) -1; return 0; } - if (endswith(rvalue, "%")) { - double percent; - - if (sscanf(rvalue, "%lf%%", &percent) != 1 || percent <= 0) { - log_syntax(unit, LOG_ERR, filename, line, EINVAL, "CPU quota '%s' invalid. Ignoring.", rvalue); - return 0; - } + if (!endswith(rvalue, "%")) { - c->cpu_quota_per_sec_usec = (usec_t) (percent * USEC_PER_SEC / 100); - c->cpu_quota_usec = (usec_t) -1; - } else { - r = parse_sec(rvalue, &c->cpu_quota_usec); - if (r < 0) { - log_syntax(unit, LOG_ERR, filename, line, EINVAL, "CPU quota '%s' invalid. Ignoring.", rvalue); - return 0; - } + log_syntax(unit, LOG_ERR, filename, line, EINVAL, "CPU quota '%s' not ending in '%%'. Ignoring.", rvalue); + return 0; + } - c->cpu_quota_per_sec_usec = (usec_t) -1; + if (sscanf(rvalue, "%lf%%", &percent) != 1 || percent <= 0) { + log_syntax(unit, LOG_ERR, filename, line, EINVAL, "CPU quota '%s' invalid. Ignoring.", rvalue); + return 0; } + c->cpu_quota_per_sec_usec = (usec_t) (percent * USEC_PER_SEC / 100); + return 0; } diff --git a/src/core/main.c b/src/core/main.c index c1b0ffd9b..74c50f51b 100644 --- a/src/core/main.c +++ b/src/core/main.c @@ -109,7 +109,6 @@ static struct rlimit *arg_default_rlimit[_RLIMIT_MAX] = {}; static uint64_t arg_capability_bounding_set_drop = 0; static nsec_t arg_timer_slack_nsec = (nsec_t) -1; static usec_t arg_default_timer_accuracy_usec = 1 * USEC_PER_MINUTE; -static usec_t arg_default_cpu_quota_period_usec = 100 * USEC_PER_MSEC; static Set* arg_syscall_archs = NULL; static FILE* arg_serialization = NULL; static bool arg_default_cpu_accounting = false; @@ -684,7 +683,6 @@ static int parse_config_file(void) { #endif { "Manager", "TimerSlackNSec", config_parse_nsec, 0, &arg_timer_slack_nsec }, { "Manager", "DefaultTimerAccuracySec", config_parse_sec, 0, &arg_default_timer_accuracy_usec }, - { "Manager", "DefaultCPUQuotaPeriodSec", config_parse_sec, 0, &arg_default_cpu_quota_period_usec }, { "Manager", "DefaultStandardOutput", config_parse_output, 0, &arg_default_std_output }, { "Manager", "DefaultStandardError", config_parse_output, 0, &arg_default_std_error }, { "Manager", "DefaultTimeoutStartSec", config_parse_sec, 0, &arg_default_timeout_start_usec }, @@ -1619,7 +1617,6 @@ int main(int argc, char *argv[]) { m->confirm_spawn = arg_confirm_spawn; m->default_timer_accuracy_usec = arg_default_timer_accuracy_usec; - m->default_cpu_quota_period_usec = arg_default_cpu_quota_period_usec; m->default_std_output = arg_default_std_output; m->default_std_error = arg_default_std_error; m->default_restart_usec = arg_default_restart_usec; diff --git a/src/core/manager.c b/src/core/manager.c index 297574641..0cb204432 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -429,7 +429,6 @@ int manager_new(SystemdRunningAs running_as, Manager **_m) { m->running_as = running_as; m->exit_code = _MANAGER_EXIT_CODE_INVALID; m->default_timer_accuracy_usec = USEC_PER_MINUTE; - m->default_cpu_quota_period_usec = 100 * USEC_PER_MSEC; m->idle_pipe[0] = m->idle_pipe[1] = m->idle_pipe[2] = m->idle_pipe[3] = -1; diff --git a/src/core/manager.h b/src/core/manager.h index ab4cafc3a..f2c1b0d22 100644 --- a/src/core/manager.h +++ b/src/core/manager.h @@ -243,7 +243,6 @@ struct Manager { bool default_cpu_accounting; bool default_memory_accounting; bool default_blockio_accounting; - usec_t default_cpu_quota_period_usec; usec_t default_timer_accuracy_usec; diff --git a/src/core/system.conf b/src/core/system.conf index 4d775fafb..65a35a068 100644 --- a/src/core/system.conf +++ b/src/core/system.conf @@ -24,7 +24,6 @@ #SystemCallArchitectures= #TimerSlackNSec= #DefaultTimerAccuracySec=1min -#DefaultCPUQuotaPeriodSec=100ms #DefaultStandardOutput=journal #DefaultStandardError=inherit #DefaultTimeoutStartSec=90s diff --git a/src/core/unit.c b/src/core/unit.c index 99ee709ef..3f862aa4c 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -121,7 +121,6 @@ static void unit_init(Unit *u) { cc->cpu_accounting = u->manager->default_cpu_accounting; cc->blockio_accounting = u->manager->default_blockio_accounting; cc->memory_accounting = u->manager->default_memory_accounting; - cc->cpu_quota_period_usec = u->manager->default_cpu_quota_period_usec; } ec = unit_get_exec_context(u); diff --git a/src/libsystemd/sd-bus/bus-util.c b/src/libsystemd/sd-bus/bus-util.c index 6220934bd..6c9548f9a 100644 --- a/src/libsystemd/sd-bus/bus-util.c +++ b/src/libsystemd/sd-bus/bus-util.c @@ -1274,40 +1274,10 @@ int bus_append_unit_property_assignment(sd_bus_message *m, const char *assignmen r = sd_bus_message_append(m, "v", "t", (usec_t) percent * USEC_PER_SEC / 100); } else { - usec_t us; - - r = parse_sec(eq, &us); - if (r < 0) { - log_error("CPU quota '%s' invalid.", eq); - return -EINVAL; - } - - r = sd_bus_message_append_basic(m, SD_BUS_TYPE_STRING, "CPUQuotaUSec"); - if (r < 0) - return bus_log_create_error(r); - - r = sd_bus_message_append(m, "v", "t", us); - } - - if (r < 0) - return bus_log_create_error(r); - - return 0; - - } else if (streq(field, "CPUQuotaPeriodSec")) { - usec_t us; - - r = parse_sec(eq, &us); - if (r < 0) { - log_error("CPU period '%s' invalid.", eq); + log_error("CPU quota needs to be in percent."); return -EINVAL; } - r = sd_bus_message_append_basic(m, SD_BUS_TYPE_STRING, "CPUQuotaPeriodUSec"); - if (r < 0) - return bus_log_create_error(r); - - r = sd_bus_message_append(m, "v", "t", us); if (r < 0) return bus_log_create_error(r); -- 2.30.2