oomd: Fix Killed signal reason being lost

Emitting "oom" doesn't mesh with the org.freedesktop.oom1.Manager
Killed() API contract, which defines "memory-used" and "memory-pressure"
as possible reasons. Consumers that key off reason thus will either lose
policy attribution or may reject the unknown value completely.

Plumb the reason through so it is visible to consumers.

diff --git a/src/oom/oomd-manager.c b/src/oom/oomd-manager.c
index 1ab7099d2ec0ef060ace6923b4aa51a4d03ca975..09790b90f578e2b00312d65aec1d135c8570d24d 100644 (file)
--- a/src/oom/oomd-manager.c
+++ b/src/oom/oomd-manager.c
@@ -416,7 +416,7 @@ static int monitor_swap_contexts_handler(sd_event_source *s, uint64_t usec, void
                         return 0;
                 }
 
-                r = oomd_cgroup_kill_mark(m, selected);
+                r = oomd_cgroup_kill_mark(m, selected, "memory-used");
                 if (r == -ENOMEM)
                         return log_oom();
                 if (r < 0)
@@ -534,7 +534,7 @@ static int monitor_memory_pressure_contexts_handler(sd_event_source *s, uint64_t
                                 return 0;
                         }
 
-                        r = oomd_cgroup_kill_mark(m, selected);
+                        r = oomd_cgroup_kill_mark(m, selected, "memory-pressure");
                         if (r == -ENOMEM)
                                 return log_oom();
                         if (r < 0)

diff --git a/src/oom/oomd-util.c b/src/oom/oomd-util.c
index 8167e10a8b8075bc31d011a7dc9933ae3c3ed2d1..55e17df46f08c32a1663bcfe2a066f6ad8cfd98f 100644 (file)
--- a/src/oom/oomd-util.c
+++ b/src/oom/oomd-util.c
@@ -27,6 +27,7 @@
 typedef struct OomdKillState {
         Manager *manager;
         OomdCGroupContext *ctx;
+        const char *reason;
         /* This holds sd_varlink references */
         Set *links;
 } OomdKillState;
@@ -245,11 +246,12 @@ int oomd_sort_cgroup_contexts(Hashmap *h, oomd_compare_t compare_func, const cha
         return (int) k;
 }
 
-int oomd_cgroup_kill(Manager *m, OomdCGroupContext *ctx, bool recurse) {
+int oomd_cgroup_kill(Manager *m, OomdCGroupContext *ctx, bool recurse, const char *reason) {
         _cleanup_set_free_ Set *pids_killed = NULL;
         int r;
 
         assert(ctx);
+        assert(!m || reason);
 
         pids_killed = set_new(NULL);
         if (!pids_killed)
@@ -288,7 +290,7 @@ int oomd_cgroup_kill(Manager *m, OomdCGroupContext *ctx, bool recurse) {
                                           "Killed",
                                           "ss",
                                           ctx->path,
-                                          "oom");
+                                          reason);
 
         return !set_isempty(pids_killed);
 }
@@ -334,7 +336,7 @@ static void oomd_kill_state_remove(OomdKillState *ks) {
         if (!set_isempty(ks->links))
                 return;
 
-        r = oomd_cgroup_kill(ks->manager, ks->ctx, /* recurse= */ true);
+        r = oomd_cgroup_kill(ks->manager, ks->ctx, /* recurse= */ true, ks->reason);
         if (r < 0)
                 log_debug_errno(r, "Failed to kill cgroup '%s', ignoring: %m", ks->ctx->path);
         oomd_kill_state_free(ks);
@@ -465,11 +467,12 @@ static int oomd_prekill_hook(Manager *m, OomdKillState *ks) {
         return 0;
 }
 
-int oomd_cgroup_kill_mark(Manager *m, OomdCGroupContext *ctx) {
+int oomd_cgroup_kill_mark(Manager *m, OomdCGroupContext *ctx, const char *reason) {
         int r;
 
         assert(ctx);
         assert(m);
+        assert(reason);
 
         if (m->dry_run) {
                 _cleanup_free_ char *cg_path = NULL;
@@ -489,6 +492,7 @@ int oomd_cgroup_kill_mark(Manager *m, OomdCGroupContext *ctx) {
         *ks = (OomdKillState) {
                 .manager = m,
                 .ctx = oomd_cgroup_context_ref(ctx),
+                .reason = reason,
         };
 
         r = set_ensure_put(&m->kill_states, &oomd_kill_state_hash_ops, ks);

diff --git a/src/oom/oomd-util.h b/src/oom/oomd-util.h
index cf80c6f57b5818ff2de18ac5f05f5bb62ff08d81..d4e1a9207bd50ae1bf1e6201a5adad21cce4bff0 100644 (file)
--- a/src/oom/oomd-util.h
+++ b/src/oom/oomd-util.h
@@ -124,8 +124,8 @@ int oomd_sort_cgroup_contexts(Hashmap *h, oomd_compare_t compare_func, const cha
 int oomd_fetch_cgroup_oom_preference(OomdCGroupContext *ctx, const char *prefix);
 
 /* Returns a negative value on error, 0 if no processes were killed, or 1 if processes were killed. */
-int oomd_cgroup_kill(Manager *m, OomdCGroupContext *ctx, bool recurse);
-int oomd_cgroup_kill_mark(Manager *m, OomdCGroupContext *ctx);
+int oomd_cgroup_kill(Manager *m, OomdCGroupContext *ctx, bool recurse, const char *reason);
+int oomd_cgroup_kill_mark(Manager *m, OomdCGroupContext *ctx, const char *reason);
 
 /* The following oomd_kill_by_* functions return 1 if processes were killed, or negative otherwise. */
 /* If `prefix` is supplied, only cgroups whose paths start with `prefix` are eligible candidates. Otherwise,

diff --git a/src/oom/test-oomd-util.c b/src/oom/test-oomd-util.c
index 76ff43cb27210d2cb565240373a88bfc5310796f..0dada0de6c8d1034cf27eae656844e25c617813a 100644 (file)
--- a/src/oom/test-oomd-util.c
+++ b/src/oom/test-oomd-util.c
@@ -88,7 +88,11 @@ TEST(oomd_cgroup_kill) {
                 ASSERT_OK(fork_and_sleep(5, &two));
                 ASSERT_OK(cg_attach(subcgroup, two.pid));
 
-                ASSERT_OK_POSITIVE(oomd_cgroup_kill(NULL /* manager */, &(OomdCGroupContext){ .path = subcgroup }, false /* recurse */));
+                ASSERT_OK_POSITIVE(oomd_cgroup_kill(
+                                                   /* m= */ NULL,
+                                                   &(OomdCGroupContext){ .path = subcgroup },
+                                                   /* recurse= */ false,
+                                                   /* reason= */ NULL));
 
                 ASSERT_OK(cg_get_xattr(subcgroup, "user.oomd_ooms", &v, /* ret_size= */ NULL));
                 ASSERT_STREQ(v, i == 0 ? "1" : "2");