1
0
mirror of https://github.com/systemd/systemd.git synced 2024-10-30 06:25:37 +03:00

procfs-util: fix confusion wrt. quantity limit and maximum value

From packit/rawhide-arm64 logs:
Assertion 'limit >= INT_MAX || get_process_ppid(limit+1, NULL) == -ESRCH' failed at src/test/test-process-util.c:855, function test_get_process_ppid(). Aborting.
――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――

The kernel has a few different limits. In particular kernel.threads-max can be
set to some lower value, and kernel.pid_max can be set to a higher value. This
is nice because it reduces PID reuse, even if the number of threads that is
allowed is limited. But the tests assumed that we cannot have a thread with
PID above MIN(kernel.threads-max, kernel.pid_max-1), which is not valid.

So let's rework the whole thing: let's expose the helpers to read
kernel.threads-max and kernel.pid_max, and print what they return in tests.
procfs_tasks_get_limit() was something that is only used in tests, and wasn't
very well defined, so let's drop it.

Fixes #21193.
This commit is contained in:
Zbigniew Jędrzejewski-Szmek 2021-11-02 18:18:21 +01:00
parent db2aef5a1d
commit c3dead53d5
5 changed files with 85 additions and 68 deletions

View File

@ -111,35 +111,57 @@ uint64_t physical_memory_scale(uint64_t v, uint64_t max) {
}
uint64_t system_tasks_max(void) {
uint64_t a = TASKS_MAX, b = TASKS_MAX;
uint64_t a = TASKS_MAX, b = TASKS_MAX, c = TASKS_MAX;
_cleanup_free_ char *root = NULL;
int r;
/* Determine the maximum number of tasks that may run on this system. We check three sources to determine this
* limit:
/* Determine the maximum number of tasks that may run on this system. We check three sources to
* determine this limit:
*
* a) the maximum tasks value the kernel allows on this architecture
* b) the cgroups pids_max attribute for the system
* c) the kernel's configured maximum PID value
* a) kernel.threads-max sysctl: the maximum number of tasks (threads) the kernel allows.
*
* And then pick the smallest of the three */
* This puts a direct limit on the number of concurrent tasks.
*
* b) kernel.pid_max sysctl: the maximum PID value.
*
* This limits the numeric range PIDs can take, and thus indirectly also limits the number of
* concurrent threads. It's primarily a compatibility concept: some crappy old code used a signed
* 16bit type for PIDs, hence the kernel provides a way to ensure the PIDs never go beyond
* INT16_MAX by default.
*
* Also note the weird definition: PIDs assigned will be kept below this value, which means
* the number of tasks that can be created is one lower, as PID 0 is not a valid process ID.
*
* c) pids.max on the root cgroup: the kernel's configured maximum number of tasks.
*
* and then pick the smallest of the three.
*
* By default pid_max is set to much lower values than threads-max, hence the limit people come into
* contact with first, as it's the lowest boundary they need to bump when they want higher number of
* processes.
*/
r = procfs_tasks_get_limit(&a);
r = procfs_get_threads_max(&a);
if (r < 0)
log_debug_errno(r, "Failed to read maximum number of tasks from /proc, ignoring: %m");
log_debug_errno(r, "Failed to read kernel.threads-max, ignoring: %m");
r = procfs_get_pid_max(&b);
if (r < 0)
log_debug_errno(r, "Failed to read kernel.pid_max, ignoring: %m");
else if (b > 0)
/* Subtract one from pid_max, since PID 0 is not a valid PID */
b--;
r = cg_get_root_path(&root);
if (r < 0)
log_debug_errno(r, "Failed to determine cgroup root path, ignoring: %m");
else {
r = cg_get_attribute_as_uint64("pids", root, "pids.max", &b);
r = cg_get_attribute_as_uint64("pids", root, "pids.max", &c);
if (r < 0)
log_debug_errno(r, "Failed to read pids.max attribute of cgroup root, ignoring: %m");
log_debug_errno(r, "Failed to read pids.max attribute of root cgroup, ignoring: %m");
}
return MIN3(TASKS_MAX,
a <= 0 ? TASKS_MAX : a,
b <= 0 ? TASKS_MAX : b);
return MIN3(a, b, c);
}
uint64_t system_tasks_max_scale(uint64_t v, uint64_t max) {

View File

@ -13,54 +13,34 @@
#include "stdio-util.h"
#include "string-util.h"
int procfs_tasks_get_limit(uint64_t *ret) {
int procfs_get_pid_max(uint64_t *ret) {
_cleanup_free_ char *value = NULL;
uint64_t pid_max, threads_max;
int r;
assert(ret);
/* So there are two sysctl files that control the system limit of processes:
*
* 1. kernel.threads-max: this is probably the sysctl that makes more sense, as it directly puts a limit on
* concurrent tasks.
*
* 2. kernel.pid_max: this limits the numeric range PIDs can take, and thus indirectly also limits the number
* of concurrent threads. AFAICS it's primarily a compatibility concept: some crappy old code used a signed
* 16bit type for PIDs, hence the kernel provides a way to ensure the PIDs never go beyond INT16_MAX by
* default.
*
* By default #2 is set to much lower values than #1, hence the limit people come into contact with first, as
* it's the lowest boundary they need to bump when they want higher number of processes.
*
* Also note the weird definition of #2: PIDs assigned will be kept below this value, which means the number of
* tasks that can be created is one lower, as PID 0 is not a valid process ID. */
r = read_one_line_file("/proc/sys/kernel/pid_max", &value);
if (r < 0)
return r;
r = safe_atou64(value, &pid_max);
if (r < 0)
return r;
return safe_atou64(value, ret);
}
int procfs_get_threads_max(uint64_t *ret) {
_cleanup_free_ char *value = NULL;
int r;
assert(ret);
value = mfree(value);
r = read_one_line_file("/proc/sys/kernel/threads-max", &value);
if (r < 0)
return r;
r = safe_atou64(value, &threads_max);
if (r < 0)
return r;
/* Subtract one from pid_max, since PID 0 is not a valid PID */
*ret = MIN(pid_max-1, threads_max);
return 0;
return safe_atou64(value, ret);
}
int procfs_tasks_set_limit(uint64_t limit) {
char buffer[DECIMAL_STR_MAX(uint64_t)+1];
_cleanup_free_ char *value = NULL;
uint64_t pid_max;
int r;
@ -75,10 +55,7 @@ int procfs_tasks_set_limit(uint64_t limit) {
* set it to the maximum. */
limit = CLAMP(limit, 20U, TASKS_MAX);
r = read_one_line_file("/proc/sys/kernel/pid_max", &value);
if (r < 0)
return r;
r = safe_atou64(value, &pid_max);
r = procfs_get_pid_max(&pid_max);
if (r < 0)
return r;
@ -99,14 +76,10 @@ int procfs_tasks_set_limit(uint64_t limit) {
/* Hmm, we couldn't write this? If so, maybe it was already set properly? In that case let's not
* generate an error */
value = mfree(value);
if (read_one_line_file("/proc/sys/kernel/threads-max", &value) < 0)
if (procfs_get_threads_max(&threads_max) < 0)
return r; /* return original error */
if (safe_atou64(value, &threads_max) < 0)
return r; /* return original error */
if (MIN(pid_max-1, threads_max) != limit)
if (MIN(pid_max - 1, threads_max) != limit)
return r; /* return original error */
/* Yay! Value set already matches what we were trying to set, hence consider this a success. */

View File

@ -5,7 +5,9 @@
#include "time-util.h"
int procfs_tasks_get_limit(uint64_t *ret);
int procfs_get_pid_max(uint64_t *ret);
int procfs_get_threads_max(uint64_t *ret);
int procfs_tasks_set_limit(uint64_t limit);
int procfs_tasks_get_current(uint64_t *ret);

View File

@ -851,8 +851,14 @@ static void test_get_process_ppid(void) {
assert_se(get_process_ppid(1, NULL) == -EADDRNOTAVAIL);
/* the process with the PID above the global limit definitely doesn't exist. Verify that */
assert_se(procfs_tasks_get_limit(&limit) >= 0);
assert_se(limit >= INT_MAX || get_process_ppid(limit+1, NULL) == -ESRCH);
assert_se(procfs_get_pid_max(&limit) >= 0);
log_debug("kernel.pid_max = %"PRIu64, limit);
if (limit < INT_MAX) {
r = get_process_ppid(limit + 1, NULL);
log_debug_errno(r, "get_process_limit(%"PRIu64") → %d/%m", limit + 1, r);
assert(r == -ESRCH);
}
for (pid_t pid = 0;;) {
_cleanup_free_ char *c1 = NULL, *c2 = NULL;

View File

@ -6,11 +6,12 @@
#include "format-util.h"
#include "log.h"
#include "procfs-util.h"
#include "process-util.h"
#include "tests.h"
int main(int argc, char *argv[]) {
nsec_t nsec;
uint64_t v;
uint64_t v, w;
int r;
log_parse_environment();
@ -25,26 +26,39 @@ int main(int argc, char *argv[]) {
assert_se(procfs_tasks_get_current(&v) >= 0);
log_info("Current number of tasks: %" PRIu64, v);
r = procfs_tasks_get_limit(&v);
if (r == -ENOENT || ERRNO_IS_PRIVILEGE(r))
return log_tests_skipped("can't read /proc/sys/kernel/pid_max");
v = TASKS_MAX;
r = procfs_get_pid_max(&v);
assert(r >= 0 || r == -ENOENT || ERRNO_IS_PRIVILEGE(r));
log_info("kernel.pid_max: %"PRIu64, v);
w = TASKS_MAX;
r = procfs_get_threads_max(&w);
assert(r >= 0 || r == -ENOENT || ERRNO_IS_PRIVILEGE(r));
log_info("kernel.threads-max: %"PRIu64, w);
v = MIN(v - (v > 0), w);
assert_se(r >= 0);
log_info("Limit of tasks: %" PRIu64, v);
assert_se(v > 0);
assert_se(procfs_tasks_set_limit(v) >= 0);
r = procfs_tasks_set_limit(v);
if (r == -ENOENT || ERRNO_IS_PRIVILEGE(r))
return log_tests_skipped("can't set task limits");
assert(r >= 0);
if (v > 100) {
uint64_t w;
r = procfs_tasks_set_limit(v-1);
assert_se(IN_SET(r, 0, -EPERM, -EACCES, -EROFS));
log_info("Reducing limit by one to %"PRIu64"", v-1);
assert_se(procfs_tasks_get_limit(&w) >= 0);
assert_se((r == 0 && w == v - 1) || (r < 0 && w == v));
r = procfs_tasks_set_limit(v-1);
log_info_errno(r, "procfs_tasks_set_limit: %m");
assert_se(r >= 0 || ERRNO_IS_PRIVILEGE(r));
assert_se(procfs_get_threads_max(&w) >= 0);
assert_se(r >= 0 ? w == v - 1 : w == v);
assert_se(procfs_tasks_set_limit(v) >= 0);
assert_se(procfs_tasks_get_limit(&w) >= 0);
assert_se(procfs_get_threads_max(&w) >= 0);
assert_se(v == w);
}