Re: [PATCH] mm,oom: use per signal_struct flag rather than clear TIF_MEMDIE

[Michal Hocko <mhocko@kernel.org>]

On Fri 24-06-16 20:02:01, Tetsuo Handa wrote:
> Currently, the OOM reaper calls exit_oom_victim() on remote TIF_MEMDIE
> thread after an OOM reap attempt was made. This behavior is intended
> for allowing oom_scan_process_thread() to select next OOM victim by
> making atomic_read(&task->signal->oom_victims) == 0.
> 
> But since threads can be blocked for unbounded period at __mmput() from
> mmput() from exit_mm() from do_exit(), we can't risk the OOM reaper
> being blocked for unbounded period waiting for TIF_MEMDIE threads.
> Therefore, when we hit a situation that a TIF_MEMDIE thread which is
> the only thread of that thread group reached tsk->mm = NULL line in
> exit_mm() from do_exit() before __oom_reap_task() finds a mm via
> find_lock_task_mm(), oom_reap_task() does not wait for the TIF_MEMDIE
> thread to return from __mmput() and instead calls exit_oom_victim().
> 
> Patch "mm, oom: hide mm which is shared with kthread or global init"
> tried to avoid OOM livelock by setting MMF_OOM_REAPED, but it is racy
> because setting MMF_OOM_REAPED will not help when find_lock_task_mm()
> in oom_scan_process_thread() failed.

I haven't thought that through yet (I will wait for the monday fresh
brain) but wouldn't the following be sufficient?
---
diff --git a/mm/oom_kill.c b/mm/oom_kill.c
index 4c21f744daa6..72360d7284a6 100644
--- a/mm/oom_kill.c
+++ b/mm/oom_kill.c
@@ -295,7 +295,8 @@ enum oom_scan_t oom_scan_process_thread(struct oom_control *oc,
 			if (test_bit(MMF_OOM_REAPED, &p->mm->flags))
 				ret = OOM_SCAN_CONTINUE;
 			task_unlock(p);
-		}
+		} else if (task->state == EXIT_ZOMBIE)
+			ret = OOM_SCAN_CONTINUE;
 
 		return ret;
 	}
@@ -592,14 +593,7 @@ static void oom_reap_task(struct task_struct *tsk)
 		debug_show_all_locks();
 	}
 
-	/*
-	 * Clear TIF_MEMDIE because the task shouldn't be sitting on a
-	 * reasonably reclaimable memory anymore or it is not a good candidate
-	 * for the oom victim right now because it cannot release its memory
-	 * itself nor by the oom reaper.
-	 */
 	tsk->oom_reaper_list = NULL;
-	exit_oom_victim(tsk);
 
 	/* Drop a reference taken by wake_oom_reaper */
 	put_task_struct(tsk);
-- 
Michal Hocko
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>