oomd: calculate 'used' memory with MemAvailable instead of MemFree

The calculation for used memory in oomd_system_context_acquire is given
by MemTotal - MemFree from /proc/meminfo. This is too strict of a
calculation because it does not consider memory that is still available
for starting new applictions without swapping (MemAvailable). As a
result, systemd-oomd can start to kill processes before it is necessary.
This is more apparent on systems with low swap space.

Instead, compute 'used' memory as MemTotal - MemAvailable in
oomd_system_context_acquire and procfs_memory_get (which is used by
oomd_cgroup_context_acquire). And, rename oomd_mem_free_below to
oomd_mem_available_below for clarity.

diff --git a/src/basic/procfs-util.c b/src/basic/procfs-util.c
index 65f96abb0625a8ff3014864f53892a44de236727..4f607307b61a9d4dedd206a7619ebd200368a85d 100644 (file)
--- a/src/basic/procfs-util.c
+++ b/src/basic/procfs-util.c
@@ -219,7 +219,7 @@ int convert_meminfo_value_to_uint64_bytes(const char *word, uint64_t *ret) {
 }
 
 int procfs_memory_get(uint64_t *ret_total, uint64_t *ret_used) {
-        uint64_t mem_total = UINT64_MAX, mem_free = UINT64_MAX;
+        uint64_t mem_total = UINT64_MAX, mem_available = UINT64_MAX;
         _cleanup_fclose_ FILE *f = NULL;
         int r;
 
@@ -242,9 +242,9 @@ int procfs_memory_get(uint64_t *ret_total, uint64_t *ret_used) {
                 if (p)
                         v = &mem_total;
                 else {
-                        p = first_word(line, "MemFree:");
+                        p = first_word(line, "MemAvailable:");
                         if (p)
-                                v = &mem_free;
+                                v = &mem_available;
                         else
                                 continue;
                 }
@@ -253,16 +253,16 @@ int procfs_memory_get(uint64_t *ret_total, uint64_t *ret_used) {
                 if (r < 0)
                         return r;
 
-                if (mem_total != UINT64_MAX && mem_free != UINT64_MAX)
+                if (mem_total != UINT64_MAX && mem_available != UINT64_MAX)
                         break;
         }
 
-        if (mem_free > mem_total)
+        if (mem_available > mem_total)
                 return -EINVAL;
 
         if (ret_total)
                 *ret_total = mem_total;
         if (ret_used)
-                *ret_used = mem_total - mem_free;
+                *ret_used = mem_total - mem_available;
         return 0;
 }

diff --git a/src/oom/oomd-manager.c b/src/oom/oomd-manager.c
index b0a81474ccf961311ae64e69e249aba36916da58..cbbf95a7e4c79dd37d4fa3abfcdc65eba130e562 100644 (file)
--- a/src/oom/oomd-manager.c
+++ b/src/oom/oomd-manager.c
@@ -384,8 +384,8 @@ 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). */
 
