From f267c3142a43562d273e4cfdd05df21ed836d48b Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 20 May 2021 21:26:56 +0200 Subject: [PATCH 1/5] fileio: make return parameters of read_virtual_file() optional Prompted by: #19647 --- src/basic/fileio.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/basic/fileio.c b/src/basic/fileio.c index 0df03077d3..853e679aaa 100644 --- a/src/basic/fileio.c +++ b/src/basic/fileio.c @@ -371,8 +371,6 @@ int read_virtual_file(const char *filename, size_t max_size, char **ret_contents int n_retries; bool truncated = false; - assert(ret_contents); - /* Virtual filesystems such as sysfs or procfs use kernfs, and kernfs can work with two sorts of * virtual files. One sort uses "seq_file", and the results of the first read are buffered for the * second read. The other sort uses "raw" reads which always go direct to the device. In the latter @@ -486,7 +484,9 @@ int read_virtual_file(const char *filename, size_t max_size, char **ret_contents return -EBADMSG; buf[n] = 0; - *ret_contents = TAKE_PTR(buf); + + if (ret_contents) + *ret_contents = TAKE_PTR(buf); return !truncated; } From ea42da38258ee182201ecc6c9afb18a9ea46021c Mon Sep 17 00:00:00 2001 From: Dan Streetman Date: Wed, 19 May 2021 10:01:59 -0400 Subject: [PATCH 2/5] macro: add ONCE macro that evaluates to 1 one time Every location that this macro is used, it will be true the first time it's checked, then false each time after that. This can be useful for things such as one-time logging. --- src/fundamental/macro-fundamental.h | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/fundamental/macro-fundamental.h b/src/fundamental/macro-fundamental.h index 6ff8372f3c..967518600d 100644 --- a/src/fundamental/macro-fundamental.h +++ b/src/fundamental/macro-fundamental.h @@ -50,6 +50,17 @@ #define UNIQ_T(x, uniq) CONCATENATE(__unique_prefix_, CONCATENATE(x, uniq)) #define UNIQ __COUNTER__ +/* Note that this works differently from pthread_once(): this macro does + * not synchronize code execution, i.e. code that is run conditionalized + * on this macro will run concurrently to all other code conditionalized + * the same way, there's no ordering or completion enforced. */ +#define ONCE __ONCE(UNIQ_T(_once_, UNIQ)) +#define __ONCE(o) \ + ({ \ + static bool (o) = false; \ + __sync_bool_compare_and_swap(&(o), false, true); \ + }) + #undef MAX #define MAX(a, b) __MAX(UNIQ, (a), UNIQ, (b)) #define __MAX(aq, a, bq, b) \ From 264f0afe0d200fcc64647eed417f0f2b293224a7 Mon Sep 17 00:00:00 2001 From: Dan Streetman Date: Wed, 19 May 2021 10:22:21 -0400 Subject: [PATCH 3/5] log: add log_once() and log_once_errno() macros These macros will log a message at the specified level only the first time they are called. On all later calls, if the specified level is debug, the logs will be suppressed; otherwise the message will be logged at debug. --- src/basic/log.h | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/src/basic/log.h b/src/basic/log.h index 51ba3d8fde..738c181070 100644 --- a/src/basic/log.h +++ b/src/basic/log.h @@ -236,6 +236,29 @@ int log_emergency_level(void); #define log_error_errno(error, ...) log_full_errno(LOG_ERR, error, __VA_ARGS__) #define log_emergency_errno(error, ...) log_full_errno(log_emergency_level(), error, __VA_ARGS__) +/* This logs at the specified level the first time it is called, and then + * logs at debug. If the specified level is debug, this logs only the first + * time it is called. */ +#define log_once(level, ...) \ + ({ \ + if (ONCE) \ + log_full(level, __VA_ARGS__); \ + else if (LOG_PRI(level) != LOG_DEBUG) \ + log_debug(__VA_ARGS__); \ + }) + +#define log_once_errno(level, error, ...) \ + ({ \ + int _err = (error); \ + if (ONCE) \ + _err = log_full_errno(level, _err, __VA_ARGS__); \ + else if (LOG_PRI(level) != LOG_DEBUG) \ + _err = log_debug_errno(_err, __VA_ARGS__); \ + else \ + _err = -ERRNO_VALUE(_err); \ + _err; \ + }) + #if LOG_TRACE # define log_trace(...) log_debug(__VA_ARGS__) #else From 0de2fd18705caa49fc1179e241cd2fbb22bd42e3 Mon Sep 17 00:00:00 2001 From: Dan Streetman Date: Wed, 19 May 2021 14:22:28 -0400 Subject: [PATCH 4/5] psi: update is_pressure_supported to read file The kernel still provides the /proc and cgroup pressure files even if its psi support is disabled, so we need to actually read the files to verify they don't return -EOPNOTSUPP --- src/shared/psi-util.c | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/src/shared/psi-util.c b/src/shared/psi-util.c index 7a184d53f7..97a4353792 100644 --- a/src/shared/psi-util.c +++ b/src/shared/psi-util.c @@ -4,6 +4,7 @@ #include #include "alloc-util.h" +#include "errno-util.h" #include "extract-word.h" #include "fd-util.h" #include "fileio.h" @@ -107,12 +108,18 @@ int read_resource_pressure(const char *path, PressureType type, ResourcePressure int is_pressure_supported(void) { const char *p; - FOREACH_STRING(p, "/proc/pressure/cpu", "/proc/pressure/io", "/proc/pressure/memory") - if (access(p, F_OK) < 0) { - if (errno == ENOENT) - return 0; - return -errno; - } + /* The pressure files, both under /proc and in cgroups, will exist + * even if the kernel has PSI support disabled; we have to read + * the file to make sure it doesn't return -EOPNOTSUPP */ + FOREACH_STRING(p, "/proc/pressure/cpu", "/proc/pressure/io", "/proc/pressure/memory") { + int r; + + r = read_virtual_file(p, 0, NULL, NULL); + if (r == -ENOENT || ERRNO_IS_NOT_SUPPORTED(r)) + return 0; + if (r < 0) + return r; + } return 1; } From 135400276c2b8262d2946bfa1369ae46055d664b Mon Sep 17 00:00:00 2001 From: Dan Streetman Date: Fri, 14 May 2021 08:08:33 -0400 Subject: [PATCH 5/5] oom: log one-time warning if kernel doesn't provide memory.swap.current The kernel can be compiled without support for any memory.swap.* files, or it can be disabled at boot time with the 'swapaccount=0' boot parameter, so if the file doesn't exist log warning indicating the kernel doesn't support the file and the user may need to try using the 'swapaccount=1' boot param. Note that the actual error from the call to fopen() is ENOENT, but that is translated into ENODATA in cg_get_attribute_as_uint64() --- src/oom/oomd-util.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/oom/oomd-util.c b/src/oom/oomd-util.c index 5bf81479c9..0550ac6c74 100644 --- a/src/oom/oomd-util.c +++ b/src/oom/oomd-util.c @@ -333,7 +333,11 @@ int oomd_cgroup_context_acquire(const char *path, OomdCGroupContext **ret) { return log_debug_errno(r, "Error getting memory.low from %s: %m", path); r = cg_get_attribute_as_uint64(SYSTEMD_CGROUP_CONTROLLER, path, "memory.swap.current", &ctx->swap_usage); - if (r < 0) + if (r == -ENODATA) + /* The kernel can be compiled without support for memory.swap.* files, + * or it can be disabled with boot param 'swapaccount=0' */ + log_once(LOG_WARNING, "No kernel support for memory.swap.current from %s (try boot param swapaccount=1), ignoring.", path); + else if (r < 0) return log_debug_errno(r, "Error getting memory.swap.current from %s: %m", path); r = cg_get_keyed_attribute(SYSTEMD_CGROUP_CONTROLLER, path, "memory.stat", STRV_MAKE("pgscan"), &val);