From f4f1956d429668199a4cda9da332ca32b6e460de Mon Sep 17 00:00:00 2001 From: Chris Down Date: Mon, 16 Feb 2026 01:30:02 +0800 Subject: [PATCH 1/2] oomd: Fix bug where we drop queued kill state on duplicate cgroup oomd_cgroup_kill_mark() allocates a temporary OomdKillState and inserts it into kill_states via set_ensure_put(). This is keyed by cgroup path. When the same cgroup is already queued, set_ensure_put() dutifully returns 0. The function then returns with _cleanup_(oomd_kill_state_removep) still armed, which eventually calls oomd_kill_state_free(). oomd_kill_state_free() removes from kill_states by cgroup-path key, so because this path already exists, it will remove the existing queued kill state instead of just dropping the temporary object. This is wrong and results in mistakenly drops the queued kill state on duplicates. This can happen when a cgroup is marked multiple times before the first queued kill state is consumed. The result is lost kill-state tracking and incorrect prekill/kill sequencing. Handle r == 0 explicitly by freeing only the temporary object and leaving the already queued state intact. --- src/oom/oomd-util.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/oom/oomd-util.c b/src/oom/oomd-util.c index 340d2bb60e..97803a8428 100644 --- a/src/oom/oomd-util.c +++ b/src/oom/oomd-util.c @@ -494,6 +494,14 @@ int oomd_cgroup_kill_mark(Manager *m, OomdCGroupContext *ctx) { r = set_ensure_put(&m->kill_states, &oomd_kill_state_hash_ops, ks); if (r < 0) return log_oom_debug(); + if (r == 0) { + /* The cgroup is already queued. Drop only this temporary object, because running the normal + * cleanup path would remove by cgroup path key and could interfere with the existing queued + * kill state. */ + oomd_cgroup_context_unref(ks->ctx); + ks = mfree(ks); + return 0; + } r = oomd_prekill_hook(m, ks); if (r < 0) From e93511b10ea82bb65f4f5b38f98c52340eb20f83 Mon Sep 17 00:00:00 2001 From: Chris Down Date: Mon, 16 Feb 2026 01:31:12 +0800 Subject: [PATCH 2/2] oomd: Return tristate status from oomd_cgroup_kill_mark() oomd_cgroup_kill_mark() currently returns 0 on all non-error paths. But the manager only logs that it marked for killing on `if (r > 0)`, which is thus unreachable. Changing it to `r >= 0` would also be wrong, because then we would log on no-op paths. So let's fix this by making the return value express what actually happened: - < 0: failure to queue the kill state - 0: no new mark was created (already queued or dry-run) - > 0: a new kill state was queued --- src/oom/oomd-manager.c | 5 ++--- src/oom/oomd-util.c | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/oom/oomd-manager.c b/src/oom/oomd-manager.c index 3898ab707c..1ab7099d2e 100644 --- a/src/oom/oomd-manager.c +++ b/src/oom/oomd-manager.c @@ -541,9 +541,8 @@ static int monitor_memory_pressure_contexts_handler(sd_event_source *s, uint64_t log_error_errno(r, "Failed to select any cgroups under %s based on pressure, ignoring: %m", t->path); else { /* Don't act on all the high pressure cgroups at once; return as soon as we kill one. - * If r == 0 then it means there were not eligible candidates, the candidate cgroup - * disappeared, or the candidate cgroup has no processes by the time we tried to kill - * it. In either case, go through the event loop again and select a new candidate if + * If r == 0 then the cgroup is already queued for kill by an earlier iteration. + * In either case, go through the event loop again and select a new candidate if * pressure is still high. */ m->mem_pressure_post_action_delay_start = usec_now; if (selected && r > 0) { diff --git a/src/oom/oomd-util.c b/src/oom/oomd-util.c index 97803a8428..b044989ab3 100644 --- a/src/oom/oomd-util.c +++ b/src/oom/oomd-util.c @@ -507,7 +507,7 @@ int oomd_cgroup_kill_mark(Manager *m, OomdCGroupContext *ctx) { if (r < 0) log_warning_errno(r, "oomd prekill hook failed for %s, ignoring: %m", ctx->path); - return 0; + return 1; } typedef void (*dump_candidate_func)(const OomdCGroupContext *ctx, FILE *f, const char *prefix);