From bd0abfaea1514bdd7cb60228d7a3f94c17ba916d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 19 May 2019 16:05:02 +0200 Subject: [PATCH 01/17] core/dbus-execute: remove unnecessary initialization --- src/core/dbus-execute.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/dbus-execute.c b/src/core/dbus-execute.c index 48a2ebc5ef..8e83c6390e 100644 --- a/src/core/dbus-execute.c +++ b/src/core/dbus-execute.c @@ -1558,7 +1558,7 @@ int bus_exec_context_set_transient_property( #endif if (streq(name, "CPUAffinity")) { const void *a; - size_t n = 0; + size_t n; r = sd_bus_message_read_array(message, 'y', &a, &n); if (r < 0) From a832893f9c4f0a0329768e90f67e2fa24bb0008e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 19 May 2019 18:02:38 +0200 Subject: [PATCH 02/17] shared/cpu-set-util: move the part to print cpu-set into a separate function Also avoid unnecessary asprintf() when we can write to the output area directly. --- src/core/dbus-execute.c | 29 +++++------------------------ src/shared/cpu-set-util.c | 21 +++++++++++++++++++++ src/shared/cpu-set-util.h | 1 + src/test/test-cpu-set-util.c | 29 +++++++++++++++++++++++++++++ 4 files changed, 56 insertions(+), 24 deletions(-) diff --git a/src/core/dbus-execute.c b/src/core/dbus-execute.c index 8e83c6390e..5a56aad592 100644 --- a/src/core/dbus-execute.c +++ b/src/core/dbus-execute.c @@ -1571,33 +1571,14 @@ int bus_exec_context_set_transient_property( unit_write_settingf(u, flags, name, "%s=", name); } else { _cleanup_free_ char *str = NULL; - size_t allocated = 0, len = 0, i, ncpus; + size_t ncpus; + + str = cpu_set_to_string(a, n); + if (!str) + return -ENOMEM; ncpus = CPU_SIZE_TO_NUM(n); - for (i = 0; i < ncpus; i++) { - _cleanup_free_ char *p = NULL; - size_t add; - - 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)) - return -ENOMEM; - - strcpy(mempcpy(str + len, p, add), " "); - len += add + 1; - } - - if (len != 0) - str[len - 1] = '\0'; - if (!c->cpuset || c->cpuset_ncpus < ncpus) { cpu_set_t *cpuset; diff --git a/src/shared/cpu-set-util.c b/src/shared/cpu-set-util.c index 9a789ae756..aa5c4d110d 100644 --- a/src/shared/cpu-set-util.c +++ b/src/shared/cpu-set-util.c @@ -2,6 +2,7 @@ #include #include +#include #include #include "alloc-util.h" @@ -12,6 +13,26 @@ #include "parse-util.h" #include "string-util.h" +char* cpu_set_to_string(const cpu_set_t *set, size_t setsize) { + _cleanup_free_ char *str = NULL; + size_t allocated = 0, len = 0; + int i, r; + + for (i = 0; (size_t) i < setsize * 8; i++) { + if (!CPU_ISSET_S(i, setsize, set)) + continue; + + if (!GREEDY_REALLOC(str, allocated, len + 1 + DECIMAL_STR_MAX(int))) + return NULL; + + r = sprintf(str + len, len > 0 ? " %d" : "%d", i); + assert_se(r > 0); + len += r; + } + + return TAKE_PTR(str) ?: strdup(""); +} + cpu_set_t* cpu_set_malloc(unsigned *ncpus) { cpu_set_t *c; unsigned n = 1024; diff --git a/src/shared/cpu-set-util.h b/src/shared/cpu-set-util.h index 1b6bd35b1c..fea8ac8362 100644 --- a/src/shared/cpu-set-util.h +++ b/src/shared/cpu-set-util.h @@ -22,6 +22,7 @@ static inline cpu_set_t* cpu_set_mfree(cpu_set_t *p) { cpu_set_t* cpu_set_malloc(unsigned *ncpus); +char* cpu_set_to_string(const cpu_set_t *set, size_t setsize); 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) { diff --git a/src/test/test-cpu-set-util.c b/src/test/test-cpu-set-util.c index c9272459b4..ff5edb2a69 100644 --- a/src/test/test-cpu-set-util.c +++ b/src/test/test-cpu-set-util.c @@ -6,6 +6,7 @@ static void test_parse_cpu_set(void) { cpu_set_t *c = NULL; + _cleanup_free_ char *str = NULL; int ncpus; int cpu; @@ -15,6 +16,10 @@ 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); + + assert_se(str = cpu_set_to_string(c, CPU_ALLOC_SIZE(ncpus))); + log_info("cpu_set_to_string: %s", str); + str = mfree(str); c = cpu_set_mfree(c); /* A more interesting range */ @@ -25,6 +30,9 @@ 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)); + assert_se(str = cpu_set_to_string(c, CPU_ALLOC_SIZE(ncpus))); + log_info("cpu_set_to_string: %s", str); + str = mfree(str); c = cpu_set_mfree(c); /* Quoted strings */ @@ -33,6 +41,9 @@ 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)); + assert_se(str = cpu_set_to_string(c, CPU_ALLOC_SIZE(ncpus))); + log_info("cpu_set_to_string: %s", str); + str = mfree(str); c = cpu_set_mfree(c); /* Use commas as separators */ @@ -43,6 +54,9 @@ 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)); + assert_se(str = cpu_set_to_string(c, CPU_ALLOC_SIZE(ncpus))); + log_info("cpu_set_to_string: %s", str); + str = mfree(str); c = cpu_set_mfree(c); /* Commas with spaces (and trailing comma, space) */ @@ -51,6 +65,9 @@ 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)); + assert_se(str = cpu_set_to_string(c, CPU_ALLOC_SIZE(ncpus))); + log_info("cpu_set_to_string: %s", str); + str = mfree(str); c = cpu_set_mfree(c); /* Ranges */ @@ -61,6 +78,9 @@ 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)); + assert_se(str = cpu_set_to_string(c, CPU_ALLOC_SIZE(ncpus))); + log_info("cpu_set_to_string: %s", str); + str = mfree(str); c = cpu_set_mfree(c); /* Ranges with trailing comma, space */ @@ -71,6 +91,9 @@ 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)); + assert_se(str = cpu_set_to_string(c, CPU_ALLOC_SIZE(ncpus))); + log_info("cpu_set_to_string: %s", str); + str = mfree(str); c = cpu_set_mfree(c); /* Negative range (returns empty cpu_set) */ @@ -85,6 +108,9 @@ 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)); + assert_se(str = cpu_set_to_string(c, CPU_ALLOC_SIZE(ncpus))); + log_info("cpu_set_to_string: %s", str); + str = mfree(str); c = cpu_set_mfree(c); /* Mix ranges and individual CPUs */ @@ -95,6 +121,9 @@ 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)); + assert_se(str = cpu_set_to_string(c, CPU_ALLOC_SIZE(ncpus))); + log_info("cpu_set_to_string: %s", str); + str = mfree(str); c = cpu_set_mfree(c); /* Garbage */ From b12ef7141648be40fd8c4b0209a742f2151736d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 19 May 2019 18:08:39 +0200 Subject: [PATCH 03/17] shared/cpu-set-util: remove now-unused CPU_SIZE_TO_NUM() --- src/shared/cpu-set-util.h | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/shared/cpu-set-util.h b/src/shared/cpu-set-util.h index fea8ac8362..9574b81582 100644 --- a/src/shared/cpu-set-util.h +++ b/src/shared/cpu-set-util.h @@ -5,12 +5,6 @@ #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) From 0985c7c4e22c8dbbea4398cf3453da45ebf63800 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 21 May 2019 08:45:19 +0200 Subject: [PATCH 04/17] Rework cpu affinity parsing The CPU_SET_S api is pretty bad. In particular, it has a parameter for the size of the array, but operations which take two (CPU_EQUAL_S) or even three arrays (CPU_{AND,OR,XOR}_S) still take just one size. This means that all arrays must be of the same size, or buffer overruns will occur. This is exactly what our code would do, if it received an array of unexpected size over the network. ("Unexpected" here means anything different from what cpu_set_malloc() detects as the "right" size.) Let's rework this, and store the size in bytes of the allocated storage area. The code will now parse any number up to 8191, independently of what the current kernel supports. This matches the kernel maximum setting for any architecture, to make things more portable. Fixes #12605. --- src/core/dbus-execute.c | 35 ++----- src/core/execute.c | 12 +-- src/core/execute.h | 4 +- src/core/load-fragment.c | 31 +------ src/core/main.c | 14 +-- src/nspawn/nspawn-oci.c | 25 +++-- src/nspawn/nspawn-settings.c | 33 +------ src/nspawn/nspawn-settings.h | 4 +- src/nspawn/nspawn.c | 29 +++--- src/shared/bus-unit-util.c | 4 +- src/shared/cpu-set-util.c | 133 +++++++++++++++++++++----- src/shared/cpu-set-util.h | 45 ++++++--- src/test/test-cpu-set-util.c | 175 +++++++++++++++++++---------------- src/test/test-sizeof.c | 3 + 14 files changed, 293 insertions(+), 254 deletions(-) diff --git a/src/core/dbus-execute.c b/src/core/dbus-execute.c index 5a56aad592..326e451b9a 100644 --- a/src/core/dbus-execute.c +++ b/src/core/dbus-execute.c @@ -219,7 +219,7 @@ static int property_get_cpu_affinity( assert(reply); assert(c); - return sd_bus_message_append_array(reply, 'y', c->cpuset, CPU_ALLOC_SIZE(c->cpuset_ncpus)); + return sd_bus_message_append_array(reply, 'y', c->cpu_set.set, c->cpu_set.allocated); } static int property_get_timer_slack_nsec( @@ -1566,37 +1566,22 @@ int bus_exec_context_set_transient_property( if (!UNIT_WRITE_FLAGS_NOOP(flags)) { if (n == 0) { - c->cpuset = cpu_set_mfree(c->cpuset); - c->cpuset_ncpus = 0; + cpu_set_reset(&c->cpu_set); unit_write_settingf(u, flags, name, "%s=", name); } else { _cleanup_free_ char *str = NULL; - size_t ncpus; + const CPUSet set = {(cpu_set_t*) a, n}; - str = cpu_set_to_string(a, n); + str = cpu_set_to_string(&set); if (!str) return -ENOMEM; - ncpus = CPU_SIZE_TO_NUM(n); - - 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); + /* We forego any optimizations here, and always create the structure using + * cpu_set_add_all(), because we don't want to care if the existing size we + * got over dbus is appropriate. */ + r = cpu_set_add_all(&c->cpu_set, &set); + if (r < 0) + return r; unit_write_settingf(u, flags, name, "%s=%s", name, str); } diff --git a/src/core/execute.c b/src/core/execute.c index a8b6c92873..01d36c1831 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -3144,8 +3144,8 @@ static int exec_child( } } - if (context->cpuset) - if (sched_setaffinity(0, CPU_ALLOC_SIZE(context->cpuset_ncpus), context->cpuset) < 0) { + if (context->cpu_set.set) + if (sched_setaffinity(0, context->cpu_set.allocated, context->cpu_set.set) < 0) { *exit_status = EXIT_CPUAFFINITY; return log_unit_error_errno(unit, errno, "Failed to set up CPU affinity: %m"); } @@ -3896,7 +3896,7 @@ void exec_context_done(ExecContext *c) { c->temporary_filesystems = NULL; c->n_temporary_filesystems = 0; - c->cpuset = cpu_set_mfree(c->cpuset); + cpu_set_reset(&c->cpu_set); c->utmp_id = mfree(c->utmp_id); c->selinux_context = mfree(c->selinux_context); @@ -4328,10 +4328,10 @@ void exec_context_dump(const ExecContext *c, FILE* f, const char *prefix) { prefix, yes_no(c->cpu_sched_reset_on_fork)); } - if (c->cpuset) { + if (c->cpu_set.set) { fprintf(f, "%sCPUAffinity:", prefix); - for (i = 0; i < c->cpuset_ncpus; i++) - if (CPU_ISSET_S(i, CPU_ALLOC_SIZE(c->cpuset_ncpus), c->cpuset)) + for (i = 0; i < c->cpu_set.allocated * 8; i++) + if (CPU_ISSET_S(i, c->cpu_set.allocated, c->cpu_set.set)) fprintf(f, " %u", i); fputs("\n", f); } diff --git a/src/core/execute.h b/src/core/execute.h index 23bf3b546a..7ddc36e6f3 100644 --- a/src/core/execute.h +++ b/src/core/execute.h @@ -14,6 +14,7 @@ typedef struct Manager Manager; #include #include "cgroup-util.h" +#include "cpu-set-util.h" #include "fdset.h" #include "list.h" #include "missing_resource.h" @@ -172,8 +173,7 @@ struct ExecContext { int cpu_sched_policy; int cpu_sched_priority; - unsigned cpuset_ncpus; - cpu_set_t *cpuset; + CPUSet cpu_set; ExecInput std_input; ExecOutput std_output; diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c index 11a9a7bdeb..bf414e62f1 100644 --- a/src/core/load-fragment.c +++ b/src/core/load-fragment.c @@ -1263,42 +1263,13 @@ int config_parse_exec_cpu_affinity(const char *unit, void *userdata) { ExecContext *c = data; - _cleanup_cpu_free_ cpu_set_t *cpuset = NULL; - int ncpus; assert(filename); assert(lvalue); assert(rvalue); assert(data); - ncpus = parse_cpu_set_and_warn(rvalue, &cpuset, unit, filename, line, lvalue); - if (ncpus < 0) - return ncpus; - - if (ncpus == 0) { - /* An empty assignment resets the CPU list */ - c->cpuset = cpu_set_mfree(c->cpuset); - c->cpuset_ncpus = 0; - return 0; - } - - if (!c->cpuset) { - c->cpuset = TAKE_PTR(cpuset); - c->cpuset_ncpus = (unsigned) ncpus; - return 0; - } - - 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 = TAKE_PTR(cpuset); - c->cpuset_ncpus = (unsigned) ncpus; - return 0; - } - - CPU_OR_S(CPU_ALLOC_SIZE((unsigned) ncpus), c->cpuset, c->cpuset, cpuset); - - return 0; + return parse_cpu_set_extend(rvalue, &c->cpu_set, true, unit, filename, line, lvalue); } int config_parse_capability_set( diff --git a/src/core/main.c b/src/core/main.c index e34da11984..67a62a5ad2 100644 --- a/src/core/main.c +++ b/src/core/main.c @@ -567,16 +567,18 @@ static int config_parse_cpu_affinity2( void *data, void *userdata) { - _cleanup_cpu_free_ cpu_set_t *c = NULL; - int ncpus; + _cleanup_(cpu_set_reset) CPUSet c = {}; + int r; - ncpus = parse_cpu_set_and_warn(rvalue, &c, unit, filename, line, lvalue); - if (ncpus < 0) - return ncpus; + r = parse_cpu_set_full(rvalue, &c, true, unit, filename, line, lvalue); + if (r < 0) + return r; - if (sched_setaffinity(0, CPU_ALLOC_SIZE(ncpus), c) < 0) + if (sched_setaffinity(0, c.allocated, c.set) < 0) log_warning_errno(errno, "Failed to set CPU affinity: %m"); + // FIXME: parsing and execution should be seperated. + return 0; } diff --git a/src/nspawn/nspawn-oci.c b/src/nspawn/nspawn-oci.c index 97323f31dd..f5e52bef5e 100644 --- a/src/nspawn/nspawn-oci.c +++ b/src/nspawn/nspawn-oci.c @@ -1266,8 +1266,7 @@ struct cpu_data { uint64_t shares; uint64_t quota; uint64_t period; - cpu_set_t *cpuset; - unsigned ncpus; + CPUSet cpu_set; }; static int oci_cgroup_cpu_shares(const char *name, JsonVariant *v, JsonDispatchFlags flags, void *userdata) { @@ -1302,21 +1301,20 @@ static int oci_cgroup_cpu_quota(const char *name, JsonVariant *v, JsonDispatchFl static int oci_cgroup_cpu_cpus(const char *name, JsonVariant *v, JsonDispatchFlags flags, void *userdata) { struct cpu_data *data = userdata; - cpu_set_t *set; + CPUSet set; const char *n; - int ncpus; + int r; assert(data); assert_se(n = json_variant_string(v)); - ncpus = parse_cpu_set(n, &set); - if (ncpus < 0) - return json_log(v, flags, ncpus, "Failed to parse CPU set specification: %s", n); + r = parse_cpu_set(n, &set); + if (r < 0) + return json_log(v, flags, r, "Failed to parse CPU set specification: %s", n); - CPU_FREE(data->cpuset); - data->cpuset = set; - data->ncpus = ncpus; + cpu_set_reset(&data->cpu_set); + data->cpu_set = set; return 0; } @@ -1345,13 +1343,12 @@ static int oci_cgroup_cpu(const char *name, JsonVariant *v, JsonDispatchFlags fl r = json_dispatch(v, table, oci_unexpected, flags, &data); if (r < 0) { - CPU_FREE(data.cpuset); + cpu_set_reset(&data.cpu_set); return r; } - CPU_FREE(s->cpuset); - s->cpuset = data.cpuset; - s->cpuset_ncpus = data.ncpus; + cpu_set_reset(&s->cpu_set); + s->cpu_set = data.cpu_set; if (data.shares != UINT64_MAX) { r = settings_allocate_properties(s); diff --git a/src/nspawn/nspawn-settings.c b/src/nspawn/nspawn-settings.c index 476cb0779e..9ff37c6dbd 100644 --- a/src/nspawn/nspawn-settings.c +++ b/src/nspawn/nspawn-settings.c @@ -133,7 +133,7 @@ Settings* settings_free(Settings *s) { strv_free(s->syscall_blacklist); rlimit_free_all(s->rlimit); free(s->hostname); - s->cpuset = cpu_set_mfree(s->cpuset); + cpu_set_reset(&s->cpu_set); strv_free(s->network_interfaces); strv_free(s->network_macvlan); @@ -803,41 +803,12 @@ int config_parse_cpu_affinity( void *data, void *userdata) { - _cleanup_cpu_free_ cpu_set_t *cpuset = NULL; Settings *settings = data; - int ncpus; assert(rvalue); assert(settings); - ncpus = parse_cpu_set_and_warn(rvalue, &cpuset, unit, filename, line, lvalue); - if (ncpus < 0) - return ncpus; - - if (ncpus == 0) { - /* An empty assignment resets the CPU list */ - settings->cpuset = cpu_set_mfree(settings->cpuset); - settings->cpuset_ncpus = 0; - return 0; - } - - if (!settings->cpuset) { - settings->cpuset = TAKE_PTR(cpuset); - settings->cpuset_ncpus = (unsigned) ncpus; - return 0; - } - - if (settings->cpuset_ncpus < (unsigned) ncpus) { - CPU_OR_S(CPU_ALLOC_SIZE(settings->cpuset_ncpus), cpuset, settings->cpuset, cpuset); - CPU_FREE(settings->cpuset); - settings->cpuset = TAKE_PTR(cpuset); - settings->cpuset_ncpus = (unsigned) ncpus; - return 0; - } - - CPU_OR_S(CPU_ALLOC_SIZE((unsigned) ncpus), settings->cpuset, settings->cpuset, cpuset); - - return 0; + return parse_cpu_set_extend(rvalue, &settings->cpu_set, true, unit, filename, line, lvalue); } DEFINE_CONFIG_PARSE_ENUM(config_parse_resolv_conf, resolv_conf_mode, ResolvConfMode, "Failed to parse resolv.conf mode"); diff --git a/src/nspawn/nspawn-settings.h b/src/nspawn/nspawn-settings.h index 64910c3ecc..f1a1a75466 100644 --- a/src/nspawn/nspawn-settings.h +++ b/src/nspawn/nspawn-settings.h @@ -13,6 +13,7 @@ #include "capability-util.h" #include "conf-parser.h" +#include "cpu-set-util.h" #include "macro.h" #include "missing_resource.h" #include "nspawn-expose-ports.h" @@ -163,8 +164,7 @@ typedef struct Settings { int no_new_privileges; int oom_score_adjust; bool oom_score_adjust_set; - cpu_set_t *cpuset; - unsigned cpuset_ncpus; + CPUSet cpu_set; ResolvConfMode resolv_conf; LinkJournal link_journal; bool link_journal_try; diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c index 27829431ac..25184e11e9 100644 --- a/src/nspawn/nspawn.c +++ b/src/nspawn/nspawn.c @@ -220,8 +220,7 @@ static struct rlimit *arg_rlimit[_RLIMIT_MAX] = {}; static bool arg_no_new_privileges = false; static int arg_oom_score_adjust = 0; static bool arg_oom_score_adjust_set = false; -static cpu_set_t *arg_cpuset = NULL; -static unsigned arg_cpuset_ncpus = 0; +static CPUSet arg_cpu_set = {}; static ResolvConfMode arg_resolv_conf = RESOLV_CONF_AUTO; static TimezoneMode arg_timezone = TIMEZONE_AUTO; static unsigned arg_console_width = (unsigned) -1, arg_console_height = (unsigned) -1; @@ -259,7 +258,7 @@ STATIC_DESTRUCTOR_REGISTER(arg_syscall_blacklist, strv_freep); #if HAVE_SECCOMP STATIC_DESTRUCTOR_REGISTER(arg_seccomp, seccomp_releasep); #endif -STATIC_DESTRUCTOR_REGISTER(arg_cpuset, CPU_FREEp); +STATIC_DESTRUCTOR_REGISTER(arg_cpu_set, cpu_set_reset); STATIC_DESTRUCTOR_REGISTER(arg_sysctl, strv_freep); static int help(void) { @@ -1329,17 +1328,14 @@ static int parse_argv(int argc, char *argv[]) { break; case ARG_CPU_AFFINITY: { - _cleanup_cpu_free_ cpu_set_t *cpuset = NULL; + CPUSet cpuset; r = parse_cpu_set(optarg, &cpuset); if (r < 0) - return log_error_errno(r, "Failed to parse CPU affinity mask: %s", optarg); + return log_error_errno(r, "Failed to parse CPU affinity mask %s: %m", optarg); - if (arg_cpuset) - CPU_FREE(arg_cpuset); - - arg_cpuset = TAKE_PTR(cpuset); - arg_cpuset_ncpus = r; + cpu_set_reset(&arg_cpu_set); + arg_cpu_set = cpuset; arg_settings_mask |= SETTING_CPU_AFFINITY; break; } @@ -2922,8 +2918,8 @@ static int inner_child( return log_error_errno(r, "Failed to adjust OOM score: %m"); } - if (arg_cpuset) - if (sched_setaffinity(0, CPU_ALLOC_SIZE(arg_cpuset_ncpus), arg_cpuset) < 0) + if (arg_cpu_set.set) + if (sched_setaffinity(0, arg_cpu_set.allocated, arg_cpu_set.set) < 0) return log_error_errno(errno, "Failed to set CPU affinity: %m"); (void) setup_hostname(); @@ -3869,15 +3865,14 @@ static int merge_settings(Settings *settings, const char *path) { } if ((arg_settings_mask & SETTING_CPU_AFFINITY) == 0 && - settings->cpuset) { + settings->cpu_set.set) { if (!arg_settings_trusted) log_warning("Ignoring CPUAffinity= setting, file '%s' is not trusted.", path); else { - if (arg_cpuset) - CPU_FREE(arg_cpuset); - arg_cpuset = TAKE_PTR(settings->cpuset); - arg_cpuset_ncpus = settings->cpuset_ncpus; + cpu_set_reset(&arg_cpu_set); + arg_cpu_set = settings->cpu_set; + settings->cpu_set = (CPUSet) {}; } } diff --git a/src/shared/bus-unit-util.c b/src/shared/bus-unit-util.c index 2b425efc9c..e4889dc55d 100644 --- a/src/shared/bus-unit-util.c +++ b/src/shared/bus-unit-util.c @@ -992,13 +992,13 @@ static int bus_append_execute_property(sd_bus_message *m, const char *field, con } if (streq(field, "CPUAffinity")) { - _cleanup_cpu_free_ cpu_set_t *cpuset = NULL; + _cleanup_(cpu_set_reset) CPUSet cpuset = {}; r = parse_cpu_set(eq, &cpuset); if (r < 0) return log_error_errno(r, "Failed to parse %s value: %s", field, eq); - return bus_append_byte_array(m, field, cpuset, CPU_ALLOC_SIZE(r)); + return bus_append_byte_array(m, field, cpuset.set, cpuset.allocated); } if (STR_IN_SET(field, "RestrictAddressFamilies", "SystemCallFilter")) { diff --git a/src/shared/cpu-set-util.c b/src/shared/cpu-set-util.c index aa5c4d110d..b8d13a2ba6 100644 --- a/src/shared/cpu-set-util.c +++ b/src/shared/cpu-set-util.c @@ -10,16 +10,17 @@ #include "extract-word.h" #include "log.h" #include "macro.h" +#include "memory-util.h" #include "parse-util.h" #include "string-util.h" -char* cpu_set_to_string(const cpu_set_t *set, size_t setsize) { +char* cpu_set_to_string(const CPUSet *a) { _cleanup_free_ char *str = NULL; size_t allocated = 0, len = 0; int i, r; - for (i = 0; (size_t) i < setsize * 8; i++) { - if (!CPU_ISSET_S(i, setsize, set)) + for (i = 0; (size_t) i < a->allocated * 8; i++) { + if (!CPU_ISSET_S(i, a->allocated, a->set)) continue; if (!GREEDY_REALLOC(str, allocated, len + 1 + DECIMAL_STR_MAX(int))) @@ -62,24 +63,74 @@ cpu_set_t* cpu_set_malloc(unsigned *ncpus) { } } -int parse_cpu_set_internal( +static int cpu_set_realloc(CPUSet *cpu_set, unsigned ncpus) { + size_t need; + + assert(cpu_set); + + need = CPU_ALLOC_SIZE(ncpus); + if (need > cpu_set->allocated) { + cpu_set_t *t; + + t = realloc(cpu_set->set, need); + if (!t) + return -ENOMEM; + + memzero((uint8_t*) t + cpu_set->allocated, need - cpu_set->allocated); + + cpu_set->set = t; + cpu_set->allocated = need; + } + + return 0; +} + +static int cpu_set_add(CPUSet *cpu_set, unsigned cpu) { + int r; + + if (cpu >= 8192) + /* As of kernel 5.1, CONFIG_NR_CPUS can be set to 8192 on PowerPC */ + return -ERANGE; + + r = cpu_set_realloc(cpu_set, cpu + 1); + if (r < 0) + return r; + + CPU_SET_S(cpu, cpu_set->allocated, cpu_set->set); + return 0; +} + +int cpu_set_add_all(CPUSet *a, const CPUSet *b) { + int r; + + /* Do this backwards, so if we fail, we fail before changing anything. */ + for (unsigned cpu_p1 = b->allocated * 8; cpu_p1 > 0; cpu_p1--) + if (CPU_ISSET_S(cpu_p1 - 1, b->allocated, b->set)) { + r = cpu_set_add(a, cpu_p1 - 1); + if (r < 0) + return r; + } + + return 0; +} + +int parse_cpu_set_full( const char *rvalue, - cpu_set_t **cpu_set, + CPUSet *cpu_set, bool warn, const char *unit, const char *filename, unsigned line, const char *lvalue) { - _cleanup_cpu_free_ cpu_set_t *c = NULL; + _cleanup_(cpu_set_reset) CPUSet c = {}; const char *p = rvalue; - unsigned ncpus = 0; - assert(rvalue); + assert(p); for (;;) { _cleanup_free_ char *word = NULL; - unsigned cpu, cpu_lower, cpu_upper; + unsigned cpu_lower, cpu_upper; int r; r = extract_first_word(&p, &word, WHITESPACE ",", EXTRACT_QUOTES); @@ -90,31 +141,63 @@ int parse_cpu_set_internal( if (r == 0) break; - if (!c) { - c = cpu_set_malloc(&ncpus); - if (!c) - return warn ? log_oom() : -ENOMEM; - } - r = parse_range(word, &cpu_lower, &cpu_upper); if (r < 0) 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 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) { if (warn) - log_syntax(unit, LOG_WARNING, filename, line, 0, "Range '%s' is invalid, %u > %u, ignoring", word, cpu_lower, cpu_upper); - continue; + log_syntax(unit, LOG_WARNING, filename, line, 0, "Range '%s' is invalid, %u > %u, ignoring.", + word, cpu_lower, cpu_upper); + + /* Make sure something is allocated, to distinguish this from the empty case */ + r = cpu_set_realloc(&c, 1); + if (r < 0) + return r; } - for (cpu = cpu_lower; cpu <= cpu_upper; cpu++) - CPU_SET_S(cpu, CPU_ALLOC_SIZE(ncpus), c); + for (unsigned cpu_p1 = MIN(cpu_upper, UINT_MAX-1) + 1; cpu_p1 > cpu_lower; cpu_p1--) { + r = cpu_set_add(&c, cpu_p1 - 1); + if (r < 0) + return warn ? log_syntax(unit, LOG_ERR, filename, line, r, + "Cannot add CPU %u to set: %m", cpu_p1 - 1) : r; + } } - /* On success, sets *cpu_set and returns ncpus for the system. */ - if (c) - *cpu_set = TAKE_PTR(c); + /* On success, transfer ownership to the output variable */ + *cpu_set = c; + c = (CPUSet) {}; - return (int) ncpus; + return 0; +} + +int parse_cpu_set_extend( + const char *rvalue, + CPUSet *old, + bool warn, + const char *unit, + const char *filename, + unsigned line, + const char *lvalue) { + + _cleanup_(cpu_set_reset) CPUSet cpuset = {}; + int r; + + r = parse_cpu_set_full(rvalue, &cpuset, true, unit, filename, line, lvalue); + if (r < 0) + return r; + + if (!cpuset.set) { + /* An empty assignment resets the CPU list */ + cpu_set_reset(old); + return 0; + } + + if (!old->set) { + *old = cpuset; + cpuset = (CPUSet) {}; + return 0; + } + + return cpu_set_add_all(old, &cpuset); } diff --git a/src/shared/cpu-set-util.h b/src/shared/cpu-set-util.h index 9574b81582..4c1c81fc59 100644 --- a/src/shared/cpu-set-util.h +++ b/src/shared/cpu-set-util.h @@ -8,23 +8,40 @@ 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); -char* cpu_set_to_string(const cpu_set_t *set, size_t setsize); -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); +/* This wraps the libc interface with a variable to keep the allocated size. */ +typedef struct CPUSet { + cpu_set_t *set; + size_t allocated; /* in bytes */ +} CPUSet; -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 void cpu_set_reset(CPUSet *a) { + assert((a->allocated > 0) == !!a->set); + if (a->set) + CPU_FREE(a->set); + *a = (CPUSet) {}; } -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); +int cpu_set_add_all(CPUSet *a, const CPUSet *b); + +char* cpu_set_to_string(const CPUSet *a); +int parse_cpu_set_full( + const char *rvalue, + CPUSet *cpu_set, + bool warn, + const char *unit, + const char *filename, unsigned line, + const char *lvalue); +int parse_cpu_set_extend( + const char *rvalue, + CPUSet *old, + bool warn, + const char *unit, + const char *filename, + unsigned line, + const char *lvalue); + +static inline int parse_cpu_set(const char *rvalue, CPUSet *cpu_set){ + return parse_cpu_set_full(rvalue, cpu_set, false, NULL, NULL, 0, NULL); } diff --git a/src/test/test-cpu-set-util.c b/src/test/test-cpu-set-util.c index ff5edb2a69..8c17931f84 100644 --- a/src/test/test-cpu-set-util.c +++ b/src/test/test-cpu-set-util.c @@ -5,150 +5,165 @@ #include "macro.h" static void test_parse_cpu_set(void) { - cpu_set_t *c = NULL; + CPUSet c = {}; _cleanup_free_ char *str = NULL; - int ncpus; int cpu; /* Simple range (from CPUAffinity example) */ - ncpus = parse_cpu_set_and_warn("1 2", &c, NULL, "fake", 1, "CPUAffinity"); - assert_se(ncpus >= 1024); - 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); + assert_se(parse_cpu_set_full("1 2", &c, true, NULL, "fake", 1, "CPUAffinity") >= 0); + assert_se(c.set); + assert_se(c.allocated >= sizeof(__cpu_mask) / 8); + assert_se(CPU_ISSET_S(1, c.allocated, c.set)); + assert_se(CPU_ISSET_S(2, c.allocated, c.set)); + assert_se(CPU_COUNT_S(c.allocated, c.set) == 2); - assert_se(str = cpu_set_to_string(c, CPU_ALLOC_SIZE(ncpus))); + assert_se(str = cpu_set_to_string(&c)); log_info("cpu_set_to_string: %s", str); str = mfree(str); - c = cpu_set_mfree(c); + cpu_set_reset(&c); /* A more interesting range */ - ncpus = parse_cpu_set_and_warn("0 1 2 3 8 9 10 11", &c, NULL, "fake", 1, "CPUAffinity"); - assert_se(ncpus >= 1024); - assert_se(CPU_COUNT_S(CPU_ALLOC_SIZE(ncpus), c) == 8); + assert_se(parse_cpu_set_full("0 1 2 3 8 9 10 11", &c, true, NULL, "fake", 1, "CPUAffinity") >= 0); + assert_se(c.allocated >= sizeof(__cpu_mask) / 8); + assert_se(CPU_COUNT_S(c.allocated, c.set) == 8); for (cpu = 0; cpu < 4; cpu++) - assert_se(CPU_ISSET_S(cpu, CPU_ALLOC_SIZE(ncpus), c)); + assert_se(CPU_ISSET_S(cpu, c.allocated, c.set)); for (cpu = 8; cpu < 12; cpu++) - assert_se(CPU_ISSET_S(cpu, CPU_ALLOC_SIZE(ncpus), c)); - assert_se(str = cpu_set_to_string(c, CPU_ALLOC_SIZE(ncpus))); + assert_se(CPU_ISSET_S(cpu, c.allocated, c.set)); + assert_se(str = cpu_set_to_string(&c)); log_info("cpu_set_to_string: %s", str); str = mfree(str); - c = cpu_set_mfree(c); + cpu_set_reset(&c); /* Quoted strings */ - ncpus = parse_cpu_set_and_warn("8 '9' 10 \"11\"", &c, NULL, "fake", 1, "CPUAffinity"); - assert_se(ncpus >= 1024); - assert_se(CPU_COUNT_S(CPU_ALLOC_SIZE(ncpus), c) == 4); + assert_se(parse_cpu_set_full("8 '9' 10 \"11\"", &c, true, NULL, "fake", 1, "CPUAffinity") >= 0); + assert_se(c.allocated >= sizeof(__cpu_mask) / 8); + assert_se(CPU_COUNT_S(c.allocated, c.set) == 4); for (cpu = 8; cpu < 12; cpu++) - assert_se(CPU_ISSET_S(cpu, CPU_ALLOC_SIZE(ncpus), c)); - assert_se(str = cpu_set_to_string(c, CPU_ALLOC_SIZE(ncpus))); + assert_se(CPU_ISSET_S(cpu, c.allocated, c.set)); + assert_se(str = cpu_set_to_string(&c)); log_info("cpu_set_to_string: %s", str); str = mfree(str); - c = cpu_set_mfree(c); + cpu_set_reset(&c); /* Use commas as separators */ - ncpus = parse_cpu_set_and_warn("0,1,2,3 8,9,10,11", &c, NULL, "fake", 1, "CPUAffinity"); - assert_se(ncpus >= 1024); - assert_se(CPU_COUNT_S(CPU_ALLOC_SIZE(ncpus), c) == 8); + assert_se(parse_cpu_set_full("0,1,2,3 8,9,10,11", &c, true, NULL, "fake", 1, "CPUAffinity") >= 0); + assert_se(c.allocated >= sizeof(__cpu_mask) / 8); + assert_se(CPU_COUNT_S(c.allocated, c.set) == 8); for (cpu = 0; cpu < 4; cpu++) - assert_se(CPU_ISSET_S(cpu, CPU_ALLOC_SIZE(ncpus), c)); + assert_se(CPU_ISSET_S(cpu, c.allocated, c.set)); for (cpu = 8; cpu < 12; cpu++) - assert_se(CPU_ISSET_S(cpu, CPU_ALLOC_SIZE(ncpus), c)); - assert_se(str = cpu_set_to_string(c, CPU_ALLOC_SIZE(ncpus))); + assert_se(CPU_ISSET_S(cpu, c.allocated, c.set)); + assert_se(str = cpu_set_to_string(&c)); log_info("cpu_set_to_string: %s", str); str = mfree(str); - c = cpu_set_mfree(c); + cpu_set_reset(&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"); - assert_se(ncpus >= 1024); - assert_se(CPU_COUNT_S(CPU_ALLOC_SIZE(ncpus), c) == 8); + assert_se(parse_cpu_set_full("0, 1, 2, 3, 4, 5, 6, 7, ", &c, true, NULL, "fake", 1, "CPUAffinity") >= 0); + assert_se(c.allocated >= sizeof(__cpu_mask) / 8); + assert_se(CPU_COUNT_S(c.allocated, c.set) == 8); for (cpu = 0; cpu < 8; cpu++) - assert_se(CPU_ISSET_S(cpu, CPU_ALLOC_SIZE(ncpus), c)); - assert_se(str = cpu_set_to_string(c, CPU_ALLOC_SIZE(ncpus))); + assert_se(CPU_ISSET_S(cpu, c.allocated, c.set)); + assert_se(str = cpu_set_to_string(&c)); log_info("cpu_set_to_string: %s", str); str = mfree(str); - c = cpu_set_mfree(c); + cpu_set_reset(&c); /* Ranges */ - ncpus = parse_cpu_set_and_warn("0-3,8-11", &c, NULL, "fake", 1, "CPUAffinity"); - assert_se(ncpus >= 1024); - assert_se(CPU_COUNT_S(CPU_ALLOC_SIZE(ncpus), c) == 8); + assert_se(parse_cpu_set_full("0-3,8-11", &c, true, NULL, "fake", 1, "CPUAffinity") >= 0); + assert_se(c.allocated >= sizeof(__cpu_mask) / 8); + assert_se(CPU_COUNT_S(c.allocated, c.set) == 8); for (cpu = 0; cpu < 4; cpu++) - assert_se(CPU_ISSET_S(cpu, CPU_ALLOC_SIZE(ncpus), c)); + assert_se(CPU_ISSET_S(cpu, c.allocated, c.set)); for (cpu = 8; cpu < 12; cpu++) - assert_se(CPU_ISSET_S(cpu, CPU_ALLOC_SIZE(ncpus), c)); - assert_se(str = cpu_set_to_string(c, CPU_ALLOC_SIZE(ncpus))); + assert_se(CPU_ISSET_S(cpu, c.allocated, c.set)); + assert_se(str = cpu_set_to_string(&c)); log_info("cpu_set_to_string: %s", str); str = mfree(str); - c = cpu_set_mfree(c); + cpu_set_reset(&c); /* Ranges with trailing comma, space */ - ncpus = parse_cpu_set_and_warn("0-3 8-11, ", &c, NULL, "fake", 1, "CPUAffinity"); - assert_se(ncpus >= 1024); - assert_se(CPU_COUNT_S(CPU_ALLOC_SIZE(ncpus), c) == 8); + assert_se(parse_cpu_set_full("0-3 8-11, ", &c, true, NULL, "fake", 1, "CPUAffinity") >= 0); + assert_se(c.allocated >= sizeof(__cpu_mask) / 8); + assert_se(CPU_COUNT_S(c.allocated, c.set) == 8); for (cpu = 0; cpu < 4; cpu++) - assert_se(CPU_ISSET_S(cpu, CPU_ALLOC_SIZE(ncpus), c)); + assert_se(CPU_ISSET_S(cpu, c.allocated, c.set)); for (cpu = 8; cpu < 12; cpu++) - assert_se(CPU_ISSET_S(cpu, CPU_ALLOC_SIZE(ncpus), c)); - assert_se(str = cpu_set_to_string(c, CPU_ALLOC_SIZE(ncpus))); + assert_se(CPU_ISSET_S(cpu, c.allocated, c.set)); + assert_se(str = cpu_set_to_string(&c)); log_info("cpu_set_to_string: %s", str); str = mfree(str); - c = cpu_set_mfree(c); + cpu_set_reset(&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 = cpu_set_mfree(c); + assert_se(parse_cpu_set_full("3-0", &c, true, NULL, "fake", 1, "CPUAffinity") >= 0); + assert_se(c.allocated >= sizeof(__cpu_mask) / 8); + assert_se(CPU_COUNT_S(c.allocated, c.set) == 0); + cpu_set_reset(&c); /* Overlapping ranges */ - ncpus = parse_cpu_set_and_warn("0-7 4-11", &c, NULL, "fake", 1, "CPUAffinity"); - assert_se(ncpus >= 1024); - assert_se(CPU_COUNT_S(CPU_ALLOC_SIZE(ncpus), c) == 12); + assert_se(parse_cpu_set_full("0-7 4-11", &c, true, NULL, "fake", 1, "CPUAffinity") >= 0); + assert_se(c.allocated >= sizeof(__cpu_mask) / 8); + assert_se(CPU_COUNT_S(c.allocated, c.set) == 12); for (cpu = 0; cpu < 12; cpu++) - assert_se(CPU_ISSET_S(cpu, CPU_ALLOC_SIZE(ncpus), c)); - assert_se(str = cpu_set_to_string(c, CPU_ALLOC_SIZE(ncpus))); + assert_se(CPU_ISSET_S(cpu, c.allocated, c.set)); + assert_se(str = cpu_set_to_string(&c)); log_info("cpu_set_to_string: %s", str); str = mfree(str); - c = cpu_set_mfree(c); + cpu_set_reset(&c); /* Mix ranges and individual CPUs */ - ncpus = parse_cpu_set_and_warn("0,1 4-11", &c, NULL, "fake", 1, "CPUAffinity"); - assert_se(ncpus >= 1024); - assert_se(CPU_COUNT_S(CPU_ALLOC_SIZE(ncpus), c) == 10); - assert_se(CPU_ISSET_S(0, CPU_ALLOC_SIZE(ncpus), c)); - assert_se(CPU_ISSET_S(1, CPU_ALLOC_SIZE(ncpus), c)); + assert_se(parse_cpu_set_full("0,1 4-11", &c, true, NULL, "fake", 1, "CPUAffinity") >= 0); + assert_se(c.allocated >= sizeof(__cpu_mask) / 8); + assert_se(CPU_COUNT_S(c.allocated, c.set) == 10); + assert_se(CPU_ISSET_S(0, c.allocated, c.set)); + assert_se(CPU_ISSET_S(1, c.allocated, c.set)); for (cpu = 4; cpu < 12; cpu++) - assert_se(CPU_ISSET_S(cpu, CPU_ALLOC_SIZE(ncpus), c)); - assert_se(str = cpu_set_to_string(c, CPU_ALLOC_SIZE(ncpus))); + assert_se(CPU_ISSET_S(cpu, c.allocated, c.set)); + assert_se(str = cpu_set_to_string(&c)); log_info("cpu_set_to_string: %s", str); str = mfree(str); - c = cpu_set_mfree(c); + cpu_set_reset(&c); /* Garbage */ - ncpus = parse_cpu_set_and_warn("0 1 2 3 garbage", &c, NULL, "fake", 1, "CPUAffinity"); - assert_se(ncpus < 0); - assert_se(!c); + assert_se(parse_cpu_set_full("0 1 2 3 garbage", &c, true, NULL, "fake", 1, "CPUAffinity") == -EINVAL); + assert_se(!c.set); + assert_se(c.allocated == 0); /* Range with garbage */ - ncpus = parse_cpu_set_and_warn("0-3 8-garbage", &c, NULL, "fake", 1, "CPUAffinity"); - assert_se(ncpus < 0); - assert_se(!c); + assert_se(parse_cpu_set_full("0-3 8-garbage", &c, true, NULL, "fake", 1, "CPUAffinity") == -EINVAL); + assert_se(!c.set); + assert_se(c.allocated == 0); /* Empty string */ - c = NULL; - ncpus = parse_cpu_set_and_warn("", &c, NULL, "fake", 1, "CPUAffinity"); - assert_se(ncpus == 0); /* empty string returns 0 */ - assert_se(!c); + assert_se(parse_cpu_set_full("", &c, true, NULL, "fake", 1, "CPUAffinity") == 0); + assert_se(!c.set); /* empty string returns NULL */ + assert_se(c.allocated == 0); /* Runaway quoted string */ - ncpus = parse_cpu_set_and_warn("0 1 2 3 \"4 5 6 7 ", &c, NULL, "fake", 1, "CPUAffinity"); - assert_se(ncpus < 0); - assert_se(!c); + assert_se(parse_cpu_set_full("0 1 2 3 \"4 5 6 7 ", &c, true, NULL, "fake", 1, "CPUAffinity") == -EINVAL); + assert_se(!c.set); + assert_se(c.allocated == 0); + + /* Maximum allocation */ + assert_se(parse_cpu_set_full("8000-8191", &c, true, NULL, "fake", 1, "CPUAffinity") == 0); + assert_se(CPU_COUNT_S(c.allocated, c.set) == 192); + assert_se(str = cpu_set_to_string(&c)); + log_info("cpu_set_to_string: %s", str); + str = mfree(str); + cpu_set_reset(&c); } int main(int argc, char *argv[]) { + log_info("CPU_ALLOC_SIZE(1) = %zu", CPU_ALLOC_SIZE(1)); + log_info("CPU_ALLOC_SIZE(9) = %zu", CPU_ALLOC_SIZE(9)); + log_info("CPU_ALLOC_SIZE(64) = %zu", CPU_ALLOC_SIZE(64)); + log_info("CPU_ALLOC_SIZE(65) = %zu", CPU_ALLOC_SIZE(65)); + log_info("CPU_ALLOC_SIZE(1024) = %zu", CPU_ALLOC_SIZE(1024)); + log_info("CPU_ALLOC_SIZE(1025) = %zu", CPU_ALLOC_SIZE(1025)); + log_info("CPU_ALLOC_SIZE(8191) = %zu", CPU_ALLOC_SIZE(8191)); + test_parse_cpu_set(); return 0; diff --git a/src/test/test-sizeof.c b/src/test/test-sizeof.c index 35b087653e..7fc16a62b6 100644 --- a/src/test/test-sizeof.c +++ b/src/test/test-sizeof.c @@ -1,5 +1,6 @@ /* SPDX-License-Identifier: LGPL-2.1+ */ +#include #include #include @@ -65,6 +66,8 @@ int main(void) { info(uid_t); info(gid_t); + info(__cpu_mask); + info(enum Enum); info(enum BigEnum); info(enum BigEnum2); From f44b3035d4a698aa0ce08a552199b54d43de3d85 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 28 May 2019 21:28:31 +0200 Subject: [PATCH 05/17] Move cpus_in_affinity_mask() to cpu-set-util.[ch] It just seems to fit better there and it's always better to have things in shared/ rather than basic/. --- src/basic/process-util.c | 39 ++------------------------------------- src/basic/process-util.h | 2 -- src/shared/condition.c | 1 + src/shared/cpu-set-util.c | 34 ++++++++++++++++++++++++++++++++++ src/shared/cpu-set-util.h | 2 ++ src/test/test-condition.c | 1 + 6 files changed, 40 insertions(+), 39 deletions(-) diff --git a/src/basic/process-util.c b/src/basic/process-util.c index 3dc3534e1a..85ea3cf1d1 100644 --- a/src/basic/process-util.c +++ b/src/basic/process-util.c @@ -4,7 +4,6 @@ #include #include #include -#include #include #include #include @@ -1534,45 +1533,11 @@ int set_oom_score_adjust(int value) { WRITE_STRING_FILE_VERIFY_ON_FAILURE|WRITE_STRING_FILE_DISABLE_BUFFER); } -int cpus_in_affinity_mask(void) { - size_t n = 16; - int r; - - for (;;) { - cpu_set_t *c; - - c = CPU_ALLOC(n); - if (!c) - return -ENOMEM; - - if (sched_getaffinity(0, CPU_ALLOC_SIZE(n), c) >= 0) { - int k; - - k = CPU_COUNT_S(CPU_ALLOC_SIZE(n), c); - CPU_FREE(c); - - if (k <= 0) - return -EINVAL; - - return k; - } - - r = -errno; - CPU_FREE(c); - - if (r != -EINVAL) - return r; - if (n > SIZE_MAX/2) - return -ENOMEM; - n *= 2; - } -} - static const char *const ioprio_class_table[] = { [IOPRIO_CLASS_NONE] = "none", [IOPRIO_CLASS_RT] = "realtime", [IOPRIO_CLASS_BE] = "best-effort", - [IOPRIO_CLASS_IDLE] = "idle" + [IOPRIO_CLASS_IDLE] = "idle", }; DEFINE_STRING_TABLE_LOOKUP_WITH_FALLBACK(ioprio_class, int, IOPRIO_N_CLASSES); @@ -1593,7 +1558,7 @@ static const char* const sched_policy_table[] = { [SCHED_BATCH] = "batch", [SCHED_IDLE] = "idle", [SCHED_FIFO] = "fifo", - [SCHED_RR] = "rr" + [SCHED_RR] = "rr", }; DEFINE_STRING_TABLE_LOOKUP_WITH_FALLBACK(sched_policy, int, INT_MAX); diff --git a/src/basic/process-util.h b/src/basic/process-util.h index 7e1d65a7bf..1571f1a652 100644 --- a/src/basic/process-util.h +++ b/src/basic/process-util.h @@ -194,5 +194,3 @@ assert_cc(TASKS_MAX <= (unsigned long) PID_T_MAX) (pid) = 0; \ _pid_; \ }) - -int cpus_in_affinity_mask(void); diff --git a/src/shared/condition.c b/src/shared/condition.c index d1ac9de756..2d521bc8c6 100644 --- a/src/shared/condition.c +++ b/src/shared/condition.c @@ -21,6 +21,7 @@ #include "cap-list.h" #include "cgroup-util.h" #include "condition.h" +#include "cpu-set-util.h" #include "efivars.h" #include "env-file.h" #include "extract-word.h" diff --git a/src/shared/cpu-set-util.c b/src/shared/cpu-set-util.c index b8d13a2ba6..b0c7acd789 100644 --- a/src/shared/cpu-set-util.c +++ b/src/shared/cpu-set-util.c @@ -201,3 +201,37 @@ int parse_cpu_set_extend( return cpu_set_add_all(old, &cpuset); } + +int cpus_in_affinity_mask(void) { + size_t n = 16; + int r; + + for (;;) { + cpu_set_t *c; + + c = CPU_ALLOC(n); + if (!c) + return -ENOMEM; + + if (sched_getaffinity(0, CPU_ALLOC_SIZE(n), c) >= 0) { + int k; + + k = CPU_COUNT_S(CPU_ALLOC_SIZE(n), c); + CPU_FREE(c); + + if (k <= 0) + return -EINVAL; + + return k; + } + + r = -errno; + CPU_FREE(c); + + if (r != -EINVAL) + return r; + if (n > SIZE_MAX/2) + return -ENOMEM; + n *= 2; + } +} diff --git a/src/shared/cpu-set-util.h b/src/shared/cpu-set-util.h index 4c1c81fc59..602c983ae3 100644 --- a/src/shared/cpu-set-util.h +++ b/src/shared/cpu-set-util.h @@ -45,3 +45,5 @@ int parse_cpu_set_extend( static inline int parse_cpu_set(const char *rvalue, CPUSet *cpu_set){ return parse_cpu_set_full(rvalue, cpu_set, false, NULL, NULL, 0, NULL); } + +int cpus_in_affinity_mask(void); diff --git a/src/test/test-condition.c b/src/test/test-condition.c index 2a5e3aba80..4bbca2074f 100644 --- a/src/test/test-condition.c +++ b/src/test/test-condition.c @@ -13,6 +13,7 @@ #include "audit-util.h" #include "cgroup-util.h" #include "condition.h" +#include "cpu-set-util.h" #include "efivars.h" #include "hostname-util.h" #include "id128-util.h" From 9d1345f0657c707df89b41b2738776efb40aec8e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 21 May 2019 09:01:34 +0200 Subject: [PATCH 06/17] test-cpu-set-util: add simple test for cpus_in_affinity_mask() --- src/test/test-cpu-set-util.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/test/test-cpu-set-util.c b/src/test/test-cpu-set-util.c index 8c17931f84..2d1cd5710e 100644 --- a/src/test/test-cpu-set-util.c +++ b/src/test/test-cpu-set-util.c @@ -155,6 +155,14 @@ static void test_parse_cpu_set(void) { cpu_set_reset(&c); } +static void test_cpus_in_affinity_mask(void) { + int r; + + r = cpus_in_affinity_mask(); + assert(r > 0); + log_info("cpus_in_affinity_mask: %d", r); +} + int main(int argc, char *argv[]) { log_info("CPU_ALLOC_SIZE(1) = %zu", CPU_ALLOC_SIZE(1)); log_info("CPU_ALLOC_SIZE(9) = %zu", CPU_ALLOC_SIZE(9)); @@ -165,6 +173,7 @@ int main(int argc, char *argv[]) { log_info("CPU_ALLOC_SIZE(8191) = %zu", CPU_ALLOC_SIZE(8191)); test_parse_cpu_set(); + test_cpus_in_affinity_mask(); return 0; } From b54d7241f25b859c6c008e516c2131c39902e6e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 24 May 2019 08:50:41 +0200 Subject: [PATCH 07/17] test-cpu-set-util: add a smoke test for test_parse_cpu_set_extend() --- src/test/test-cpu-set-util.c | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/src/test/test-cpu-set-util.c b/src/test/test-cpu-set-util.c index 2d1cd5710e..2ef298c68d 100644 --- a/src/test/test-cpu-set-util.c +++ b/src/test/test-cpu-set-util.c @@ -9,6 +9,8 @@ static void test_parse_cpu_set(void) { _cleanup_free_ char *str = NULL; int cpu; + log_info("/* %s */", __func__); + /* Simple range (from CPUAffinity example) */ assert_se(parse_cpu_set_full("1 2", &c, true, NULL, "fake", 1, "CPUAffinity") >= 0); assert_se(c.set); @@ -155,6 +157,28 @@ static void test_parse_cpu_set(void) { cpu_set_reset(&c); } +static void test_parse_cpu_set_extend(void) { + CPUSet c = {}; + _cleanup_free_ char *s1 = NULL, *s2 = NULL; + + log_info("/* %s */", __func__); + + assert_se(parse_cpu_set_extend("1 3", &c, true, NULL, "fake", 1, "CPUAffinity") == 0); + assert_se(CPU_COUNT_S(c.allocated, c.set) == 2); + assert_se(s1 = cpu_set_to_string(&c)); + log_info("cpu_set_to_string: %s", s1); + + assert_se(parse_cpu_set_extend("4", &c, true, NULL, "fake", 1, "CPUAffinity") == 0); + assert_se(CPU_COUNT_S(c.allocated, c.set) == 3); + assert_se(s2 = cpu_set_to_string(&c)); + log_info("cpu_set_to_string: %s", s2); + + assert_se(parse_cpu_set_extend("", &c, true, NULL, "fake", 1, "CPUAffinity") == 0); + assert_se(!c.set); + assert_se(c.allocated == 0); + log_info("cpu_set_to_string: (null)"); +} + static void test_cpus_in_affinity_mask(void) { int r; @@ -173,6 +197,7 @@ int main(int argc, char *argv[]) { log_info("CPU_ALLOC_SIZE(8191) = %zu", CPU_ALLOC_SIZE(8191)); test_parse_cpu_set(); + test_parse_cpu_set_extend(); test_cpus_in_affinity_mask(); return 0; From 61fbbac1d517a0b3498a689c736c6ca918497904 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 24 May 2019 08:35:51 +0200 Subject: [PATCH 08/17] pid1: parse CPUAffinity= in incremental fashion This makes the handling of this option match what we do in unit files. I think consistency is important here. (As it happens, it is the only option in system.conf that is "non-atomic", i.e. where there's a list of things which can be split over multiple assignments. All other options are single-valued, so there's no issue of how to handle multiple assignments.) --- man/systemd-system.conf.xml | 11 +++++++---- man/systemd.exec.xml | 2 +- src/core/main.c | 37 +++++++++++++++++++++++++++---------- 3 files changed, 35 insertions(+), 15 deletions(-) diff --git a/man/systemd-system.conf.xml b/man/systemd-system.conf.xml index 41baff8bfe..f5d419c519 100644 --- a/man/systemd-system.conf.xml +++ b/man/systemd-system.conf.xml @@ -96,10 +96,13 @@ CPUAffinity= - Configures the CPU affinity for the service manager as well as the default CPU affinity for all - forked off processes. Takes a list of CPU indices or ranges separated by either whitespace or commas. CPU - ranges are specified by the lower and upper CPU indices separated by a dash. Individual services may override - the CPU affinity for their processes with the CPUAffinity= setting in unit files, see + Configures the CPU affinity for the service manager as well as the default CPU + affinity for all forked off processes. Takes a list of CPU indices or ranges separated by either + whitespace or commas. CPU ranges are specified by the lower and upper CPU indices separated by a + dash. This option may be specified more than once, in which case the specified CPU affinity masks are + merged. If the empty string is assigned, the mask is reset, all assignments prior to this will have + no effect. Individual services may override the CPU affinity for their processes with the + CPUAffinity= setting in unit files, see systemd.exec5. diff --git a/man/systemd.exec.xml b/man/systemd.exec.xml index 719c2c5ff3..d65b842f44 100644 --- a/man/systemd.exec.xml +++ b/man/systemd.exec.xml @@ -754,7 +754,7 @@ CapabilityBoundingSet=~CAP_B CAP_C Controls the CPU affinity of the executed processes. Takes a list of CPU indices or ranges separated by either whitespace or commas. CPU ranges are specified by the lower and upper CPU indices separated - by a dash. This option may be specified more than once, in which case the specified CPU affinity masks are + by a dash. This option may be specified more than once, in which case the specified CPU affinity masks are merged. If the empty string is assigned, the mask is reset, all assignments prior to this will have no effect. See sched_setaffinity2 for diff --git a/src/core/main.c b/src/core/main.c index 67a62a5ad2..1f83566757 100644 --- a/src/core/main.c +++ b/src/core/main.c @@ -139,6 +139,8 @@ static sd_id128_t arg_machine_id = {}; static EmergencyAction arg_cad_burst_action = EMERGENCY_ACTION_REBOOT_FORCE; static OOMPolicy arg_default_oom_policy = OOM_STOP; +static CPUSet arg_cpu_affinity = {}; + _noreturn_ static void freeze_or_exit_or_reboot(void) { /* If we are running in a container, let's prefer exiting, after all we can propagate an exit code to @@ -567,17 +569,11 @@ static int config_parse_cpu_affinity2( void *data, void *userdata) { - _cleanup_(cpu_set_reset) CPUSet c = {}; - int r; + CPUSet *affinity = data; - r = parse_cpu_set_full(rvalue, &c, true, unit, filename, line, lvalue); - if (r < 0) - return r; + assert(affinity); - if (sched_setaffinity(0, c.allocated, c.set) < 0) - log_warning_errno(errno, "Failed to set CPU affinity: %m"); - - // FIXME: parsing and execution should be seperated. + (void) parse_cpu_set_extend(rvalue, affinity, true, unit, filename, line, lvalue); return 0; } @@ -719,7 +715,7 @@ static int parse_config_file(void) { { "Manager", "CrashShell", config_parse_bool, 0, &arg_crash_shell }, { "Manager", "CrashReboot", config_parse_bool, 0, &arg_crash_reboot }, { "Manager", "ShowStatus", config_parse_show_status, 0, &arg_show_status }, - { "Manager", "CPUAffinity", config_parse_cpu_affinity2, 0, NULL }, + { "Manager", "CPUAffinity", config_parse_cpu_affinity2, 0, &arg_cpu_affinity }, { "Manager", "JoinControllers", config_parse_warn_compat, DISABLED_CONFIGURATION, NULL }, { "Manager", "RuntimeWatchdogSec", config_parse_sec, 0, &arg_runtime_watchdog }, { "Manager", "ShutdownWatchdogSec", config_parse_sec, 0, &arg_shutdown_watchdog }, @@ -1733,6 +1729,21 @@ static void initialize_core_pattern(bool skip_setup) { log_warning_errno(r, "Failed to write '%s' to /proc/sys/kernel/core_pattern, ignoring: %m", arg_early_core_pattern); } +static void update_cpu_affinity(bool skip_setup) { + _cleanup_free_ char *mask = NULL; + + if (skip_setup || !arg_cpu_affinity.set) + return; + + assert(arg_cpu_affinity.allocated > 0); + + mask = cpu_set_to_string(&arg_cpu_affinity); + log_debug("Setting CPU affinity to %s.", strnull(mask)); + + if (sched_setaffinity(0, arg_cpu_affinity.allocated, arg_cpu_affinity.set) < 0) + log_warning_errno(errno, "Failed to set CPU affinity: %m"); +} + static void do_reexecute( int argc, char *argv[], @@ -1905,6 +1916,8 @@ static int invoke_main_loop( set_manager_defaults(m); + update_cpu_affinity(false); + if (saved_log_level >= 0) manager_override_log_level(m, saved_log_level); if (saved_log_target >= 0) @@ -2063,6 +2076,8 @@ static int initialize_runtime( if (arg_action != ACTION_RUN) return 0; + update_cpu_affinity(skip_setup); + if (arg_system) { /* Make sure we leave a core dump without panicking the kernel. */ install_crash_handler(); @@ -2195,6 +2210,8 @@ static void free_arguments(void) { arg_confirm_spawn = mfree(arg_confirm_spawn); arg_default_environment = strv_free(arg_default_environment); arg_syscall_archs = set_free(arg_syscall_archs); + + cpu_set_reset(&arg_cpu_affinity); } static int load_configuration(int argc, char **argv, const char **ret_error_message) { From 470a5e6dcec4637439ae953002127af214d396ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 24 May 2019 08:59:23 +0200 Subject: [PATCH 09/17] pid1: don't reset setting from /proc/cmdline upon restart We have settings which may be set on the kernel command line, and also in /proc/cmdline (for pid1). The settings in /proc/cmdline have higher priority of course. When a reload was done, we'd reload just the configuration file, losing the overrides. So read /proc/cmdline again during reload. Also, when initially reading the configuration file when program starts, don't treat any errors as fatal. The configuration done in there doesn't seem important enough to refuse boot. --- src/core/main.c | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/src/core/main.c b/src/core/main.c index 1f83566757..44b0a65f22 100644 --- a/src/core/main.c +++ b/src/core/main.c @@ -141,6 +141,8 @@ static OOMPolicy arg_default_oom_policy = OOM_STOP; static CPUSet arg_cpu_affinity = {}; +static int parse_configuration(void); + _noreturn_ static void freeze_or_exit_or_reboot(void) { /* If we are running in a container, let's prefer exiting, after all we can propagate an exit code to @@ -1910,9 +1912,7 @@ static int invoke_main_loop( saved_log_level = m->log_level_overridden ? log_get_max_level() : -1; saved_log_target = m->log_target_overridden ? log_get_target() : _LOG_TARGET_INVALID; - r = parse_config_file(); - if (r < 0) - log_warning_errno(r, "Failed to parse config file, ignoring: %m"); + (void) parse_configuration(); set_manager_defaults(m); @@ -2214,18 +2214,14 @@ static void free_arguments(void) { cpu_set_reset(&arg_cpu_affinity); } -static int load_configuration(int argc, char **argv, const char **ret_error_message) { +static int parse_configuration(void) { int r; - assert(ret_error_message); - arg_default_tasks_max = system_tasks_max_scale(DEFAULT_TASKS_MAX_PERCENTAGE, 100U); r = parse_config_file(); - if (r < 0) { - *ret_error_message = "Failed to parse config file"; - return r; - } + if (r < 0) + log_warning_errno(r, "Failed to parse config file, ignoring: %m"); if (arg_system) { r = proc_cmdline_parse(parse_proc_cmdline_item, NULL, 0); @@ -2236,6 +2232,16 @@ static int load_configuration(int argc, char **argv, const char **ret_error_mess /* Note that this also parses bits from the kernel command line, including "debug". */ log_parse_environment(); + return 0; +} + +static int load_configuration(int argc, char **argv, const char **ret_error_message) { + int r; + + assert(ret_error_message); + + (void) parse_configuration(); + r = parse_argv(argc, argv); if (r < 0) { *ret_error_message = "Failed to parse commandline arguments"; From fb39af4ce42d7ef9af63009f271f404038703704 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 24 May 2019 09:41:44 +0200 Subject: [PATCH 10/17] pid1: when reloading configuration, forget old settings If we had a configuration setting from a configuration file, and it was removed, we'd still remember the old value, because there's was no mechanism to "reset" everything, just to assign new values. Note that the effect of this is limited. For settings that have an "ongoing" effect, like systemd.confirm_spawn, the new value is simply used. But some settings can only be set at start. In particular, CPUAffinity= will be updated if set to a new value, but if CPUAffinity= is fully removed, it will not be reset, simply because we don't know what to reset it to. We might have inherited a setting, or we might have set it ourselves. In principle we could remember the "original" value that was set when we were executed, but propagate this over reloads and reexecs, but that would be a lot of work for little gain. So this corner case of removal of CPUAffinity= is not handled fully, and a reboot is needed to execute the change. As a work-around, a full mask of CPUAffinity=0-8191 can be specified. --- src/core/main.c | 144 ++++++++++++++++++++++++++++++++---------------- 1 file changed, 96 insertions(+), 48 deletions(-) diff --git a/src/core/main.c b/src/core/main.c index 44b0a65f22..872494d32e 100644 --- a/src/core/main.c +++ b/src/core/main.c @@ -96,50 +96,52 @@ static enum { ACTION_DUMP_CONFIGURATION_ITEMS, ACTION_DUMP_BUS_PROPERTIES, } arg_action = ACTION_RUN; -static char *arg_default_unit = NULL; -static bool arg_system = false; -static bool arg_dump_core = true; -static int arg_crash_chvt = -1; -static bool arg_crash_shell = false; -static bool arg_crash_reboot = false; -static char *arg_confirm_spawn = NULL; -static ShowStatus arg_show_status = _SHOW_STATUS_INVALID; -static bool arg_switched_root = false; -static PagerFlags arg_pager_flags = 0; -static bool arg_service_watchdogs = true; -static ExecOutput arg_default_std_output = EXEC_OUTPUT_JOURNAL; -static ExecOutput arg_default_std_error = EXEC_OUTPUT_INHERIT; -static usec_t arg_default_restart_usec = DEFAULT_RESTART_USEC; -static usec_t arg_default_timeout_start_usec = DEFAULT_TIMEOUT_USEC; -static usec_t arg_default_timeout_stop_usec = DEFAULT_TIMEOUT_USEC; -static usec_t arg_default_timeout_abort_usec = DEFAULT_TIMEOUT_USEC; -static bool arg_default_timeout_abort_set = false; -static usec_t arg_default_start_limit_interval = DEFAULT_START_LIMIT_INTERVAL; -static unsigned arg_default_start_limit_burst = DEFAULT_START_LIMIT_BURST; -static usec_t arg_runtime_watchdog = 0; -static usec_t arg_shutdown_watchdog = 10 * USEC_PER_MINUTE; -static char *arg_early_core_pattern = NULL; -static char *arg_watchdog_device = NULL; -static char **arg_default_environment = NULL; -static struct rlimit *arg_default_rlimit[_RLIMIT_MAX] = {}; -static uint64_t arg_capability_bounding_set = CAP_ALL; -static bool arg_no_new_privs = false; -static nsec_t arg_timer_slack_nsec = NSEC_INFINITY; -static usec_t arg_default_timer_accuracy_usec = 1 * USEC_PER_MINUTE; -static Set* arg_syscall_archs = NULL; -static FILE* arg_serialization = NULL; -static int arg_default_cpu_accounting = -1; -static bool arg_default_io_accounting = false; -static bool arg_default_ip_accounting = false; -static bool arg_default_blockio_accounting = false; -static bool arg_default_memory_accounting = MEMORY_ACCOUNTING_DEFAULT; -static bool arg_default_tasks_accounting = true; -static uint64_t arg_default_tasks_max = UINT64_MAX; -static sd_id128_t arg_machine_id = {}; -static EmergencyAction arg_cad_burst_action = EMERGENCY_ACTION_REBOOT_FORCE; -static OOMPolicy arg_default_oom_policy = OOM_STOP; -static CPUSet arg_cpu_affinity = {}; +/* Those variables are initalized to 0 automatically, so we avoid uninitialized memory access. + * Real defaults are assigned in reset_arguments() below. */ +static char *arg_default_unit; +static bool arg_system; +static bool arg_dump_core; +static int arg_crash_chvt; +static bool arg_crash_shell; +static bool arg_crash_reboot; +static char *arg_confirm_spawn; +static ShowStatus arg_show_status; +static bool arg_switched_root; +static PagerFlags arg_pager_flags; +static bool arg_service_watchdogs; +static ExecOutput arg_default_std_output; +static ExecOutput arg_default_std_error; +static usec_t arg_default_restart_usec; +static usec_t arg_default_timeout_start_usec; +static usec_t arg_default_timeout_stop_usec; +static usec_t arg_default_timeout_abort_usec; +static bool arg_default_timeout_abort_set; +static usec_t arg_default_start_limit_interval; +static unsigned arg_default_start_limit_burst; +static usec_t arg_runtime_watchdog; +static usec_t arg_shutdown_watchdog; +static char *arg_early_core_pattern; +static char *arg_watchdog_device; +static char **arg_default_environment; +static struct rlimit *arg_default_rlimit[_RLIMIT_MAX]; +static uint64_t arg_capability_bounding_set; +static bool arg_no_new_privs; +static nsec_t arg_timer_slack_nsec; +static usec_t arg_default_timer_accuracy_usec; +static Set* arg_syscall_archs; +static FILE* arg_serialization; +static int arg_default_cpu_accounting; +static bool arg_default_io_accounting; +static bool arg_default_ip_accounting; +static bool arg_default_blockio_accounting; +static bool arg_default_memory_accounting; +static bool arg_default_tasks_accounting; +static uint64_t arg_default_tasks_max; +static sd_id128_t arg_machine_id; +static EmergencyAction arg_cad_burst_action; +static OOMPolicy arg_default_oom_policy; +static CPUSet arg_cpu_affinity; static int parse_configuration(void); @@ -2201,16 +2203,59 @@ static int do_queue_default_job( return 0; } -static void free_arguments(void) { - - /* Frees all arg_* variables, with the exception of arg_serialization */ - rlimit_free_all(arg_default_rlimit); +static void reset_arguments(void) { + /* Frees/resets arg_* variables, with a few exceptions commented below. */ arg_default_unit = mfree(arg_default_unit); + + /* arg_system — ignore */ + + arg_dump_core = true; + arg_crash_chvt = -1; + arg_crash_shell = false; + arg_crash_reboot = false; arg_confirm_spawn = mfree(arg_confirm_spawn); + arg_show_status = _SHOW_STATUS_INVALID; + arg_switched_root = false; + arg_pager_flags = 0; + arg_service_watchdogs = true; + arg_default_std_output = EXEC_OUTPUT_JOURNAL; + arg_default_std_error = EXEC_OUTPUT_INHERIT; + arg_default_restart_usec = DEFAULT_RESTART_USEC; + arg_default_timeout_start_usec = DEFAULT_TIMEOUT_USEC; + arg_default_timeout_stop_usec = DEFAULT_TIMEOUT_USEC; + arg_default_timeout_abort_usec = DEFAULT_TIMEOUT_USEC; + arg_default_timeout_abort_set = false; + arg_default_start_limit_interval = DEFAULT_START_LIMIT_INTERVAL; + arg_default_start_limit_burst = DEFAULT_START_LIMIT_BURST; + arg_runtime_watchdog = 0; + arg_shutdown_watchdog = 10 * USEC_PER_MINUTE; + arg_early_core_pattern = NULL; + arg_watchdog_device = NULL; + arg_default_environment = strv_free(arg_default_environment); + rlimit_free_all(arg_default_rlimit); + + arg_capability_bounding_set = CAP_ALL; + arg_no_new_privs = false; + arg_timer_slack_nsec = NSEC_INFINITY; + arg_default_timer_accuracy_usec = 1 * USEC_PER_MINUTE; + arg_syscall_archs = set_free(arg_syscall_archs); + /* arg_serialization — ignore */ + + arg_default_cpu_accounting = -1; + arg_default_io_accounting = false; + arg_default_ip_accounting = false; + arg_default_blockio_accounting = false; + arg_default_memory_accounting = MEMORY_ACCOUNTING_DEFAULT; + arg_default_tasks_accounting = true; + arg_default_tasks_max = UINT64_MAX; + arg_machine_id = (sd_id128_t) {}; + arg_cad_burst_action = EMERGENCY_ACTION_REBOOT_FORCE; + arg_default_oom_policy = OOM_STOP; + cpu_set_reset(&arg_cpu_affinity); } @@ -2219,6 +2264,9 @@ static int parse_configuration(void) { arg_default_tasks_max = system_tasks_max_scale(DEFAULT_TASKS_MAX_PERCENTAGE, 100U); + /* Assign configuration defaults */ + reset_arguments(); + r = parse_config_file(); if (r < 0) log_warning_errno(r, "Failed to parse config file, ignoring: %m"); @@ -2702,7 +2750,7 @@ finish: m = manager_free(m); } - free_arguments(); + reset_arguments(); mac_selinux_finish(); if (reexecute) From 167a776dbe9d033523bd6881e5a695f2155dc321 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 28 May 2019 21:38:41 +0200 Subject: [PATCH 11/17] test-execute: use CPUSet too cpu_set_malloc() was the last user. It doesn't seem useful to keep it just to save the allocation of a few hundred bytes in a test, so it is dropped and a fixed maximum is allocated (1024 bytes). --- src/shared/cpu-set-util.c | 31 +------------------------------ src/shared/cpu-set-util.h | 3 +-- src/test/test-execute.c | 13 ++++++------- 3 files changed, 8 insertions(+), 39 deletions(-) diff --git a/src/shared/cpu-set-util.c b/src/shared/cpu-set-util.c index b0c7acd789..e9218a542b 100644 --- a/src/shared/cpu-set-util.c +++ b/src/shared/cpu-set-util.c @@ -34,36 +34,7 @@ char* cpu_set_to_string(const CPUSet *a) { return TAKE_PTR(str) ?: strdup(""); } -cpu_set_t* cpu_set_malloc(unsigned *ncpus) { - cpu_set_t *c; - unsigned n = 1024; - - /* Allocates the cpuset in the right size */ - - for (;;) { - c = CPU_ALLOC(n); - if (!c) - return NULL; - - if (sched_getaffinity(0, CPU_ALLOC_SIZE(n), c) >= 0) { - CPU_ZERO_S(CPU_ALLOC_SIZE(n), c); - - if (ncpus) - *ncpus = n; - - return c; - } - - CPU_FREE(c); - - if (errno != EINVAL) - return NULL; - - n *= 2; - } -} - -static int cpu_set_realloc(CPUSet *cpu_set, unsigned ncpus) { +int cpu_set_realloc(CPUSet *cpu_set, unsigned ncpus) { size_t need; assert(cpu_set); diff --git a/src/shared/cpu-set-util.h b/src/shared/cpu-set-util.h index 602c983ae3..fc1aa6b63a 100644 --- a/src/shared/cpu-set-util.h +++ b/src/shared/cpu-set-util.h @@ -8,8 +8,6 @@ DEFINE_TRIVIAL_CLEANUP_FUNC(cpu_set_t*, CPU_FREE); #define _cleanup_cpu_free_ _cleanup_(CPU_FREEp) -cpu_set_t* cpu_set_malloc(unsigned *ncpus); - /* This wraps the libc interface with a variable to keep the allocated size. */ typedef struct CPUSet { cpu_set_t *set; @@ -26,6 +24,7 @@ static inline void cpu_set_reset(CPUSet *a) { int cpu_set_add_all(CPUSet *a, const CPUSet *b); char* cpu_set_to_string(const CPUSet *a); +int cpu_set_realloc(CPUSet *cpu_set, unsigned ncpus); int parse_cpu_set_full( const char *rvalue, CPUSet *cpu_set, diff --git a/src/test/test-execute.c b/src/test/test-execute.c index 9f1cb0ca38..4a2f546827 100644 --- a/src/test/test-execute.c +++ b/src/test/test-execute.c @@ -179,13 +179,12 @@ static void test_exec_bindpaths(Manager *m) { } static void test_exec_cpuaffinity(Manager *m) { - _cleanup_cpu_free_ cpu_set_t *c = NULL; - unsigned n; + _cleanup_(cpu_set_reset) CPUSet c = {}; - assert_se(c = cpu_set_malloc(&n)); - assert_se(sched_getaffinity(0, CPU_ALLOC_SIZE(n), c) >= 0); + assert_se(cpu_set_realloc(&c, 8192) >= 0); /* just allocate the maximum possible size */ + assert_se(sched_getaffinity(0, c.allocated, c.set) >= 0); - if (CPU_ISSET_S(0, CPU_ALLOC_SIZE(n), c) == 0) { + if (!CPU_ISSET_S(0, c.allocated, c.set)) { log_notice("Cannot use CPU 0, skipping %s", __func__); return; } @@ -193,8 +192,8 @@ static void test_exec_cpuaffinity(Manager *m) { test(__func__, m, "exec-cpuaffinity1.service", 0, CLD_EXITED); test(__func__, 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) { + if (!CPU_ISSET_S(1, c.allocated, c.set) || + !CPU_ISSET_S(2, c.allocated, c.set)) { log_notice("Cannot use CPU 1 or 2, skipping remaining tests in %s", __func__); return; } From cb0d3acf55ef335001cac5dd9c335ec5e75e9b56 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 28 May 2019 21:40:10 +0200 Subject: [PATCH 12/17] shared/cpu-set-util: drop now-unused cleanup function --- src/shared/cpu-set-util.h | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/shared/cpu-set-util.h b/src/shared/cpu-set-util.h index fc1aa6b63a..d1b6814bba 100644 --- a/src/shared/cpu-set-util.h +++ b/src/shared/cpu-set-util.h @@ -5,9 +5,6 @@ #include "macro.h" -DEFINE_TRIVIAL_CLEANUP_FUNC(cpu_set_t*, CPU_FREE); -#define _cleanup_cpu_free_ _cleanup_(CPU_FREEp) - /* This wraps the libc interface with a variable to keep the allocated size. */ typedef struct CPUSet { cpu_set_t *set; From c367f996f5f091a63f812f0140b304c649be77fc Mon Sep 17 00:00:00 2001 From: Michal Sekletar Date: Wed, 22 May 2019 13:55:49 +0200 Subject: [PATCH 13/17] shared/cpu-set-util: make transfer of cpu_set_t over bus endian safe --- src/core/dbus-execute.c | 6 +++++- src/shared/bus-unit-util.c | 8 +++++++- src/shared/cpu-set-util.c | 38 ++++++++++++++++++++++++++++++++++++++ src/shared/cpu-set-util.h | 3 +++ 4 files changed, 53 insertions(+), 2 deletions(-) diff --git a/src/core/dbus-execute.c b/src/core/dbus-execute.c index 326e451b9a..d7d339b9d7 100644 --- a/src/core/dbus-execute.c +++ b/src/core/dbus-execute.c @@ -1559,18 +1559,22 @@ int bus_exec_context_set_transient_property( if (streq(name, "CPUAffinity")) { const void *a; size_t n; + _cleanup_(cpu_set_reset) CPUSet set = {}; r = sd_bus_message_read_array(message, 'y', &a, &n); if (r < 0) return r; + r = cpu_set_from_dbus(a, n, &set); + if (r < 0) + return r; + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { if (n == 0) { cpu_set_reset(&c->cpu_set); unit_write_settingf(u, flags, name, "%s=", name); } else { _cleanup_free_ char *str = NULL; - const CPUSet set = {(cpu_set_t*) a, n}; str = cpu_set_to_string(&set); if (!str) diff --git a/src/shared/bus-unit-util.c b/src/shared/bus-unit-util.c index e4889dc55d..fb86391975 100644 --- a/src/shared/bus-unit-util.c +++ b/src/shared/bus-unit-util.c @@ -993,12 +993,18 @@ static int bus_append_execute_property(sd_bus_message *m, const char *field, con if (streq(field, "CPUAffinity")) { _cleanup_(cpu_set_reset) CPUSet cpuset = {}; + _cleanup_free_ uint8_t *array = NULL; + size_t allocated; r = parse_cpu_set(eq, &cpuset); if (r < 0) return log_error_errno(r, "Failed to parse %s value: %s", field, eq); - return bus_append_byte_array(m, field, cpuset.set, cpuset.allocated); + r = cpu_set_to_dbus(&cpuset, &array, &allocated); + if (r < 0) + return log_error_errno(r, "Failed to serialize CPUAffinity: %m"); + + return bus_append_byte_array(m, field, array, allocated); } if (STR_IN_SET(field, "RestrictAddressFamilies", "SystemCallFilter")) { diff --git a/src/shared/cpu-set-util.c b/src/shared/cpu-set-util.c index e9218a542b..4980e1964c 100644 --- a/src/shared/cpu-set-util.c +++ b/src/shared/cpu-set-util.c @@ -206,3 +206,41 @@ int cpus_in_affinity_mask(void) { n *= 2; } } + +int cpu_set_to_dbus(const CPUSet *set, uint8_t **ret, size_t *allocated) { + uint8_t *out; + + assert(set); + assert(ret); + + out = new0(uint8_t, set->allocated); + if (!out) + return -ENOMEM; + + for (unsigned cpu = 0; cpu < set->allocated * 8; cpu++) + if (CPU_ISSET_S(cpu, set->allocated, set->set)) + out[cpu / 8] |= 1u << (cpu % 8); + + *ret = out; + *allocated = set->allocated; + return 0; +} + +int cpu_set_from_dbus(const uint8_t *bits, size_t size, CPUSet *set) { + _cleanup_(cpu_set_reset) CPUSet s = {}; + int r; + + assert(bits); + assert(set); + + for (unsigned cpu = size * 8; cpu > 0; cpu--) + if (bits[(cpu - 1) / 8] & (1u << ((cpu - 1) % 8))) { + r = cpu_set_add(&s, cpu - 1); + if (r < 0) + return r; + } + + *set = s; + s = (CPUSet) {}; + return 0; +} diff --git a/src/shared/cpu-set-util.h b/src/shared/cpu-set-util.h index d1b6814bba..f99a818c60 100644 --- a/src/shared/cpu-set-util.h +++ b/src/shared/cpu-set-util.h @@ -42,4 +42,7 @@ static inline int parse_cpu_set(const char *rvalue, CPUSet *cpu_set){ return parse_cpu_set_full(rvalue, cpu_set, false, NULL, NULL, 0, NULL); } +int cpu_set_to_dbus(const CPUSet *set, uint8_t **ret, size_t *allocated); +int cpu_set_from_dbus(const uint8_t *bits, size_t size, CPUSet *set); + int cpus_in_affinity_mask(void); From 1bf0d6c28f8c884e187c7dacc1a969c0763ff4e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 29 May 2019 09:44:16 +0200 Subject: [PATCH 14/17] test-cpu-set-util: add test for dbus conversions --- src/test/test-cpu-set-util.c | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/src/test/test-cpu-set-util.c b/src/test/test-cpu-set-util.c index 2ef298c68d..ea443a1c3b 100644 --- a/src/test/test-cpu-set-util.c +++ b/src/test/test-cpu-set-util.c @@ -179,6 +179,36 @@ static void test_parse_cpu_set_extend(void) { log_info("cpu_set_to_string: (null)"); } +static void test_cpu_set_to_from_dbus(void) { + _cleanup_(cpu_set_reset) CPUSet c = {}, c2 = {}; + _cleanup_free_ char *s = NULL; + + log_info("/* %s */", __func__); + + assert_se(parse_cpu_set_extend("1 3 8 100-200", &c, true, NULL, "fake", 1, "CPUAffinity") == 0); + assert_se(s = cpu_set_to_string(&c)); + log_info("cpu_set_to_string: %s", s); + assert_se(CPU_COUNT_S(c.allocated, c.set) == 104); + + _cleanup_free_ uint8_t *array = NULL; + size_t allocated; + static const char expected[32] = + "\x0A\x01\x00\x00\x00\x00\x00\x00\x00\x00" + "\x00\x00\xF0\xFF\xFF\xFF\xFF\xFF\xFF\xFF" + "\xFF\xFF\xFF\xFF\xFF\x01"; + + assert_se(cpu_set_to_dbus(&c, &array, &allocated) == 0); + assert_se(array); + assert_se(allocated == c.allocated); + + assert(memcmp(array, expected, sizeof expected) == 0); + + assert_se(cpu_set_from_dbus(array, allocated, &c2) == 0); + assert_se(c2.set); + assert_se(c2.allocated == c.allocated); + assert_se(memcmp(c.set, c2.set, c.allocated) == 0); +} + static void test_cpus_in_affinity_mask(void) { int r; @@ -199,6 +229,7 @@ int main(int argc, char *argv[]) { test_parse_cpu_set(); test_parse_cpu_set_extend(); test_cpus_in_affinity_mask(); + test_cpu_set_to_from_dbus(); return 0; } From 71b28519b55b496237146f9bcb5a627455f15f7e Mon Sep 17 00:00:00 2001 From: Michal Sekletar Date: Thu, 23 May 2019 14:27:18 +0200 Subject: [PATCH 15/17] shared/cpu-set-util: introduce cpu_set_to_range() --- src/shared/cpu-set-util.c | 37 ++++++++++++++++++++++++++ src/shared/cpu-set-util.h | 2 ++ src/test/test-cpu-set-util.c | 50 ++++++++++++++++++++++++++++++++++++ 3 files changed, 89 insertions(+) diff --git a/src/shared/cpu-set-util.c b/src/shared/cpu-set-util.c index 4980e1964c..13a43dc6cc 100644 --- a/src/shared/cpu-set-util.c +++ b/src/shared/cpu-set-util.c @@ -34,6 +34,43 @@ char* cpu_set_to_string(const CPUSet *a) { return TAKE_PTR(str) ?: strdup(""); } +char *cpu_set_to_range_string(const CPUSet *set) { + unsigned range_start = 0, range_end; + _cleanup_free_ char *str = NULL; + size_t allocated = 0, len = 0; + bool in_range = false; + int r; + + for (unsigned i = 0; i < set->allocated * 8; i++) + if (CPU_ISSET_S(i, set->allocated, set->set)) { + if (in_range) + range_end++; + else { + range_start = range_end = i; + in_range = true; + } + } else if (in_range) { + in_range = false; + + if (!GREEDY_REALLOC(str, allocated, len + 2 + 2 * DECIMAL_STR_MAX(unsigned))) + return NULL; + + r = sprintf(str + len, len > 0 ? " %d-%d" : "%d-%d", range_start, range_end); + assert_se(r > 0); + len += r; + } + + if (in_range) { + if (!GREEDY_REALLOC(str, allocated, len + 2 + 2 * DECIMAL_STR_MAX(int))) + return NULL; + + r = sprintf(str + len, len > 0 ? " %d-%d" : "%d-%d", range_start, range_end); + assert_se(r > 0); + } + + return TAKE_PTR(str) ?: strdup(""); +} + int cpu_set_realloc(CPUSet *cpu_set, unsigned ncpus) { size_t need; diff --git a/src/shared/cpu-set-util.h b/src/shared/cpu-set-util.h index f99a818c60..fd6a15f446 100644 --- a/src/shared/cpu-set-util.h +++ b/src/shared/cpu-set-util.h @@ -21,7 +21,9 @@ static inline void cpu_set_reset(CPUSet *a) { int cpu_set_add_all(CPUSet *a, const CPUSet *b); char* cpu_set_to_string(const CPUSet *a); +char *cpu_set_to_range_string(const CPUSet *a); int cpu_set_realloc(CPUSet *cpu_set, unsigned ncpus); + int parse_cpu_set_full( const char *rvalue, CPUSet *cpu_set, diff --git a/src/test/test-cpu-set-util.c b/src/test/test-cpu-set-util.c index ea443a1c3b..d66d7d304d 100644 --- a/src/test/test-cpu-set-util.c +++ b/src/test/test-cpu-set-util.c @@ -2,6 +2,7 @@ #include "alloc-util.h" #include "cpu-set-util.h" +#include "string-util.h" #include "macro.h" static void test_parse_cpu_set(void) { @@ -11,6 +12,22 @@ static void test_parse_cpu_set(void) { log_info("/* %s */", __func__); + /* Single value */ + assert_se(parse_cpu_set_full("0", &c, true, NULL, "fake", 1, "CPUAffinity") >= 0); + assert_se(c.set); + assert_se(c.allocated >= sizeof(__cpu_mask) / 8); + assert_se(CPU_ISSET_S(0, c.allocated, c.set)); + assert_se(CPU_COUNT_S(c.allocated, c.set) == 1); + + assert_se(str = cpu_set_to_string(&c)); + log_info("cpu_set_to_string: %s", str); + str = mfree(str); + assert_se(str = cpu_set_to_range_string(&c)); + log_info("cpu_set_to_range_string: %s", str); + assert_se(streq(str, "0-0")); + str = mfree(str); + cpu_set_reset(&c); + /* Simple range (from CPUAffinity example) */ assert_se(parse_cpu_set_full("1 2", &c, true, NULL, "fake", 1, "CPUAffinity") >= 0); assert_se(c.set); @@ -22,6 +39,10 @@ static void test_parse_cpu_set(void) { assert_se(str = cpu_set_to_string(&c)); log_info("cpu_set_to_string: %s", str); str = mfree(str); + assert_se(str = cpu_set_to_range_string(&c)); + log_info("cpu_set_to_range_string: %s", str); + assert_se(streq(str, "1-2")); + str = mfree(str); cpu_set_reset(&c); /* A more interesting range */ @@ -32,9 +53,14 @@ static void test_parse_cpu_set(void) { assert_se(CPU_ISSET_S(cpu, c.allocated, c.set)); for (cpu = 8; cpu < 12; cpu++) assert_se(CPU_ISSET_S(cpu, c.allocated, c.set)); + assert_se(str = cpu_set_to_string(&c)); log_info("cpu_set_to_string: %s", str); str = mfree(str); + assert_se(str = cpu_set_to_range_string(&c)); + log_info("cpu_set_to_range_string: %s", str); + assert_se(streq(str, "0-3 8-11")); + str = mfree(str); cpu_set_reset(&c); /* Quoted strings */ @@ -46,6 +72,10 @@ static void test_parse_cpu_set(void) { assert_se(str = cpu_set_to_string(&c)); log_info("cpu_set_to_string: %s", str); str = mfree(str); + assert_se(str = cpu_set_to_range_string(&c)); + log_info("cpu_set_to_range_string: %s", str); + assert_se(streq(str, "8-11")); + str = mfree(str); cpu_set_reset(&c); /* Use commas as separators */ @@ -70,6 +100,10 @@ static void test_parse_cpu_set(void) { assert_se(str = cpu_set_to_string(&c)); log_info("cpu_set_to_string: %s", str); str = mfree(str); + assert_se(str = cpu_set_to_range_string(&c)); + log_info("cpu_set_to_range_string: %s", str); + assert_se(streq(str, "0-7")); + str = mfree(str); cpu_set_reset(&c); /* Ranges */ @@ -96,6 +130,10 @@ static void test_parse_cpu_set(void) { assert_se(str = cpu_set_to_string(&c)); log_info("cpu_set_to_string: %s", str); str = mfree(str); + assert_se(str = cpu_set_to_range_string(&c)); + log_info("cpu_set_to_range_string: %s", str); + assert_se(streq(str, "0-3 8-11")); + str = mfree(str); cpu_set_reset(&c); /* Negative range (returns empty cpu_set) */ @@ -113,6 +151,10 @@ static void test_parse_cpu_set(void) { assert_se(str = cpu_set_to_string(&c)); log_info("cpu_set_to_string: %s", str); str = mfree(str); + assert_se(str = cpu_set_to_range_string(&c)); + log_info("cpu_set_to_range_string: %s", str); + assert_se(streq(str, "0-11")); + str = mfree(str); cpu_set_reset(&c); /* Mix ranges and individual CPUs */ @@ -126,6 +168,10 @@ static void test_parse_cpu_set(void) { assert_se(str = cpu_set_to_string(&c)); log_info("cpu_set_to_string: %s", str); str = mfree(str); + assert_se(str = cpu_set_to_range_string(&c)); + log_info("cpu_set_to_range_string: %s", str); + assert_se(streq(str, "0-1 4-11")); + str = mfree(str); cpu_set_reset(&c); /* Garbage */ @@ -154,6 +200,10 @@ static void test_parse_cpu_set(void) { assert_se(str = cpu_set_to_string(&c)); log_info("cpu_set_to_string: %s", str); str = mfree(str); + assert_se(str = cpu_set_to_range_string(&c)); + log_info("cpu_set_to_range_string: %s", str); + assert_se(streq(str, "8000-8191")); + str = mfree(str); cpu_set_reset(&c); } From a047f4f10ed2f922d6079c033d24a443b0e95f38 Mon Sep 17 00:00:00 2001 From: Michal Sekletar Date: Wed, 22 May 2019 17:14:21 +0200 Subject: [PATCH 16/17] systemctl: present CPUAffinity mask as a list of CPU index ranges --- src/systemctl/systemctl.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index 059f7220b5..0574a9f4ce 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -31,6 +31,7 @@ #include "cgroup-show.h" #include "cgroup-util.h" #include "copy.h" +#include "cpu-set-util.h" #include "dropin.h" #include "efivars.h" #include "env-util.h" @@ -5403,6 +5404,27 @@ static int print_property(const char *name, const char *expected_value, sd_bus_m if (all || !isempty(fields)) bus_print_property_value(name, expected_value, value, strempty(fields)); + return 1; + } else if (contents[0] == SD_BUS_TYPE_BYTE && streq(name, "CPUAffinity")) { + _cleanup_free_ char *affinity = NULL; + _cleanup_(cpu_set_reset) CPUSet set = {}; + const void *a; + size_t n; + + r = sd_bus_message_read_array(m, 'y', &a, &n); + if (r < 0) + return bus_log_parse_error(r); + + r = cpu_set_from_dbus(a, n, &set); + if (r < 0) + return log_error_errno(r, "Failed to deserialize CPUAffinity: %m"); + + affinity = cpu_set_to_range_string(&set); + if (!affinity) + return log_oom(); + + bus_print_property_value(name, expected_value, value, affinity); + return 1; } From 1f57a176af5152d05719bf43740e87a47e37af50 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 29 May 2019 10:17:43 +0200 Subject: [PATCH 17/17] shared/cpu-set-util: only force range printing one time MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The idea is to have at least one range to make the new format clearly distinguishable from the old. But it is enough to just do it once. In particular, in case the affinity would be specified like 0, 2, 4, 6…, this gives much shorter output. --- src/shared/cpu-set-util.c | 10 ++++++++-- src/test/test-cpu-set-util.c | 7 ++++--- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/shared/cpu-set-util.c b/src/shared/cpu-set-util.c index 13a43dc6cc..7c8f4d42d2 100644 --- a/src/shared/cpu-set-util.c +++ b/src/shared/cpu-set-util.c @@ -55,7 +55,10 @@ char *cpu_set_to_range_string(const CPUSet *set) { if (!GREEDY_REALLOC(str, allocated, len + 2 + 2 * DECIMAL_STR_MAX(unsigned))) return NULL; - r = sprintf(str + len, len > 0 ? " %d-%d" : "%d-%d", range_start, range_end); + if (range_end > range_start || len == 0) + r = sprintf(str + len, len > 0 ? " %d-%d" : "%d-%d", range_start, range_end); + else + r = sprintf(str + len, len > 0 ? " %d" : "%d", range_start); assert_se(r > 0); len += r; } @@ -64,7 +67,10 @@ char *cpu_set_to_range_string(const CPUSet *set) { if (!GREEDY_REALLOC(str, allocated, len + 2 + 2 * DECIMAL_STR_MAX(int))) return NULL; - r = sprintf(str + len, len > 0 ? " %d-%d" : "%d-%d", range_start, range_end); + if (range_end > range_start || len == 0) + r = sprintf(str + len, len > 0 ? " %d-%d" : "%d-%d", range_start, range_end); + else + r = sprintf(str + len, len > 0 ? " %d" : "%d", range_start); assert_se(r > 0); } diff --git a/src/test/test-cpu-set-util.c b/src/test/test-cpu-set-util.c index d66d7d304d..cb970a61c1 100644 --- a/src/test/test-cpu-set-util.c +++ b/src/test/test-cpu-set-util.c @@ -29,19 +29,20 @@ static void test_parse_cpu_set(void) { cpu_set_reset(&c); /* Simple range (from CPUAffinity example) */ - assert_se(parse_cpu_set_full("1 2", &c, true, NULL, "fake", 1, "CPUAffinity") >= 0); + assert_se(parse_cpu_set_full("1 2 4", &c, true, NULL, "fake", 1, "CPUAffinity") >= 0); assert_se(c.set); assert_se(c.allocated >= sizeof(__cpu_mask) / 8); assert_se(CPU_ISSET_S(1, c.allocated, c.set)); assert_se(CPU_ISSET_S(2, c.allocated, c.set)); - assert_se(CPU_COUNT_S(c.allocated, c.set) == 2); + assert_se(CPU_ISSET_S(4, c.allocated, c.set)); + assert_se(CPU_COUNT_S(c.allocated, c.set) == 3); assert_se(str = cpu_set_to_string(&c)); log_info("cpu_set_to_string: %s", str); str = mfree(str); assert_se(str = cpu_set_to_range_string(&c)); log_info("cpu_set_to_range_string: %s", str); - assert_se(streq(str, "1-2")); + assert_se(streq(str, "1-2 4")); str = mfree(str); cpu_set_reset(&c);