From: Yu Watanabe Date: Fri, 20 Jun 2025 21:50:26 +0000 (+0900) Subject: cpu-set-util: introduce config_parse_cpu_set() X-Git-Tag: v258-rc1~238^2~1 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=fe3ada076ea8799a4d75f2e63adf540992d6fbb2;p=thirdparty%2Fsystemd.git cpu-set-util: introduce config_parse_cpu_set() Then, make parse_cpu_set() as a tiny wrapper of it. Note, previously when an invalid CPU range, e.g. "3-0", is specified, we ignore the range but allocate an empty set. But, with this commit, now the conf parser simply ignore it without no side effect. This potentially changes behavior of a system with such invalid setting, but the change should be favorable for consistency with other parsers. --- diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c index ed677aa31fb..fd78e4374fd 100644 --- a/src/core/load-fragment.c +++ b/src/core/load-fragment.c @@ -1572,15 +1572,9 @@ int config_parse_numa_mask( } /* When parsing system.conf or user.conf, rather than unit files, userdata is NULL. */ - if (!userdata) { - r = parse_cpu_set_extend(rvalue, cpu_set, true, unit, filename, line, lvalue); - if (r < 0) - log_syntax(unit, LOG_WARNING, filename, line, r, "Failed to parse NUMA node mask, ignoring: %s", rvalue); - - return 0; - } - - return config_parse_unit_cpu_set(unit, filename, line, section, section_line, lvalue, ltype, rvalue, data, userdata); + return userdata ? + config_parse_unit_cpu_set(unit, filename, line, section, section_line, lvalue, ltype, rvalue, data, userdata) : + config_parse_cpu_set(unit, filename, line, section, section_line, lvalue, ltype, rvalue, data, userdata); } int config_parse_exec_cpu_sched_prio(const char *unit, @@ -3783,8 +3777,7 @@ int config_parse_unit_cpu_set( void *data, void *userdata) { - CPUSet *c = data; - const Unit *u = userdata; + const Unit *u = ASSERT_PTR(userdata); _cleanup_free_ char *k = NULL; int r; @@ -3800,11 +3793,7 @@ int config_parse_unit_cpu_set( return 0; } - r = parse_cpu_set_extend(k, c, true, unit, filename, line, lvalue); - if (r < 0) - return 0; - - return 1; + return config_parse_cpu_set(unit, filename, line, section, section_line, lvalue, ltype, k, data, userdata); } int config_parse_memory_limit( @@ -6274,25 +6263,6 @@ void unit_dump_config_items(FILE *f) { } } -int config_parse_cpu_affinity2( - const char *unit, - const char *filename, - unsigned line, - const char *section, - unsigned section_line, - const char *lvalue, - int ltype, - const char *rvalue, - void *data, - void *userdata) { - - CPUSet *affinity = ASSERT_PTR(data); - - (void) parse_cpu_set_extend(rvalue, affinity, true, unit, filename, line, lvalue); - - return 0; -} - int config_parse_show_status( const char* unit, const char *filename, diff --git a/src/core/load-fragment.h b/src/core/load-fragment.h index 1365174de60..d87bff09968 100644 --- a/src/core/load-fragment.h +++ b/src/core/load-fragment.h @@ -140,7 +140,6 @@ CONFIG_PARSER_PROTOTYPE(config_parse_oom_policy); CONFIG_PARSER_PROTOTYPE(config_parse_numa_policy); CONFIG_PARSER_PROTOTYPE(config_parse_numa_mask); CONFIG_PARSER_PROTOTYPE(config_parse_ip_filter_bpf_progs); -CONFIG_PARSER_PROTOTYPE(config_parse_cpu_affinity2); CONFIG_PARSER_PROTOTYPE(config_parse_show_status); CONFIG_PARSER_PROTOTYPE(config_parse_status_unit_format); CONFIG_PARSER_PROTOTYPE(config_parse_output_restricted); diff --git a/src/core/main.c b/src/core/main.c index 5046d05b103..ee980ed189c 100644 --- a/src/core/main.c +++ b/src/core/main.c @@ -741,7 +741,7 @@ static int parse_config_file(void) { { "Manager", "CrashAction", config_parse_crash_action, 0, &arg_crash_action }, { "Manager", "ShowStatus", config_parse_show_status, 0, &arg_show_status }, { "Manager", "StatusUnitFormat", config_parse_status_unit_format, 0, &arg_status_unit_format }, - { "Manager", "CPUAffinity", config_parse_cpu_affinity2, 0, &arg_cpu_affinity }, + { "Manager", "CPUAffinity", config_parse_cpu_set, 0, &arg_cpu_affinity }, { "Manager", "NUMAPolicy", config_parse_numa_policy, 0, &arg_numa_policy.type }, { "Manager", "NUMAMask", config_parse_numa_mask, 0, &arg_numa_policy.nodes }, { "Manager", "JoinControllers", config_parse_warn_compat, DISABLED_LEGACY, NULL }, diff --git a/src/nspawn/nspawn-gperf.gperf b/src/nspawn/nspawn-gperf.gperf index 883ca97bc89..b735093691c 100644 --- a/src/nspawn/nspawn-gperf.gperf +++ b/src/nspawn/nspawn-gperf.gperf @@ -55,7 +55,7 @@ Exec.LimitRTTIME, config_parse_rlimit, RLIMIT_RTTIME, Exec.Hostname, config_parse_hostname, 0, offsetof(Settings, hostname) Exec.NoNewPrivileges, config_parse_tristate, 0, offsetof(Settings, no_new_privileges) Exec.OOMScoreAdjust, config_parse_oom_score_adjust, 0, 0 -Exec.CPUAffinity, config_parse_cpu_affinity, 0, 0 +Exec.CPUAffinity, config_parse_cpu_set, 0, offsetof(Settings, cpu_set) Exec.ResolvConf, config_parse_resolv_conf, 0, offsetof(Settings, resolv_conf) Exec.LinkJournal, config_parse_link_journal, 0, 0 Exec.Timezone, config_parse_timezone_mode, 0, offsetof(Settings, timezone) diff --git a/src/nspawn/nspawn-settings.c b/src/nspawn/nspawn-settings.c index 98b31d5d40c..035796994c2 100644 --- a/src/nspawn/nspawn-settings.c +++ b/src/nspawn/nspawn-settings.c @@ -817,25 +817,6 @@ int config_parse_oom_score_adjust( return 0; } -int config_parse_cpu_affinity( - const char *unit, - const char *filename, - unsigned line, - const char *section, - unsigned section_line, - const char *lvalue, - int ltype, - const char *rvalue, - void *data, - void *userdata) { - - Settings *settings = ASSERT_PTR(data); - - assert(rvalue); - - 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); static const char *const resolv_conf_mode_table[_RESOLV_CONF_MODE_MAX] = { diff --git a/src/nspawn/nspawn-settings.h b/src/nspawn/nspawn-settings.h index f1c43f50f72..607db87feb3 100644 --- a/src/nspawn/nspawn-settings.h +++ b/src/nspawn/nspawn-settings.h @@ -264,7 +264,6 @@ CONFIG_PARSER_PROTOTYPE(config_parse_pid2); CONFIG_PARSER_PROTOTYPE(config_parse_private_users); CONFIG_PARSER_PROTOTYPE(config_parse_syscall_filter); CONFIG_PARSER_PROTOTYPE(config_parse_oom_score_adjust); -CONFIG_PARSER_PROTOTYPE(config_parse_cpu_affinity); CONFIG_PARSER_PROTOTYPE(config_parse_resolv_conf); CONFIG_PARSER_PROTOTYPE(config_parse_link_journal); CONFIG_PARSER_PROTOTYPE(config_parse_timezone_mode); diff --git a/src/shared/cpu-set-util.c b/src/shared/cpu-set-util.c index 174d284b457..c5277f11af8 100644 --- a/src/shared/cpu-set-util.c +++ b/src/shared/cpu-set-util.c @@ -209,91 +209,118 @@ int cpu_set_add_all(CPUSet *c) { return cpu_set_add_range(c, 0, m - 1); } -int parse_cpu_set_full( - const char *rvalue, - CPUSet *cpu_set, - bool warn, +int config_parse_cpu_set( const char *unit, const char *filename, unsigned line, - const char *lvalue) { + const char *section, + unsigned section_line, + const char *lvalue, + int ltype, /* 0 when used as conf parser, 1 when used as usual parser */ + const char *rvalue, + void *data, + void *userdata) { - _cleanup_(cpu_set_done) CPUSet c = {}; - const char *p = ASSERT_PTR(rvalue); + CPUSet *c = ASSERT_PTR(data); + int r, level = ltype ? LOG_DEBUG : LOG_DEBUG; + bool critical = ltype; - assert(cpu_set); + assert(critical || lvalue); - for (;;) { + if (isempty(rvalue)) { + cpu_set_done(c); + return 1; + } + + _cleanup_(cpu_set_done) CPUSet cpuset = {}; + for (const char *p = rvalue;;) { _cleanup_free_ char *word = NULL; - unsigned cpu_lower, cpu_upper; - int r; r = extract_first_word(&p, &word, WHITESPACE ",", EXTRACT_UNQUOTE); if (r == -ENOMEM) - return warn ? log_oom() : -ENOMEM; - if (r < 0) - return warn ? log_syntax(unit, LOG_ERR, filename, line, r, "Invalid value for %s: %s", lvalue, rvalue) : r; + return log_oom_full(level); + if (r < 0) { + if (critical) + return log_debug_errno(r, "Failed to parse CPU set: %s", rvalue); + + log_syntax(unit, LOG_WARNING, filename, line, r, "Failed to parse %s= setting, ignoring assignment: %s", + lvalue, rvalue); + return 0; + } if (r == 0) break; - 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; + unsigned lower, upper; + r = parse_range(word, &lower, &upper); + if (r < 0) { + if (critical) + return log_debug_errno(r, "Failed to parse CPU range: %s", word); - 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); + log_syntax(unit, LOG_WARNING, filename, line, r, + "Failed to parse CPU range, ignoring assignment: %s", word); + continue; + } - /* Make sure something is allocated, to distinguish this from the empty case */ - r = cpu_set_realloc(&c, 1); - if (r < 0) - return r; + if (lower > upper) { + if (critical) + return log_debug_errno(SYNTHETIC_ERRNO(EINVAL), + "Invalid CPU range (%u > %u): %s", + lower, upper, word); + + log_syntax(unit, LOG_WARNING, filename, line, r, + "Invalid CPU range (%u > %u), ignoring assignment: %s", + lower, upper, word); + continue; } - 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; + r = cpu_set_add_range(&cpuset, lower, upper); + if (r == -ENOMEM) + return log_oom_full(level); + if (r < 0) { + if (critical) + return log_debug_errno(r, "Failed to set CPU(s) '%s': %m", word); + + log_syntax(unit, LOG_WARNING, filename, line, r, + "Failed to set CPU(s), ignoring assignment: %s", word); } } - *cpu_set = TAKE_STRUCT(c); + if (!c->set) { + *c = TAKE_STRUCT(cpuset); + return 1; + } + + r = cpu_set_add_set(c, &cpuset); + if (r == -ENOMEM) + return log_oom_full(level); + assert(r >= 0); - return 0; + return 1; } -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_done) CPUSet cpuset = {}; +int parse_cpu_set(const char *s, CPUSet *ret) { + _cleanup_(cpu_set_done) CPUSet c = {}; int r; - assert(old); + assert(s); + assert(ret); - r = parse_cpu_set_full(rvalue, &cpuset, true, unit, filename, line, lvalue); + r = config_parse_cpu_set( + /* unit = */ NULL, + /* filename = */ NULL, + /* line = */ 0, + /* section = */ NULL, + /* section_line = */ 0, + /* lvalue = */ NULL, + /* ltype = */ 1, + /* rvalue = */ s, + /* data = */ &c, + /* userdata = */ NULL); if (r < 0) return r; - if (!cpuset.set) { - /* An empty assignment clears the CPU list */ - cpu_set_done(old); - return 0; - } - - if (!old->set) { - *old = TAKE_STRUCT(cpuset); - return 1; - } - - return cpu_set_add_set(old, &cpuset); + *ret = TAKE_STRUCT(c); + return 0; } int cpus_in_affinity_mask(void) { diff --git a/src/shared/cpu-set-util.h b/src/shared/cpu-set-util.h index 8696b39de19..84a21809ca0 100644 --- a/src/shared/cpu-set-util.h +++ b/src/shared/cpu-set-util.h @@ -3,6 +3,7 @@ #include +#include "conf-parser-forward.h" #include "forward.h" /* This wraps the libc interface with a variable to keep the allocated size. */ @@ -29,25 +30,8 @@ char* cpu_set_to_string(const CPUSet *c); char* cpu_set_to_range_string(const CPUSet *c); char* cpu_set_to_mask_string(const CPUSet *c); -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); -} +CONFIG_PARSER_PROTOTYPE(config_parse_cpu_set); +int parse_cpu_set(const char *s, CPUSet *ret); int cpu_set_to_dbus(const CPUSet *c, uint8_t **ret, size_t *ret_size); int cpu_set_from_dbus(const uint8_t *bits, size_t size, CPUSet *ret); diff --git a/src/test/test-cpu-set-util.c b/src/test/test-cpu-set-util.c index 9254a3a1eca..e3a960ec742 100644 --- a/src/test/test-cpu-set-util.c +++ b/src/test/test-cpu-set-util.c @@ -132,12 +132,6 @@ TEST(parse_cpu_set) { ASSERT_CPUSET_STRING(c, "0 1 2 3 8 9 10 11", "0-3 8-11", "f0f"); cpu_set_done(&c); - /* Negative range (returns empty cpu_set) */ - ASSERT_OK(parse_cpu_set("3-0", &c)); - ASSERT_CPUSET_COUNT(c, 0); - ASSERT_CPUSET_STRING(c, "", "", "0"); - cpu_set_done(&c); - /* Overlapping ranges */ ASSERT_OK(parse_cpu_set("0-7 4-11", &c)); ASSERT_CPUSET_COUNT(c, 12); @@ -156,6 +150,10 @@ TEST(parse_cpu_set) { ASSERT_CPUSET_STRING(c, "0 2 4 5 6 7 8 9 10 11", "0 2 4-11", "ff5"); cpu_set_done(&c); + /* Negative range */ + ASSERT_ERROR(parse_cpu_set("3-0", &c), EINVAL); + ASSERT_CPUSET_EMPTY(c); + /* Garbage */ ASSERT_ERROR(parse_cpu_set("0 1 2 3 garbage", &c), EINVAL); ASSERT_CPUSET_EMPTY(c); @@ -188,18 +186,31 @@ TEST(parse_cpu_set) { cpu_set_done(&c); } -TEST(parse_cpu_set_extend) { +#define parse(str, c) \ + config_parse_cpu_set( \ + "unit", \ + "filename", \ + /* line = */ 0, \ + "[Section]", \ + /* section_line = */ 0, \ + "CPUAffinity", \ + /* ltype = */ 0, \ + str, \ + c, \ + /* userdata = */ NULL) + +TEST(config_parse_cpu_set) { CPUSet c = {}; - ASSERT_OK_POSITIVE(parse_cpu_set_extend("1 3", &c, true, NULL, "fake", 1, "CPUAffinity")); + ASSERT_OK_POSITIVE(parse("1 3", &c)); ASSERT_CPUSET_COUNT(c, 2); ASSERT_CPUSET_STRING(c, "1 3", "1 3", "a"); - ASSERT_OK_POSITIVE(parse_cpu_set_extend("4", &c, true, NULL, "fake", 1, "CPUAffinity")); + ASSERT_OK_POSITIVE(parse("4", &c)); ASSERT_CPUSET_COUNT(c, 3); ASSERT_CPUSET_STRING(c, "1 3 4", "1 3-4", "1a"); - ASSERT_OK_ZERO(parse_cpu_set_extend("", &c, true, NULL, "fake", 1, "CPUAffinity")); + ASSERT_OK_POSITIVE(parse("", &c)); ASSERT_CPUSET_EMPTY(c); } @@ -254,4 +265,4 @@ TEST(print_cpu_alloc_size) { log_info("CPU_ALLOC_SIZE(8191) = %zu", CPU_ALLOC_SIZE(8191)); } -DEFINE_TEST_MAIN(LOG_INFO); +DEFINE_TEST_MAIN(LOG_DEBUG); diff --git a/src/udev/net/link-config.c b/src/udev/net/link-config.c index 0363c32ea2b..59ee406f494 100644 --- a/src/udev/net/link-config.c +++ b/src/udev/net/link-config.c @@ -1371,15 +1371,8 @@ int config_parse_rps_cpu_mask( CPUSet *mask = ASSERT_PTR(data); int r; - assert(filename); - assert(lvalue); assert(rvalue); - if (isempty(rvalue)) { - cpu_set_done(mask); - return 0; - } - if (streq(rvalue, "disable")) { _cleanup_(cpu_set_done) CPUSet c = {}; @@ -1403,11 +1396,7 @@ int config_parse_rps_cpu_mask( return cpu_set_done_and_replace(*mask, c); } - r = parse_cpu_set_extend(rvalue, mask, /* warn= */ true, unit, filename, line, lvalue); - if (r < 0) - return 0; - - return 0; + return config_parse_cpu_set(unit, filename, line, section, section_line, lvalue, ltype, rvalue, data, userdata); } static const char* const mac_address_policy_table[_MAC_ADDRESS_POLICY_MAX] = {