From: Anita Zhang Date: Thu, 24 Jun 2021 09:37:57 +0000 (-0700) Subject: oomd: switch system context parsing to use /proc/meminfo X-Git-Tag: v249-rc3~18^2~2 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=47136b9d9a75fff5f9e2e777aaed736e6f66c7f7;p=thirdparty%2Fsystemd.git oomd: switch system context parsing to use /proc/meminfo Makes it easier in the next commits to unify on one way to read swap and memory info. --- diff --git a/src/basic/procfs-util.c b/src/basic/procfs-util.c index ccab71f7d2a..db3e29d04ac 100644 --- a/src/basic/procfs-util.c +++ b/src/basic/procfs-util.c @@ -201,6 +201,41 @@ int procfs_cpu_get_usage(nsec_t *ret) { return 0; } +int convert_meminfo_value_to_uint64_bytes(char *word, uint64_t *ret) { + char *digits, *e; + uint64_t v; + size_t n; + int r; + + assert(word); + assert(ret); + + /* Determine length of numeric value */ + n = strspn(word, WHITESPACE); + digits = word + n; + n = strspn(digits, DIGITS); + if (n == 0) + return -EINVAL; + e = digits + n; + + /* Ensure the line ends in " kB" */ + n = strspn(e, WHITESPACE); + if (n == 0) + return -EINVAL; + if (!streq(e + n, "kB")) + return -EINVAL; + + *e = 0; + r = safe_atou64(digits, &v); + if (r < 0) + return r; + if (v == UINT64_MAX) + return -EINVAL; + + *ret = v * 1024U; + return 0; +} + int procfs_memory_get(uint64_t *ret_total, uint64_t *ret_used) { uint64_t mem_total = UINT64_MAX, mem_free = UINT64_MAX; _cleanup_fclose_ FILE *f = NULL; @@ -213,8 +248,7 @@ int procfs_memory_get(uint64_t *ret_total, uint64_t *ret_used) { for (;;) { _cleanup_free_ char *line = NULL; uint64_t *v; - char *p, *e; - size_t n; + char *p; r = read_line(f, LONG_LINE_MAX, &line); if (r < 0) @@ -233,25 +267,9 @@ int procfs_memory_get(uint64_t *ret_total, uint64_t *ret_used) { continue; } - /* Determine length of numeric value */ - n = strspn(p, DIGITS); - if (n == 0) - return -EINVAL; - e = p + n; - - /* Ensure the line ends in " kB" */ - n = strspn(e, WHITESPACE); - if (n == 0) - return -EINVAL; - if (!streq(e + n, "kB")) - return -EINVAL; - - *e = 0; - r = safe_atou64(p, v); + r = convert_meminfo_value_to_uint64_bytes(p, v); if (r < 0) return r; - if (*v == UINT64_MAX) - return -EINVAL; if (mem_total != UINT64_MAX && mem_free != UINT64_MAX) break; @@ -261,8 +279,8 @@ int procfs_memory_get(uint64_t *ret_total, uint64_t *ret_used) { return -EINVAL; if (ret_total) - *ret_total = mem_total * 1024U; + *ret_total = mem_total; if (ret_used) - *ret_used = (mem_total - mem_free) * 1024U; + *ret_used = mem_total - mem_free; return 0; } diff --git a/src/basic/procfs-util.h b/src/basic/procfs-util.h index 8258c9e3ea7..b7bf7b729db 100644 --- a/src/basic/procfs-util.h +++ b/src/basic/procfs-util.h @@ -15,3 +15,6 @@ int procfs_memory_get(uint64_t *ret_total, uint64_t *ret_used); static inline int procfs_memory_get_used(uint64_t *ret) { return procfs_memory_get(NULL, ret); } + +/* This function destroys "word" (it'll be truncated to perform conversion) */ +int convert_meminfo_value_to_uint64_bytes(char *word, uint64_t *ret); diff --git a/src/oom/oomd-manager.c b/src/oom/oomd-manager.c index 49dc5eb26e8..ddfa2fea0dc 100644 --- a/src/oom/oomd-manager.c +++ b/src/oom/oomd-manager.c @@ -323,14 +323,10 @@ static int monitor_swap_contexts_handler(sd_event_source *s, uint64_t usec, void return log_error_errno(r, "Failed to acquire varlink connection: %m"); } - /* We still try to acquire swap information for oomctl even if no units want swap monitoring */ - r = oomd_system_context_acquire("/proc/swaps", &m->system_context); - /* If there are no units depending on swap actions, the only error we exit on is ENOMEM. - * Allow ENOENT in the event that swap is disabled on the system. */ - if (r == -ENOENT) { - zero(m->system_context); - return 0; - } else if (r == -ENOMEM || (r < 0 && !hashmap_isempty(m->monitored_swap_cgroup_contexts))) + /* We still try to acquire system information for oomctl even if no units want swap monitoring */ + r = oomd_system_context_acquire("/proc/meminfo", &m->system_context); + /* If there are no units depending on swap actions, the only error we exit on is ENOMEM. */ + if (r == -ENOMEM || (r < 0 && !hashmap_isempty(m->monitored_swap_cgroup_contexts))) return log_error_errno(r, "Failed to acquire system context: %m"); /* Return early if nothing is requesting swap monitoring */ diff --git a/src/oom/oomd-util.c b/src/oom/oomd-util.c index 0550ac6c74e..bb77c848072 100644 --- a/src/oom/oomd-util.c +++ b/src/oom/oomd-util.c @@ -4,6 +4,7 @@ #include #include "fd-util.h" +#include "fileio.h" #include "format-util.h" #include "oomd-util.h" #include "parse-util.h" @@ -357,45 +358,56 @@ int oomd_cgroup_context_acquire(const char *path, OomdCGroupContext **ret) { return 0; } -int oomd_system_context_acquire(const char *proc_swaps_path, OomdSystemContext *ret) { +int oomd_system_context_acquire(const char *proc_meminfo_path, OomdSystemContext *ret) { _cleanup_fclose_ FILE *f = NULL; + unsigned field_filled = 0; OomdSystemContext ctx = {}; + uint64_t swap_free; int r; - assert(proc_swaps_path); + assert(proc_meminfo_path); assert(ret); - f = fopen(proc_swaps_path, "re"); + f = fopen(proc_meminfo_path, "re"); if (!f) return -errno; - (void) fscanf(f, "%*s %*s %*s %*s %*s\n"); - for (;;) { - uint64_t total, used; + _cleanup_free_ char *line = NULL; + char *word; - r = fscanf(f, - "%*s " /* device/file */ - "%*s " /* type of swap */ - "%" PRIu64 " " /* swap size */ - "%" PRIu64 " " /* used */ - "%*s\n", /* priority */ - &total, &used); + r = read_line(f, LONG_LINE_MAX, &line); + if (r < 0) + return r; + if (r == 0) + return -EINVAL; + + if ((word = startswith(line, "SwapTotal:"))) { + field_filled |= 1U << 0; + r = convert_meminfo_value_to_uint64_bytes(word, &ctx.swap_total); + } else if ((word = startswith(line, "SwapFree:"))) { + field_filled |= 1U << 1; + r = convert_meminfo_value_to_uint64_bytes(word, &swap_free); + } else + continue; - if (r == EOF && feof(f)) - break; + if (r < 0) + return log_debug_errno(r, "Error converting '%s' from %s to uint64_t: %m", line, proc_meminfo_path); - if (r != 2) { - if (ferror(f)) - return log_debug_errno(errno, "Error reading from %s: %m", proc_swaps_path); + if (field_filled == 3U) + break; + } - return log_debug_errno(SYNTHETIC_ERRNO(EIO), - "Failed to parse values from %s: %m", proc_swaps_path); - } + if (field_filled != 3U) + return log_debug_errno(SYNTHETIC_ERRNO(EINVAL), "%s is missing expected fields", proc_meminfo_path); - ctx.swap_total += total * 1024U; - ctx.swap_used += used * 1024U; - } + if (swap_free > ctx.swap_total) + return log_debug_errno(SYNTHETIC_ERRNO(EINVAL), + "SwapFree (%" PRIu64 ") cannot be greater than SwapTotal (%" PRIu64 ") %m", + swap_free, + ctx.swap_total); + + ctx.swap_used = ctx.swap_total - swap_free; *ret = ctx; return 0; diff --git a/src/oom/test-oomd-util.c b/src/oom/test-oomd-util.c index 7ebea29c1a6..c7d2778200a 100644 --- a/src/oom/test-oomd-util.c +++ b/src/oom/test-oomd-util.c @@ -247,26 +247,33 @@ static void test_oomd_system_context_acquire(void) { assert_se(oomd_system_context_acquire("/verylikelynonexistentpath", &ctx) == -ENOENT); - assert_se(oomd_system_context_acquire(path, &ctx) == 0); - assert_se(ctx.swap_total == 0); - assert_se(ctx.swap_used == 0); + assert_se(oomd_system_context_acquire(path, &ctx) == -EINVAL); assert_se(write_string_file(path, "some\nwords\nacross\nmultiple\nlines", WRITE_STRING_FILE_CREATE) == 0); + assert_se(oomd_system_context_acquire(path, &ctx) == -EINVAL); + + assert_se(write_string_file(path, "MemTotal: 32495256 kB trailing\n" + "MemFree: 9880512 kB data\n" + "SwapTotal: 8388604 kB is\n" + "SwapFree: 7604 kB bad\n", WRITE_STRING_FILE_CREATE) == 0); + assert_se(oomd_system_context_acquire(path, &ctx) == -EINVAL); + + assert_se(oomd_system_context_acquire("/proc/meminfo", &ctx) == 0); + assert_se(ctx.swap_total > 0); + assert_se(ctx.swap_used <= ctx.swap_total); + + assert_se(write_string_file(path, "MemTotal: 32495256 kB\n" + "MemFree: 9880512 kB\n" + "MemAvailable: 21777088 kB\n" + "Buffers: 5968 kB\n" + "Cached: 14344796 kB\n" + "Unevictable: 740004 kB\n" + "Mlocked: 4484 kB\n" + "SwapTotal: 8388604 kB\n" + "SwapFree: 7604 kB\n", WRITE_STRING_FILE_CREATE) == 0); assert_se(oomd_system_context_acquire(path, &ctx) == 0); - assert_se(ctx.swap_total == 0); - assert_se(ctx.swap_used == 0); - - assert_se(write_string_file(path, "Filename Type Size Used Priority", WRITE_STRING_FILE_CREATE) == 0); - assert_se(oomd_system_context_acquire(path, &ctx) == 0); - assert_se(ctx.swap_total == 0); - assert_se(ctx.swap_used == 0); - - assert_se(write_string_file(path, "Filename Type Size Used Priority\n" - "/swapvol/swapfile file 18971644 0 -3\n" - "/dev/vda2 partition 1999868 993780 -2", WRITE_STRING_FILE_CREATE) == 0); - assert_se(oomd_system_context_acquire(path, &ctx) == 0); - assert_se(ctx.swap_total == 21474828288); - assert_se(ctx.swap_used == 1017630720); + assert_se(ctx.swap_total == 8589930496); + assert_se(ctx.swap_used == 8582144000); } static void test_oomd_pressure_above(void) { diff --git a/src/shared/psi-util.c b/src/shared/psi-util.c index 97a43537922..009095e8c3b 100644 --- a/src/shared/psi-util.c +++ b/src/shared/psi-util.c @@ -35,8 +35,8 @@ int read_resource_pressure(const char *path, PressureType type, ResourcePressure return -EINVAL; r = fopen_unlocked(path, "re", &f); - if (r < 0) - return r; + if (r < 0) + return r; for (;;) { _cleanup_free_ char *l = NULL;