From:   Haifeng Xu <haifeng.xu@shopee.com>
To:     mhocko@suse.com
Cc:     shakeelb@google.com, hannes@cmpxchg.org, akpm@linux-foundation.org,
        linux-mm@kvack.org, linux-kernel@vger.kernel.org,
        Haifeng Xu <haifeng.xu@shopee.com>
Subject: [PATCH RESEND] mm/oom_kill: don't kill exiting tasks in oom_kill_memcg_member
Date:   Tue, 14 Mar 2023 09:11:36 +0000
Message-Id: <20230314091136.264878-1-haifeng.xu@shopee.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

If oom_group is set, oom_kill_process() invokes oom_kill_memcg_member()
to kill all processes in the memcg. When scanning tasks in memcg, maybe
the provided task is marked as oom victim. Also, some tasks are likely
to release their address space. There is no need to kill the exiting tasks.

In order to handle these tasks which may free memory in the future, add
a function helper reap_task_will_free_mem() to mark it as oom victim and
queue it in oom reaper.

Signed-off-by: Haifeng Xu <haifeng.xu@shopee.com>
---
 mm/oom_kill.c | 35 +++++++++++++++++++++++++++++------
 1 file changed, 29 insertions(+), 6 deletions(-)

diff --git a/mm/oom_kill.c b/mm/oom_kill.c
index 044e1eed720e..f16bca506dc2 100644
--- a/mm/oom_kill.c
+++ b/mm/oom_kill.c
@@ -996,15 +996,43 @@ static void __oom_kill_process(struct task_struct *victim, const char *message)
 }
 #undef K
 
+static bool reap_task_will_free_mem(struct task_struct *victim)
+{
+	bool ret = false;
+
+	task_lock(victim);
+	if (task_will_free_mem(victim)) {
+		mark_oom_victim(victim);
+		queue_oom_reaper(victim);
+		ret = true;
+	}
+	task_unlock(victim);
+
+	return ret;
+}
+
 /*
  * Kill provided task unless it's secured by setting
  * oom_score_adj to OOM_SCORE_ADJ_MIN.
+ * If the task is marked as oom victim or will free
+ * memory, there is no need to kill it again.
  */
 static int oom_kill_memcg_member(struct task_struct *task, void *message)
 {
 	if (task->signal->oom_score_adj != OOM_SCORE_ADJ_MIN &&
 	    !is_global_init(task)) {
 		get_task_struct(task);
+
+		/*
+		 * If the task is already exiting, don't alarm the sysadmin or kill
+		 * its children or threads, just give it access to memory reserves
+		 * so it can die quickly
+		 */
+		if (tsk_is_oom_victim(task) || reap_task_will_free_mem(task)) {
+			put_task_struct(task);
+			return 0;
+		}
+
 		__oom_kill_process(task, message);
 	}
 	return 0;
@@ -1022,15 +1050,10 @@ static void oom_kill_process(struct oom_control *oc, const char *message)
 	 * its children or threads, just give it access to memory reserves
 	 * so it can die quickly
 	 */
-	task_lock(victim);
-	if (task_will_free_mem(victim)) {
-		mark_oom_victim(victim);
-		queue_oom_reaper(victim);
-		task_unlock(victim);
+	if (reap_task_will_free_mem(victim)) {
 		put_task_struct(victim);
 		return;
 	}
-	task_unlock(victim);
 
 	if (__ratelimit(&oom_rs))
 		dump_header(oc, victim);
-- 
2.25.1