-        /* 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) &&
+        /* Check amount of memory available and swap free so we don't free up swap when memory is still available. */
+        if (oomd_mem_available_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;

diff --git a/src/oom/oomd-util.c b/src/oom/oomd-util.c
index a135824c5368e5083ecbd17eaca36e0533677ff9..d1ab528a0ef8a5645cbdd609ae85aae5c7486b8b 100644 (file)
--- a/src/oom/oomd-util.c
+++ b/src/oom/oomd-util.c
@@ -122,7 +122,7 @@ uint64_t oomd_pgscan_rate(const OomdCGroupContext *c) {
         return c->pgscan - last_pgscan;
 }
 
-bool oomd_mem_free_below(const OomdSystemContext *ctx, int threshold_permyriad) {
+bool oomd_mem_available_below(const OomdSystemContext *ctx, int threshold_permyriad) {
         uint64_t mem_threshold;
 
         assert(ctx);
@@ -418,15 +418,15 @@ int oomd_system_context_acquire(const char *proc_meminfo_path, OomdSystemContext
         _cleanup_fclose_ FILE *f = NULL;
         unsigned field_filled = 0;
         OomdSystemContext ctx = {};
-        uint64_t mem_free, swap_free;
+        uint64_t mem_available, swap_free;
         int r;
 
         enum {
                 MEM_TOTAL = 1U << 0,
-                MEM_FREE = 1U << 1,
+                MEM_AVAILABLE = 1U << 1,
                 SWAP_TOTAL = 1U << 2,
                 SWAP_FREE = 1U << 3,
-                ALL = MEM_TOTAL|MEM_FREE|SWAP_TOTAL|SWAP_FREE,
+                ALL = MEM_TOTAL|MEM_AVAILABLE|SWAP_TOTAL|SWAP_FREE,
         };
 
         assert(proc_meminfo_path);
@@ -449,9 +449,9 @@ int oomd_system_context_acquire(const char *proc_meminfo_path, OomdSystemContext
                 if ((word = startswith(line, "MemTotal:"))) {
                         field_filled |= MEM_TOTAL;
                         r = convert_meminfo_value_to_uint64_bytes(word, &ctx.mem_total);
-                } else if ((word = startswith(line, "MemFree:"))) {
-                        field_filled |= MEM_FREE;
-                        r = convert_meminfo_value_to_uint64_bytes(word, &mem_free);
+                } else if ((word = startswith(line, "MemAvailable:"))) {
+                        field_filled |= MEM_AVAILABLE;
+                        r = convert_meminfo_value_to_uint64_bytes(word, &mem_available);
                 } else if ((word = startswith(line, "SwapTotal:"))) {
                         field_filled |= SWAP_TOTAL;
                         r = convert_meminfo_value_to_uint64_bytes(word, &ctx.swap_total);
@@ -471,10 +471,10 @@ int oomd_system_context_acquire(const char *proc_meminfo_path, OomdSystemContext
         if (field_filled != ALL)
                 return log_debug_errno(SYNTHETIC_ERRNO(EINVAL), "%s is missing expected fields", proc_meminfo_path);
 
-        if (mem_free > ctx.mem_total)
+        if (mem_available > ctx.mem_total)
                 return log_debug_errno(SYNTHETIC_ERRNO(EINVAL),
-                                       "MemFree (%" PRIu64 ") cannot be greater than MemTotal (%" PRIu64 ") %m",
-                                       mem_free,
+                                       "MemAvailable (%" PRIu64 ") cannot be greater than MemTotal (%" PRIu64 ") %m",
+                                       mem_available,
                                        ctx.mem_total);
 
         if (swap_free > ctx.swap_total)
@@ -483,7 +483,7 @@ int oomd_system_context_acquire(const char *proc_meminfo_path, OomdSystemContext
                                        swap_free,
                                        ctx.swap_total);
 
-        ctx.mem_used = ctx.mem_total - mem_free;
+        ctx.mem_used = ctx.mem_total - mem_available;
         ctx.swap_used = ctx.swap_total - swap_free;
 
         *ret = ctx;

diff --git a/src/oom/oomd-util.h b/src/oom/oomd-util.h
index 6aada9034414f117e637233b5a1e4555cffb80d7..de2298f3c975b892c02c59b4089e61ed84ee8bfe 100644 (file)
--- a/src/oom/oomd-util.h
+++ b/src/oom/oomd-util.h
@@ -60,8 +60,8 @@ 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 memory available (see proc(5)) is below the permyriad of memory specified by `threshold_permyriad`. */
+bool oomd_mem_available_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 82a60ad8803e9fda856ad333e6584a2551822a65..19173edcdae89bf143ed7fd410f17f51874c1313 100644 (file)
--- a/src/oom/test-oomd-util.c
+++ b/src/oom/test-oomd-util.c
@@ -282,7 +282,7 @@ static void test_oomd_system_context_acquire(void) {
                                           "SwapFree:           7604 kB\n", WRITE_STRING_FILE_CREATE) == 0);
         assert_se(oomd_system_context_acquire(path, &ctx) == 0);
         assert_se(ctx.mem_total == 33275142144);
-        assert_se(ctx.mem_used == 23157497856);
+        assert_se(ctx.mem_used == 10975404032);
         assert_se(ctx.swap_total == 8589930496);
         assert_se(ctx.swap_used == 8582144000);
 }
@@ -341,7 +341,7 @@ static void test_oomd_mem_and_swap_free_below(void) {
                 .swap_total = 20971512 * 1024U,
                 .swap_used = 20971440 * 1024U,
         };
-        assert_se(oomd_mem_free_below(&ctx, 2000) == false);
+        assert_se(oomd_mem_available_below(&ctx, 2000) == false);
         assert_se(oomd_swap_free_below(&ctx, 2000) == true);
 
         ctx = (OomdSystemContext) {
@@ -350,7 +350,7 @@ static void test_oomd_mem_and_swap_free_below(void) {
                 .swap_total = 20971512 * 1024U,
                 .swap_used = 3310136 * 1024U,
         };
-        assert_se(oomd_mem_free_below(&ctx, 2000) == true);
+        assert_se(oomd_mem_available_below(&ctx, 2000) == true);
         assert_se(oomd_swap_free_below(&ctx, 2000) == false);
 
         ctx = (OomdSystemContext) {
@@ -359,7 +359,7 @@ static void test_oomd_mem_and_swap_free_below(void) {
                 .swap_total = 0,
                 .swap_used = 0,
         };
-        assert_se(oomd_mem_free_below(&ctx, 2000) == false);
+        assert_se(oomd_mem_available_below(&ctx, 2000) == false);
         assert_se(oomd_swap_free_below(&ctx, 2000) == false);
 }