Re: [REPOST] [PATCH 2/2] mm,oom: Reverse the order of setting TIF_MEMDIE and sending SIGKILL.

Michal Hocko wrote:

quoted

quoted

diff --git a/mm/oom_kill.c b/mm/oom_kill.c
index 5249e7e..c0a5a69 100644
--- a/mm/oom_kill.c
+++ b/mm/oom_kill.c

@@ -555,12 +555,17 @@ void oom_kill_process(struct oom_control *oc, struct task_struct *p,
 	/* Get a reference to safely compare mm after task_unlock(victim) */
 	mm = victim->mm;
 	atomic_inc(&mm->mm_users);
-	mark_oom_victim(victim);
 	pr_err("Killed process %d (%s) total-vm:%lukB, anon-rss:%lukB, file-rss:%lukB\n",
 		task_pid_nr(victim), victim->comm, K(victim->mm->total_vm),
 		K(get_mm_counter(victim->mm, MM_ANONPAGES)),
 		K(get_mm_counter(victim->mm, MM_FILEPAGES)));
 	task_unlock(victim);
+	/* Send SIGKILL before setting TIF_MEMDIE. */
+	do_send_sig_info(SIGKILL, SEND_SIG_FORCED, victim, true);
+	task_lock(victim);
+	if (victim->mm)
+		mark_oom_victim(victim);
+	task_unlock(victim);

Why cannot you simply move do_send_sig_info without touching
mark_oom_victim? Are you still able to trigger the issue if you just
kill before crawling through all the tasks sharing the mm?

If you meant

diff --git a/mm/oom_kill.c b/mm/oom_kill.c
index 1ecc0bc..ea578fb 100644
--- a/mm/oom_kill.c
+++ b/mm/oom_kill.c

@@ -560,6 +560,7 @@ void oom_kill_process(struct oom_control *oc, struct task_struct *p,
                K(get_mm_counter(victim->mm, MM_ANONPAGES)),
                K(get_mm_counter(victim->mm, MM_FILEPAGES)));
        task_unlock(victim);
+       do_send_sig_info(SIGKILL, SEND_SIG_FORCED, victim, true);

        /*
         * Kill all user processes sharing victim->mm in other thread groups, if

@@ -585,7 +586,6 @@ void oom_kill_process(struct oom_control *oc, struct task_struct *p,
                }
        rcu_read_unlock();

-       do_send_sig_info(SIGKILL, SEND_SIG_FORCED, victim, true);
        put_task_struct(victim);
 }
 #undef K

then yes I still can trigger the issue under very limited condition (i.e.
ran as root user for polling kernel messages with realtime priority, after
killing all processes using SysRq-i).