diff --git a/man/oomd.conf.xml b/man/oomd.conf.xml index 27914e0699..1092fee1da 100644 --- a/man/oomd.conf.xml +++ b/man/oomd.conf.xml @@ -50,10 +50,10 @@ SwapUsedLimit= - Sets the limit for swap usage on the system before systemd-oomd - will take action. If the fraction of swap used on the system is more than what is defined here, - systemd-oomd will act on eligible descendant control groups with swap usage greater - than 5% of total swap, starting from the ones with the highest swap usage. Which + Sets the limit for memory and swap usage on the system before systemd-oomd + will take action. If the fraction of memory used and the fraction of swap used on the system are both more than + what is defined here, systemd-oomd will act on eligible descendant control groups with swap + usage greater than 5% of total swap, starting from the ones with the highest swap usage. Which control groups are monitored and what action gets taken depends on what the unit has configured for ManagedOOMSwap=. Takes a value specified in percent (when suffixed with "%"), permille ("‰") or permyriad ("‱"), between 0% and 100%, inclusive. Defaults to 90%. diff --git a/src/basic/procfs-util.c b/src/basic/procfs-util.c index ccab71f7d2..db3e29d04a 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 8258c9e3ea..b7bf7b729d 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 49dc5eb26e..ca319f4b0d 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 */ @@ -342,12 +338,16 @@ static int monitor_swap_contexts_handler(sd_event_source *s, uint64_t usec, void * is only used to decide which cgroups to kill (and even then only the resource usages of its descendent * nodes are the ones that matter). */ - if (oomd_swap_free_below(&m->system_context, 10000 - m->swap_used_limit_permyriad)) { + /* Check amount of memory free and swap free so we don't free up swap when memory is still available. */ + if (oomd_mem_free_below(&m->system_context, 10000 - m->swap_used_limit_permyriad) && + oomd_swap_free_below(&m->system_context, 10000 - m->swap_used_limit_permyriad)) { _cleanup_hashmap_free_ Hashmap *candidates = NULL; _cleanup_free_ char *selected = NULL; uint64_t threshold; - log_debug("Swap used (%"PRIu64") / total (%"PRIu64") is more than " PERMYRIAD_AS_PERCENT_FORMAT_STR, + log_debug("Memory used (%"PRIu64") / total (%"PRIu64") and " + "swap used (%"PRIu64") / total (%"PRIu64") is more than " PERMYRIAD_AS_PERCENT_FORMAT_STR, + m->system_context.mem_used, m->system_context.mem_total, m->system_context.swap_used, m->system_context.swap_total, PERMYRIAD_AS_PERCENT_FORMAT_VAL(m->swap_used_limit_permyriad)); @@ -365,9 +365,12 @@ static int monitor_swap_contexts_handler(sd_event_source *s, uint64_t usec, void log_notice_errno(r, "Failed to kill any cgroup(s) based on swap: %m"); else { if (selected) - log_notice("Killed %s due to swap used (%"PRIu64") / total (%"PRIu64") being more than " + log_notice("Killed %s due to memory used (%"PRIu64") / total (%"PRIu64") and " + "swap used (%"PRIu64") / total (%"PRIu64") being more than " PERMYRIAD_AS_PERCENT_FORMAT_STR, - selected, m->system_context.swap_used, m->system_context.swap_total, + selected, + m->system_context.mem_used, m->system_context.mem_total, + m->system_context.swap_used, m->system_context.swap_total, PERMYRIAD_AS_PERCENT_FORMAT_VAL(m->swap_used_limit_permyriad)); return 0; } diff --git a/src/oom/oomd-util.c b/src/oom/oomd-util.c index 0550ac6c74..04d2bfe0bd 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" @@ -121,6 +122,16 @@ uint64_t oomd_pgscan_rate(const OomdCGroupContext *c) { return c->pgscan - last_pgscan; } +bool oomd_mem_free_below(const OomdSystemContext *ctx, int threshold_permyriad) { + uint64_t mem_threshold; + + assert(ctx); + assert(threshold_permyriad <= 10000); + + mem_threshold = ctx->mem_total * threshold_permyriad / (uint64_t) 10000; + return (ctx->mem_total - ctx->mem_used) < mem_threshold; +} + bool oomd_swap_free_below(const OomdSystemContext *ctx, int threshold_permyriad) { uint64_t swap_threshold; @@ -357,46 +368,70 @@ 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 mem_free, 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 (r == EOF && feof(f)) - break; + if ((word = startswith(line, "MemTotal:"))) { + field_filled |= 1U << 0; + r = convert_meminfo_value_to_uint64_bytes(word, &ctx.mem_total); + } else if ((word = startswith(line, "MemFree:"))) { + field_filled |= 1U << 1; + r = convert_meminfo_value_to_uint64_bytes(word, &mem_free); + } else if ((word = startswith(line, "SwapTotal:"))) { + field_filled |= 1U << 2; + r = convert_meminfo_value_to_uint64_bytes(word, &ctx.swap_total); + } else if ((word = startswith(line, "SwapFree:"))) { + field_filled |= 1U << 3; + r = convert_meminfo_value_to_uint64_bytes(word, &swap_free); + } else + continue; - if (r != 2) { - if (ferror(f)) - return log_debug_errno(errno, "Error reading from %s: %m", proc_swaps_path); + if (r < 0) + return log_debug_errno(r, "Error converting '%s' from %s to uint64_t: %m", line, proc_meminfo_path); - return log_debug_errno(SYNTHETIC_ERRNO(EIO), - "Failed to parse values from %s: %m", proc_swaps_path); - } - - ctx.swap_total += total * 1024U; - ctx.swap_used += used * 1024U; + if (field_filled == 15U) + break; } + if (field_filled != 15U) + return log_debug_errno(SYNTHETIC_ERRNO(EINVAL), "%s is missing expected fields", proc_meminfo_path); + + if (mem_free > ctx.mem_total) + return log_debug_errno(SYNTHETIC_ERRNO(EINVAL), + "MemFree (%" PRIu64 ") cannot be greater than MemTotal (%" PRIu64 ") %m", + mem_free, + ctx.mem_total); + + 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.mem_used = ctx.mem_total - mem_free; + ctx.swap_used = ctx.swap_total - swap_free; + *ret = ctx; return 0; } @@ -513,14 +548,19 @@ void oomd_dump_memory_pressure_cgroup_context(const OomdCGroupContext *ctx, FILE } void oomd_dump_system_context(const OomdSystemContext *ctx, FILE *f, const char *prefix) { - char used[FORMAT_BYTES_MAX], total[FORMAT_BYTES_MAX]; + char mem_used[FORMAT_BYTES_MAX], mem_total[FORMAT_BYTES_MAX]; + char swap_used[FORMAT_BYTES_MAX], swap_total[FORMAT_BYTES_MAX]; assert(ctx); assert(f); fprintf(f, + "%sMemory: Used: %s Total: %s\n" "%sSwap: Used: %s Total: %s\n", strempty(prefix), - format_bytes(used, sizeof(used), ctx->swap_used), - format_bytes(total, sizeof(total), ctx->swap_total)); + format_bytes(mem_used, sizeof(mem_used), ctx->mem_used), + format_bytes(mem_total, sizeof(mem_total), ctx->mem_total), + strempty(prefix), + format_bytes(swap_used, sizeof(swap_used), ctx->swap_used), + format_bytes(swap_total, sizeof(swap_total), ctx->swap_total)); } diff --git a/src/oom/oomd-util.h b/src/oom/oomd-util.h index 81fdc5e088..3a91a31352 100644 --- a/src/oom/oomd-util.h +++ b/src/oom/oomd-util.h @@ -39,6 +39,8 @@ struct OomdCGroupContext { }; struct OomdSystemContext { + uint64_t mem_total; + uint64_t mem_used; uint64_t swap_total; uint64_t swap_used; }; @@ -57,6 +59,9 @@ DEFINE_TRIVIAL_CLEANUP_FUNC(OomdCGroupContext*, oomd_cgroup_context_free); * Returns -ENOMEM for allocation errors. */ int oomd_pressure_above(Hashmap *h, usec_t duration, Set **ret); +/* Returns true if the amount of memory free is below the permyriad of memory specified by `threshold_permyriad`. */ +bool oomd_mem_free_below(const OomdSystemContext *ctx, int threshold_permyriad); + /* Returns true if the amount of swap free is below the permyriad of swap specified by `threshold_permyriad`. */ bool oomd_swap_free_below(const OomdSystemContext *ctx, int threshold_permyriad); diff --git a/src/oom/test-oomd-util.c b/src/oom/test-oomd-util.c index 7ebea29c1a..776c65820e 100644 --- a/src/oom/test-oomd-util.c +++ b/src/oom/test-oomd-util.c @@ -247,26 +247,31 @@ 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) == 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, "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, "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(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(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 == 21474828288); - assert_se(ctx.swap_used == 1017630720); + assert_se(ctx.mem_total == 33275142144); + assert_se(ctx.mem_used == 23157497856); + assert_se(ctx.swap_total == 8589930496); + assert_se(ctx.swap_used == 8582144000); } static void test_oomd_pressure_above(void) { @@ -317,23 +322,32 @@ static void test_oomd_pressure_above(void) { assert_se(c->mem_pressure_limit_hit_start == 0); } -static void test_oomd_swap_free_below(void) { +static void test_oomd_mem_and_swap_free_below(void) { OomdSystemContext ctx = (OomdSystemContext) { + .mem_total = 20971512 * 1024U, + .mem_used = 3310136 * 1024U, .swap_total = 20971512 * 1024U, .swap_used = 20971440 * 1024U, }; + assert_se(oomd_mem_free_below(&ctx, 2000) == false); assert_se(oomd_swap_free_below(&ctx, 2000) == true); ctx = (OomdSystemContext) { + .mem_total = 20971512 * 1024U, + .mem_used = 20971440 * 1024U, .swap_total = 20971512 * 1024U, .swap_used = 3310136 * 1024U, }; + assert_se(oomd_mem_free_below(&ctx, 2000) == true); assert_se(oomd_swap_free_below(&ctx, 2000) == false); ctx = (OomdSystemContext) { + .mem_total = 0, + .mem_used = 0, .swap_total = 0, .swap_used = 0, }; + assert_se(oomd_mem_free_below(&ctx, 2000) == false); assert_se(oomd_swap_free_below(&ctx, 2000) == false); } @@ -433,7 +447,7 @@ int main(void) { test_oomd_update_cgroup_contexts_between_hashmaps(); test_oomd_system_context_acquire(); test_oomd_pressure_above(); - test_oomd_swap_free_below(); + test_oomd_mem_and_swap_free_below(); test_oomd_sort_cgroups(); /* The following tests operate on live cgroups */ diff --git a/src/shared/psi-util.c b/src/shared/psi-util.c index 97a4353792..009095e8c3 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;