SymbX/systemd
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Rework cpu affinity parsing

Browse Source 
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.
This commit is contained in:
Zbigniew Jędrzejewski-Szmek
2019-05-29 10:20:42 +02:00
parent b12ef71416
commit 0985c7c4e2
14 changed files with 293 additions and 254 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/core/dbus-execute.c
+10 -25
View File
@@ -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);
}
src/core/execute.c
+6 -6
View File
@@ -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);
}
src/core/execute.h
+2 -2
View File
@@ -14,6 +14,7 @@ typedef struct Manager Manager;
#include <sys/capability.h>
#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;
src/core/load-fragment.c
+1 -30
View File
@@ -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(
src/core/main.c
+8 -6
View File
@@ -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;
}