From 4fc66acb93d6f0002263e2dfaefa46e272ae0c9c Mon Sep 17 00:00:00 2001 From: Filipe Brandenburger Date: Fri, 25 Sep 2015 04:45:22 -0700 Subject: [PATCH 1/4] cpu-set-util: Accept commas as separators in parse_cpu_set_and_warn Tested CPUAffinity settings on both a service unit and in system.conf and confirmed they work as expected. Added a new test to confirm that trailing commas and spaces work and to prevent any regressions in that area. --- src/basic/cpu-set-util.c | 2 +- src/test/test-util.c | 17 +++++++++++++++-- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/src/basic/cpu-set-util.c b/src/basic/cpu-set-util.c index 9122ea5d48..441144d5cc 100644 --- a/src/basic/cpu-set-util.c +++ b/src/basic/cpu-set-util.c @@ -75,7 +75,7 @@ int parse_cpu_set_and_warn( unsigned cpu; int r; - r = extract_first_word(&rvalue, &word, WHITESPACE, EXTRACT_QUOTES); + r = extract_first_word(&rvalue, &word, WHITESPACE ",", EXTRACT_QUOTES); if (r < 0) { log_syntax(unit, LOG_ERR, filename, line, r, "Invalid value for %s: %s", lvalue, whole_rvalue); return r; diff --git a/src/test/test-util.c b/src/test/test-util.c index ffde10c7e1..b0eb77592e 100644 --- a/src/test/test-util.c +++ b/src/test/test-util.c @@ -1012,8 +1012,21 @@ static void test_parse_cpu_set(void) { /* 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 < 0); - assert_se(!c); + assert_se(ncpus >= 1024); + assert_se(CPU_COUNT_S(CPU_ALLOC_SIZE(ncpus), c) == 8); + for (cpu = 0; cpu < 4; cpu++) + 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); + + /* 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); + for (cpu = 0; cpu < 8; cpu++) + assert_se(CPU_ISSET_S(cpu, CPU_ALLOC_SIZE(ncpus), c)); + c = mfree(c); /* Ranges */ ncpus = parse_cpu_set_and_warn("0-3,8-11", &c, NULL, "fake", 1, "CPUAffinity"); From 28cb17ef0281efc3a46e5d0e702b0b0ddeaafaa4 Mon Sep 17 00:00:00 2001 From: Filipe Brandenburger Date: Mon, 12 Oct 2015 23:55:31 -0700 Subject: [PATCH 2/4] parse-util: Introduce new parse_range function This function will be useful for CPUAffinity settings that involve ranges of CPUs. Make it generic and include test coverage to prevent regressions. --- src/basic/parse-util.c | 39 +++++++++ src/basic/parse-util.h | 1 + src/test/test-util.c | 180 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 220 insertions(+) diff --git a/src/basic/parse-util.c b/src/basic/parse-util.c index 2437fee60c..1ee5783680 100644 --- a/src/basic/parse-util.c +++ b/src/basic/parse-util.c @@ -19,6 +19,8 @@ along with systemd; If not, see . ***/ +#include "alloc-util.h" +#include "extract-word.h" #include "parse-util.h" #include "string-util.h" #include "util.h" @@ -207,6 +209,43 @@ int parse_size(const char *t, uint64_t base, uint64_t *size) { return 0; } +int parse_range(const char *t, unsigned *lower, unsigned *upper) { + _cleanup_free_ char *word = NULL; + unsigned l, u; + int r; + + assert(lower); + assert(upper); + + /* Extract the lower bound. */ + r = extract_first_word(&t, &word, "-", EXTRACT_DONT_COALESCE_SEPARATORS); + if (r < 0) + return r; + if (r == 0) + return -EINVAL; + + r = safe_atou(word, &l); + if (r < 0) + return r; + + /* Check for the upper bound and extract it if needed */ + if (!t) + /* Single number with no dashes. */ + u = l; + else if (!*t) + /* Trailing dash is an error. */ + return -EINVAL; + else { + r = safe_atou(t, &u); + if (r < 0) + return r; + } + + *lower = l; + *upper = u; + return 0; +} + char *format_bytes(char *buf, size_t l, uint64_t t) { unsigned i; diff --git a/src/basic/parse-util.h b/src/basic/parse-util.h index 72a619c38f..0e56848e26 100644 --- a/src/basic/parse-util.h +++ b/src/basic/parse-util.h @@ -33,6 +33,7 @@ int parse_pid(const char *s, pid_t* ret_pid); int parse_mode(const char *s, mode_t *ret); int parse_size(const char *t, uint64_t base, uint64_t *size); +int parse_range(const char *t, unsigned *lower, unsigned *upper); #define FORMAT_BYTES_MAX 8 char *format_bytes(char *buf, size_t l, uint64_t t); diff --git a/src/test/test-util.c b/src/test/test-util.c index b0eb77592e..5d01d06523 100644 --- a/src/test/test-util.c +++ b/src/test/test-util.c @@ -979,6 +979,185 @@ static void test_parse_size(void) { assert_se(parse_size("-10B 20K", 1024, &bytes) == -ERANGE); } +static void test_parse_range(void) { + unsigned lower, upper; + + /* Successful cases */ + assert_se(parse_range("111", &lower, &upper) == 0); + assert_se(lower == 111); + assert_se(upper == 111); + + assert_se(parse_range("111-123", &lower, &upper) == 0); + assert_se(lower == 111); + assert_se(upper == 123); + + assert_se(parse_range("123-111", &lower, &upper) == 0); + assert_se(lower == 123); + assert_se(upper == 111); + + assert_se(parse_range("123-123", &lower, &upper) == 0); + assert_se(lower == 123); + assert_se(upper == 123); + + assert_se(parse_range("0", &lower, &upper) == 0); + assert_se(lower == 0); + assert_se(upper == 0); + + assert_se(parse_range("0-15", &lower, &upper) == 0); + assert_se(lower == 0); + assert_se(upper == 15); + + assert_se(parse_range("15-0", &lower, &upper) == 0); + assert_se(lower == 15); + assert_se(upper == 0); + + assert_se(parse_range("128-65535", &lower, &upper) == 0); + assert_se(lower == 128); + assert_se(upper == 65535); + + assert_se(parse_range("1024-4294967295", &lower, &upper) == 0); + assert_se(lower == 1024); + assert_se(upper == 4294967295); + + /* Leading whitespace is acceptable */ + assert_se(parse_range(" 111", &lower, &upper) == 0); + assert_se(lower == 111); + assert_se(upper == 111); + + assert_se(parse_range(" 111-123", &lower, &upper) == 0); + assert_se(lower == 111); + assert_se(upper == 123); + + assert_se(parse_range("111- 123", &lower, &upper) == 0); + assert_se(lower == 111); + assert_se(upper == 123); + + assert_se(parse_range("\t111-\t123", &lower, &upper) == 0); + assert_se(lower == 111); + assert_se(upper == 123); + + assert_se(parse_range(" \t 111- \t 123", &lower, &upper) == 0); + assert_se(lower == 111); + assert_se(upper == 123); + + /* Error cases, make sure they fail as expected */ + lower = upper = 9999; + assert_se(parse_range("111garbage", &lower, &upper) == -EINVAL); + assert_se(lower == 9999); + assert_se(upper == 9999); + + assert_se(parse_range("garbage111", &lower, &upper) == -EINVAL); + assert_se(lower == 9999); + assert_se(upper == 9999); + + assert_se(parse_range("garbage", &lower, &upper) == -EINVAL); + assert_se(lower == 9999); + assert_se(upper == 9999); + + assert_se(parse_range("111-123garbage", &lower, &upper) == -EINVAL); + assert_se(lower == 9999); + assert_se(upper == 9999); + + assert_se(parse_range("111garbage-123", &lower, &upper) == -EINVAL); + assert_se(lower == 9999); + assert_se(upper == 9999); + + /* Empty string */ + lower = upper = 9999; + assert_se(parse_range("", &lower, &upper) == -EINVAL); + assert_se(lower == 9999); + assert_se(upper == 9999); + + /* 111--123 will pass -123 to safe_atou which returns -ERANGE for negative */ + assert_se(parse_range("111--123", &lower, &upper) == -ERANGE); + assert_se(lower == 9999); + assert_se(upper == 9999); + + assert_se(parse_range("-111-123", &lower, &upper) == -EINVAL); + assert_se(lower == 9999); + assert_se(upper == 9999); + + assert_se(parse_range("111-123-", &lower, &upper) == -EINVAL); + assert_se(lower == 9999); + assert_se(upper == 9999); + + assert_se(parse_range("111.4-123", &lower, &upper) == -EINVAL); + assert_se(lower == 9999); + assert_se(upper == 9999); + + assert_se(parse_range("111-123.4", &lower, &upper) == -EINVAL); + assert_se(lower == 9999); + assert_se(upper == 9999); + + assert_se(parse_range("111,4-123", &lower, &upper) == -EINVAL); + assert_se(lower == 9999); + assert_se(upper == 9999); + + assert_se(parse_range("111-123,4", &lower, &upper) == -EINVAL); + assert_se(lower == 9999); + assert_se(upper == 9999); + + /* Error on trailing dash */ + assert_se(parse_range("111-", &lower, &upper) == -EINVAL); + assert_se(lower == 9999); + assert_se(upper == 9999); + + assert_se(parse_range("111-123-", &lower, &upper) == -EINVAL); + assert_se(lower == 9999); + assert_se(upper == 9999); + + assert_se(parse_range("111--", &lower, &upper) == -EINVAL); + assert_se(lower == 9999); + assert_se(upper == 9999); + + assert_se(parse_range("111- ", &lower, &upper) == -EINVAL); + assert_se(lower == 9999); + assert_se(upper == 9999); + + /* Whitespace is not a separator */ + assert_se(parse_range("111 123", &lower, &upper) == -EINVAL); + assert_se(lower == 9999); + assert_se(upper == 9999); + + assert_se(parse_range("111\t123", &lower, &upper) == -EINVAL); + assert_se(lower == 9999); + assert_se(upper == 9999); + + assert_se(parse_range("111 \t 123", &lower, &upper) == -EINVAL); + assert_se(lower == 9999); + assert_se(upper == 9999); + + /* Trailing whitespace is invalid (from safe_atou) */ + assert_se(parse_range("111 ", &lower, &upper) == -EINVAL); + assert_se(lower == 9999); + assert_se(upper == 9999); + + assert_se(parse_range("111-123 ", &lower, &upper) == -EINVAL); + assert_se(lower == 9999); + assert_se(upper == 9999); + + assert_se(parse_range("111 -123", &lower, &upper) == -EINVAL); + assert_se(lower == 9999); + assert_se(upper == 9999); + + assert_se(parse_range("111 -123 ", &lower, &upper) == -EINVAL); + assert_se(lower == 9999); + assert_se(upper == 9999); + + assert_se(parse_range("111\t-123\t", &lower, &upper) == -EINVAL); + assert_se(lower == 9999); + assert_se(upper == 9999); + + assert_se(parse_range("111 \t -123 \t ", &lower, &upper) == -EINVAL); + assert_se(lower == 9999); + assert_se(upper == 9999); + + /* Out of the "unsigned" range, this is 1<<64 */ + assert_se(parse_range("0-18446744073709551616", &lower, &upper) == -ERANGE); + assert_se(lower == 9999); + assert_se(upper == 9999); +} + static void test_parse_cpu_set(void) { cpu_set_t *c = NULL; int ncpus; @@ -2378,6 +2557,7 @@ int main(int argc, char *argv[]) { test_u64log2(); test_protect_errno(); test_parse_size(); + test_parse_range(); test_parse_cpu_set(); test_config_parse_iec_uint64(); test_strextend(); From a26662ce9b25c400ead61854ed7be636f186fdb0 Mon Sep 17 00:00:00 2001 From: Filipe Brandenburger Date: Fri, 25 Sep 2015 05:23:23 -0700 Subject: [PATCH 3/4] cpu-set-util: Support ranges in parse_cpu_set_and_warn Tested CPUAffinity ranges on both a service unit and in system.conf and confirmed they work as expected (by inspecting /proc/PID/status, for the main pid of the service and for pid 1). Also mixed ranges with both spaces, commas, trailing commas and spaces. Added new tests to increase coverage of ranges and prevent regressions. --- src/basic/cpu-set-util.c | 24 +++++++++++--------- src/test/test-util.c | 48 ++++++++++++++++++++++++++++++++++++++-- 2 files changed, 59 insertions(+), 13 deletions(-) diff --git a/src/basic/cpu-set-util.c b/src/basic/cpu-set-util.c index 441144d5cc..4950c66767 100644 --- a/src/basic/cpu-set-util.c +++ b/src/basic/cpu-set-util.c @@ -72,14 +72,12 @@ int parse_cpu_set_and_warn( for (;;) { _cleanup_free_ char *word = NULL; - unsigned cpu; + unsigned cpu, cpu_lower, cpu_upper; int r; r = extract_first_word(&rvalue, &word, WHITESPACE ",", EXTRACT_QUOTES); - if (r < 0) { - log_syntax(unit, LOG_ERR, filename, line, r, "Invalid value for %s: %s", lvalue, whole_rvalue); - return r; - } + if (r < 0) + return log_syntax(unit, LOG_ERR, filename, line, r, "Invalid value for %s: %s", lvalue, whole_rvalue); if (r == 0) break; @@ -89,13 +87,17 @@ int parse_cpu_set_and_warn( return log_oom(); } - r = safe_atou(word, &cpu); - if (r < 0 || cpu >= ncpus) { - log_syntax(unit, LOG_ERR, filename, line, r, "Failed to parse CPU affinity '%s'", rvalue); - return -EINVAL; - } + 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); + 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); - CPU_SET_S(cpu, CPU_ALLOC_SIZE(ncpus), c); + 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. */ diff --git a/src/test/test-util.c b/src/test/test-util.c index 5d01d06523..6a3bf20fe4 100644 --- a/src/test/test-util.c +++ b/src/test/test-util.c @@ -1209,14 +1209,58 @@ static void test_parse_cpu_set(void) { /* Ranges */ ncpus = parse_cpu_set_and_warn("0-3,8-11", &c, NULL, "fake", 1, "CPUAffinity"); - assert_se(ncpus < 0); - assert_se(!c); + assert_se(ncpus >= 1024); + assert_se(CPU_COUNT_S(CPU_ALLOC_SIZE(ncpus), c) == 8); + for (cpu = 0; cpu < 4; cpu++) + 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); + + /* 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); + for (cpu = 0; cpu < 4; cpu++) + 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); + + /* 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); + + /* 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); + for (cpu = 0; cpu < 12; cpu++) + assert_se(CPU_ISSET_S(cpu, CPU_ALLOC_SIZE(ncpus), c)); + c = mfree(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)); + for (cpu = 4; cpu < 12; cpu++) + assert_se(CPU_ISSET_S(cpu, CPU_ALLOC_SIZE(ncpus), c)); + c = mfree(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); + /* 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); + /* Empty string */ c = NULL; ncpus = parse_cpu_set_and_warn("", &c, NULL, "fake", 1, "CPUAffinity"); From 71b1c27a406271b71f64487ae70b58f44a4a37f0 Mon Sep 17 00:00:00 2001 From: Filipe Brandenburger Date: Tue, 13 Oct 2015 00:12:39 -0700 Subject: [PATCH 4/4] man: Update man page documentation for CPUAffinity Document support for commas as a separator and possibility of specifying ranges of CPU indices. Tested by regenerating the manpages locally and reading them on man. --- man/systemd-system.conf.xml | 6 ++++-- man/systemd.exec.xml | 6 ++++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/man/systemd-system.conf.xml b/man/systemd-system.conf.xml index 56db9ff17e..8dad0e0f59 100644 --- a/man/systemd-system.conf.xml +++ b/man/systemd-system.conf.xml @@ -109,8 +109,10 @@ CPUAffinity= Configures the initial CPU affinity for the - init process. Takes a space-separated list of CPU - indices. + init process. 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. diff --git a/man/systemd.exec.xml b/man/systemd.exec.xml index d3f56fee40..5f99fa875e 100644 --- a/man/systemd.exec.xml +++ b/man/systemd.exec.xml @@ -217,8 +217,10 @@ CPUAffinity= Controls the CPU affinity of the executed - processes. Takes a space-separated list of CPU indices. This - option may be specified more than once in which case the + 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. See