diff --git a/src/basic/cpu-set-util.c b/src/basic/cpu-set-util.c index 271d82d769..9f0a61a18e 100644 --- a/src/basic/cpu-set-util.c +++ b/src/basic/cpu-set-util.c @@ -60,19 +60,19 @@ cpu_set_t* cpu_set_malloc(unsigned *ncpus) { } } -int parse_cpu_set_and_warn( +int parse_cpu_set_internal( const char *rvalue, cpu_set_t **cpu_set, + bool warn, const char *unit, const char *filename, unsigned line, const char *lvalue) { - const char *whole_rvalue = rvalue; _cleanup_cpu_free_ cpu_set_t *c = NULL; + const char *p = rvalue; unsigned ncpus = 0; - assert(lvalue); assert(rvalue); for (;;) { @@ -80,75 +80,34 @@ int parse_cpu_set_and_warn( unsigned cpu, cpu_lower, cpu_upper; int r; - r = extract_first_word(&rvalue, &word, WHITESPACE ",", EXTRACT_QUOTES); + r = extract_first_word(&p, &word, WHITESPACE ",", EXTRACT_QUOTES); + if (r == -ENOMEM) + return warn ? log_oom() : -ENOMEM; if (r < 0) - return log_syntax(unit, LOG_ERR, filename, line, r, "Invalid value for %s: %s", lvalue, whole_rvalue); + return warn ? log_syntax(unit, LOG_ERR, filename, line, r, "Invalid value for %s: %s", lvalue, rvalue) : r; if (r == 0) break; if (!c) { c = cpu_set_malloc(&ncpus); if (!c) - return log_oom(); + return warn ? log_oom() : -ENOMEM; } r = parse_range(word, &cpu_lower, &cpu_upper); if (r < 0) - return log_syntax(unit, LOG_ERR, filename, line, r, "Failed to parse CPU affinity '%s'", word); + return warn ? log_syntax(unit, LOG_ERR, filename, line, r, "Failed to parse CPU affinity '%s'", word) : r; if (cpu_lower >= ncpus || cpu_upper >= ncpus) - return log_syntax(unit, LOG_ERR, filename, line, EINVAL, "CPU out of range '%s' ncpus is %u", word, ncpus); + return warn ? log_syntax(unit, LOG_ERR, filename, line, EINVAL, "CPU out of range '%s' ncpus is %u", word, ncpus) : -EINVAL; - if (cpu_lower > cpu_upper) - log_syntax(unit, LOG_WARNING, filename, line, 0, "Range '%s' is invalid, %u > %u", word, cpu_lower, cpu_upper); - else - for (cpu = cpu_lower; cpu <= cpu_upper; cpu++) - CPU_SET_S(cpu, CPU_ALLOC_SIZE(ncpus), c); - } - - /* On success, sets *cpu_set and returns ncpus for the system. */ - if (c) { - *cpu_set = c; - c = NULL; - } - - return (int) ncpus; -} - -int parse_cpu_set( - const char *rvalue, - cpu_set_t **cpu_set) { - - _cleanup_cpu_free_ cpu_set_t *c = NULL; - unsigned ncpus = 0; - - assert(rvalue); - - for (;;) { - _cleanup_free_ char *word = NULL; - unsigned cpu, cpu_lower, cpu_upper; - int r; - - r = extract_first_word(&rvalue, &word, WHITESPACE ",", EXTRACT_QUOTES); - if (r == -ENOMEM) - return r; - if (r <= 0) - break; - - if (!c) { - c = cpu_set_malloc(&ncpus); - if (!c) - return -ENOMEM; - } - - r = parse_range(word, &cpu_lower, &cpu_upper); - if (r < 0) - return r; - if (cpu_lower >= ncpus || cpu_upper >= ncpus) - return -EINVAL; - - if (cpu_lower <= cpu_upper) - for (cpu = cpu_lower; cpu <= cpu_upper; cpu++) - CPU_SET_S(cpu, CPU_ALLOC_SIZE(ncpus), c); + if (cpu_lower > cpu_upper) { + if (warn) + log_syntax(unit, LOG_WARNING, filename, line, 0, "Range '%s' is invalid, %u > %u, ignoring", word, cpu_lower, cpu_upper); + continue; + } + + for (cpu = cpu_lower; cpu <= cpu_upper; cpu++) + CPU_SET_S(cpu, CPU_ALLOC_SIZE(ncpus), c); } /* On success, sets *cpu_set and returns ncpus for the system. */ diff --git a/src/basic/cpu-set-util.h b/src/basic/cpu-set-util.h index 043a5e4570..c98149eaeb 100644 --- a/src/basic/cpu-set-util.h +++ b/src/basic/cpu-set-util.h @@ -25,10 +25,31 @@ #include "macro.h" +#ifdef __NCPUBITS +#define CPU_SIZE_TO_NUM(n) ((n) * __NCPUBITS) +#else +#define CPU_SIZE_TO_NUM(n) ((n) * sizeof(cpu_set_t) * 8) +#endif + DEFINE_TRIVIAL_CLEANUP_FUNC(cpu_set_t*, CPU_FREE); #define _cleanup_cpu_free_ _cleanup_(CPU_FREEp) +static inline cpu_set_t* cpu_set_mfree(cpu_set_t *p) { + if (p) + CPU_FREE(p); + return NULL; +} + cpu_set_t* cpu_set_malloc(unsigned *ncpus); -int parse_cpu_set_and_warn(const char *rvalue, cpu_set_t **cpu_set, const char *unit, const char *filename, unsigned line, const char *lvalue); -int parse_cpu_set(const char *rvalue, cpu_set_t **cpu_set); +int parse_cpu_set_internal(const char *rvalue, cpu_set_t **cpu_set, bool warn, const char *unit, const char *filename, unsigned line, const char *lvalue); + +static inline int parse_cpu_set_and_warn(const char *rvalue, cpu_set_t **cpu_set, const char *unit, const char *filename, unsigned line, const char *lvalue) { + assert(lvalue); + + return parse_cpu_set_internal(rvalue, cpu_set, true, unit, filename, line, lvalue); +} + +static inline int parse_cpu_set(const char *rvalue, cpu_set_t **cpu_set){ + return parse_cpu_set_internal(rvalue, cpu_set, false, NULL, NULL, 0, NULL); +} diff --git a/src/core/dbus-execute.c b/src/core/dbus-execute.c index 197c1c3206..196de5658e 100644 --- a/src/core/dbus-execute.c +++ b/src/core/dbus-execute.c @@ -29,6 +29,7 @@ #include "bus-util.h" #include "cap-list.h" #include "capability-util.h" +#include "cpu-set-util.h" #include "dbus-execute.h" #include "env-util.h" #include "errno-list.h" @@ -1591,29 +1592,29 @@ int bus_exec_context_set_transient_property( if (!UNIT_WRITE_FLAGS_NOOP(flags)) { if (n == 0) { - c->cpuset = mfree(c->cpuset); + c->cpuset = cpu_set_mfree(c->cpuset); + c->cpuset_ncpus = 0; unit_write_settingf(u, flags, name, "%s=", name); } else { _cleanup_free_ char *str = NULL; - uint8_t *l; - size_t allocated = 0, len = 0, i; + size_t allocated = 0, len = 0, i, ncpus; - c->cpuset = (cpu_set_t*) memdup(a, sizeof(cpu_set_t) * n); - if (c->cpuset) - return -ENOMEM; + ncpus = CPU_SIZE_TO_NUM(n); - l = (uint8_t*) a; - for (i = 0; i < n; i++) { + for (i = 0; i < ncpus; i++) { _cleanup_free_ char *p = NULL; size_t add; - r = asprintf(&p, "%hhi", l[i]); + if (!CPU_ISSET_S(i, n, (cpu_set_t*) a)) + continue; + + r = asprintf(&p, "%zu", i); if (r < 0) return -ENOMEM; add = strlen(p); - if (GREEDY_REALLOC(str, allocated, len + add + 2)) + if (!GREEDY_REALLOC(str, allocated, len + add + 2)) return -ENOMEM; strcpy(mempcpy(str + len, p, add), " "); @@ -1623,6 +1624,25 @@ int bus_exec_context_set_transient_property( if (len != 0) str[len - 1] = '\0'; + if (!c->cpuset || c->cpuset_ncpus < ncpus) { + cpu_set_t *cpuset; + + cpuset = CPU_ALLOC(ncpus); + if (!cpuset) + return -ENOMEM; + + CPU_ZERO_S(n, cpuset); + if (c->cpuset) { + CPU_OR_S(CPU_ALLOC_SIZE(c->cpuset_ncpus), cpuset, c->cpuset, (cpu_set_t*) a); + CPU_FREE(c->cpuset); + } else + CPU_OR_S(n, cpuset, cpuset, (cpu_set_t*) a); + + c->cpuset = cpuset; + c->cpuset_ncpus = ncpus; + } else + CPU_OR_S(n, c->cpuset, c->cpuset, (cpu_set_t*) a); + unit_write_settingf(u, flags, name, "%s=%s", name, str); } } diff --git a/src/core/execute.c b/src/core/execute.c index 62f8ca7731..acd767cf77 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -66,6 +66,7 @@ #include "cap-list.h" #include "capability-util.h" #include "chown-recursive.h" +#include "cpu-set-util.h" #include "def.h" #include "env-util.h" #include "errno-list.h" @@ -3628,8 +3629,7 @@ void exec_context_done(ExecContext *c) { bind_mount_free_many(c->bind_mounts, c->n_bind_mounts); - if (c->cpuset) - CPU_FREE(c->cpuset); + c->cpuset = cpu_set_mfree(c->cpuset); c->utmp_id = mfree(c->utmp_id); c->selinux_context = mfree(c->selinux_context); diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c index d99176adbe..d6c616502b 100644 --- a/src/core/load-fragment.c +++ b/src/core/load-fragment.c @@ -1268,17 +1268,30 @@ int config_parse_exec_cpu_affinity(const char *unit, if (ncpus < 0) return ncpus; - if (c->cpuset) - CPU_FREE(c->cpuset); - - if (ncpus == 0) + if (ncpus == 0) { /* An empty assignment resets the CPU list */ - c->cpuset = NULL; - else { + c->cpuset = cpu_set_mfree(c->cpuset); + c->cpuset_ncpus = 0; + return 0; + } + + if (!c->cpuset) { c->cpuset = cpuset; cpuset = NULL; + c->cpuset_ncpus = (unsigned) ncpus; + return 0; } - c->cpuset_ncpus = ncpus; + + if (c->cpuset_ncpus < (unsigned) ncpus) { + CPU_OR_S(CPU_ALLOC_SIZE(c->cpuset_ncpus), cpuset, c->cpuset, cpuset); + CPU_FREE(c->cpuset); + c->cpuset = cpuset; + cpuset = NULL; + c->cpuset_ncpus = (unsigned) ncpus; + return 0; + } + + CPU_OR_S(CPU_ALLOC_SIZE((unsigned) ncpus), c->cpuset, c->cpuset, cpuset); return 0; } diff --git a/src/shared/bus-unit-util.c b/src/shared/bus-unit-util.c index 6ed0085953..643daa639b 100644 --- a/src/shared/bus-unit-util.c +++ b/src/shared/bus-unit-util.c @@ -705,8 +705,7 @@ int bus_append_unit_property_assignment(sd_bus_message *m, const char *assignmen if (r < 0) return bus_log_create_error(r); - if (cpuset) - sd_bus_message_append_array(m, 'y', cpuset, CPU_ALLOC_SIZE(ncpus)); + sd_bus_message_append_array(m, 'y', cpuset, CPU_ALLOC_SIZE(ncpus)); r = sd_bus_message_close_container(m); diff --git a/src/test/test-cpu-set-util.c b/src/test/test-cpu-set-util.c index 8de7bbe0df..1a7220d17a 100644 --- a/src/test/test-cpu-set-util.c +++ b/src/test/test-cpu-set-util.c @@ -33,7 +33,7 @@ static void test_parse_cpu_set(void) { assert_se(CPU_ISSET_S(1, CPU_ALLOC_SIZE(ncpus), c)); assert_se(CPU_ISSET_S(2, CPU_ALLOC_SIZE(ncpus), c)); assert_se(CPU_COUNT_S(CPU_ALLOC_SIZE(ncpus), c) == 2); - c = mfree(c); + c = cpu_set_mfree(c); /* A more interesting range */ ncpus = parse_cpu_set_and_warn("0 1 2 3 8 9 10 11", &c, NULL, "fake", 1, "CPUAffinity"); @@ -43,7 +43,7 @@ static void test_parse_cpu_set(void) { assert_se(CPU_ISSET_S(cpu, CPU_ALLOC_SIZE(ncpus), c)); for (cpu = 8; cpu < 12; cpu++) assert_se(CPU_ISSET_S(cpu, CPU_ALLOC_SIZE(ncpus), c)); - c = mfree(c); + c = cpu_set_mfree(c); /* Quoted strings */ ncpus = parse_cpu_set_and_warn("8 '9' 10 \"11\"", &c, NULL, "fake", 1, "CPUAffinity"); @@ -51,7 +51,7 @@ static void test_parse_cpu_set(void) { assert_se(CPU_COUNT_S(CPU_ALLOC_SIZE(ncpus), c) == 4); for (cpu = 8; cpu < 12; cpu++) assert_se(CPU_ISSET_S(cpu, CPU_ALLOC_SIZE(ncpus), c)); - c = mfree(c); + c = cpu_set_mfree(c); /* Use commas as separators */ ncpus = parse_cpu_set_and_warn("0,1,2,3 8,9,10,11", &c, NULL, "fake", 1, "CPUAffinity"); @@ -61,7 +61,7 @@ static void test_parse_cpu_set(void) { assert_se(CPU_ISSET_S(cpu, CPU_ALLOC_SIZE(ncpus), c)); for (cpu = 8; cpu < 12; cpu++) assert_se(CPU_ISSET_S(cpu, CPU_ALLOC_SIZE(ncpus), c)); - c = mfree(c); + c = cpu_set_mfree(c); /* Commas with spaces (and trailing comma, space) */ ncpus = parse_cpu_set_and_warn("0, 1, 2, 3, 4, 5, 6, 7, ", &c, NULL, "fake", 1, "CPUAffinity"); @@ -69,7 +69,7 @@ static void test_parse_cpu_set(void) { assert_se(CPU_COUNT_S(CPU_ALLOC_SIZE(ncpus), c) == 8); for (cpu = 0; cpu < 8; cpu++) assert_se(CPU_ISSET_S(cpu, CPU_ALLOC_SIZE(ncpus), c)); - c = mfree(c); + c = cpu_set_mfree(c); /* Ranges */ ncpus = parse_cpu_set_and_warn("0-3,8-11", &c, NULL, "fake", 1, "CPUAffinity"); @@ -79,7 +79,7 @@ static void test_parse_cpu_set(void) { assert_se(CPU_ISSET_S(cpu, CPU_ALLOC_SIZE(ncpus), c)); for (cpu = 8; cpu < 12; cpu++) assert_se(CPU_ISSET_S(cpu, CPU_ALLOC_SIZE(ncpus), c)); - c = mfree(c); + c = cpu_set_mfree(c); /* Ranges with trailing comma, space */ ncpus = parse_cpu_set_and_warn("0-3 8-11, ", &c, NULL, "fake", 1, "CPUAffinity"); @@ -89,13 +89,13 @@ static void test_parse_cpu_set(void) { assert_se(CPU_ISSET_S(cpu, CPU_ALLOC_SIZE(ncpus), c)); for (cpu = 8; cpu < 12; cpu++) assert_se(CPU_ISSET_S(cpu, CPU_ALLOC_SIZE(ncpus), c)); - c = mfree(c); + c = cpu_set_mfree(c); /* Negative range (returns empty cpu_set) */ ncpus = parse_cpu_set_and_warn("3-0", &c, NULL, "fake", 1, "CPUAffinity"); assert_se(ncpus >= 1024); assert_se(CPU_COUNT_S(CPU_ALLOC_SIZE(ncpus), c) == 0); - c = mfree(c); + c = cpu_set_mfree(c); /* Overlapping ranges */ ncpus = parse_cpu_set_and_warn("0-7 4-11", &c, NULL, "fake", 1, "CPUAffinity"); @@ -103,7 +103,7 @@ static void test_parse_cpu_set(void) { assert_se(CPU_COUNT_S(CPU_ALLOC_SIZE(ncpus), c) == 12); for (cpu = 0; cpu < 12; cpu++) assert_se(CPU_ISSET_S(cpu, CPU_ALLOC_SIZE(ncpus), c)); - c = mfree(c); + c = cpu_set_mfree(c); /* Mix ranges and individual CPUs */ ncpus = parse_cpu_set_and_warn("0,1 4-11", &c, NULL, "fake", 1, "CPUAffinity"); @@ -113,7 +113,7 @@ static void test_parse_cpu_set(void) { assert_se(CPU_ISSET_S(1, CPU_ALLOC_SIZE(ncpus), c)); for (cpu = 4; cpu < 12; cpu++) assert_se(CPU_ISSET_S(cpu, CPU_ALLOC_SIZE(ncpus), c)); - c = mfree(c); + c = cpu_set_mfree(c); /* Garbage */ ncpus = parse_cpu_set_and_warn("0 1 2 3 garbage", &c, NULL, "fake", 1, "CPUAffinity"); diff --git a/src/test/test-execute.c b/src/test/test-execute.c index 1ef6bb4efa..2bff00a9f7 100644 --- a/src/test/test-execute.c +++ b/src/test/test-execute.c @@ -24,6 +24,7 @@ #include #include +#include "cpu-set-util.h" #include "errno-list.h" #include "fileio.h" #include "fs-util.h" @@ -112,6 +113,30 @@ static void test_exec_bindpaths(Manager *m) { (void) rm_rf("/tmp/test-exec-bindreadonlypaths", REMOVE_ROOT|REMOVE_PHYSICAL); } +static void test_exec_cpuaffinity(Manager *m) { + _cleanup_cpu_free_ cpu_set_t *c = NULL; + unsigned n; + + assert_se(c = cpu_set_malloc(&n)); + assert_se(sched_getaffinity(0, CPU_ALLOC_SIZE(n), c) >= 0); + + if (CPU_ISSET_S(0, CPU_ALLOC_SIZE(n), c) == 0) { + log_notice("Cannot use CPU 0, skipping %s", __func__); + return; + } + + test(m, "exec-cpuaffinity1.service", 0, CLD_EXITED); + test(m, "exec-cpuaffinity2.service", 0, CLD_EXITED); + + if (CPU_ISSET_S(1, CPU_ALLOC_SIZE(n), c) == 0 || + CPU_ISSET_S(2, CPU_ALLOC_SIZE(n), c) == 0) { + log_notice("Cannot use CPU 1 or 2, skipping remaining tests in %s", __func__); + return; + } + + test(m, "exec-cpuaffinity3.service", 0, CLD_EXITED); +} + static void test_exec_workingdirectory(Manager *m) { assert_se(mkdir_p("/tmp/test-exec_workingdirectory", 0755) >= 0); @@ -521,6 +546,7 @@ int main(int argc, char *argv[]) { test_exec_bindpaths, test_exec_capabilityambientset, test_exec_capabilityboundingset, + test_exec_cpuaffinity, test_exec_environment, test_exec_environmentfile, test_exec_group, diff --git a/test/meson.build b/test/meson.build index c50757a46b..750f5c0379 100644 --- a/test/meson.build +++ b/test/meson.build @@ -54,6 +54,9 @@ test_data_files = ''' test-execute/exec-capabilityboundingset-merge.service test-execute/exec-capabilityboundingset-reset.service test-execute/exec-capabilityboundingset-simple.service + test-execute/exec-cpuaffinity1.service + test-execute/exec-cpuaffinity2.service + test-execute/exec-cpuaffinity3.service test-execute/exec-dynamicuser-fixeduser-one-supplementarygroup.service test-execute/exec-dynamicuser-fixeduser.service test-execute/exec-dynamicuser-statedir-migrate-step1.service diff --git a/test/test-execute/exec-cpuaffinity1.service b/test/test-execute/exec-cpuaffinity1.service new file mode 100644 index 0000000000..84d550a385 --- /dev/null +++ b/test/test-execute/exec-cpuaffinity1.service @@ -0,0 +1,6 @@ +[Unit] +Description=Test for CPUAffinity (simple) + +[Service] +ExecStart=/bin/sh -c 'test $$(cat /proc/self/status | grep Cpus_allowed: | rev | cut -c 1) = 1' +CPUAffinity=0 diff --git a/test/test-execute/exec-cpuaffinity2.service b/test/test-execute/exec-cpuaffinity2.service new file mode 100644 index 0000000000..0dda77f939 --- /dev/null +++ b/test/test-execute/exec-cpuaffinity2.service @@ -0,0 +1,8 @@ +[Unit] +Description=Test for CPUAffinity (reset) + +[Service] +ExecStart=/bin/sh -c 'test $$(cat /proc/self/status | grep Cpus_allowed: | rev | cut -c 1) = 1' +CPUAffinity=0-1 3 +CPUAffinity= +CPUAffinity=0 diff --git a/test/test-execute/exec-cpuaffinity3.service b/test/test-execute/exec-cpuaffinity3.service new file mode 100644 index 0000000000..4a45d3b2d5 --- /dev/null +++ b/test/test-execute/exec-cpuaffinity3.service @@ -0,0 +1,7 @@ +[Unit] +Description=Test for CPUAffinity (merge) + +[Service] +ExecStart=/bin/sh -c 'test $$(cat /proc/self/status | grep Cpus_allowed: | rev | cut -c 1) = 7' +CPUAffinity=0,1 +CPUAffinity=1-2