Re: [PATCH 3/5] oom: clear TIF_MEMDIE after oom_reaper managed to unmap the address space
From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Date: 2016-02-06 14:33:31
Also in:
lkml
Michal Hocko wrote:
On Thu 04-02-16 15:43:19, Michal Hocko wrote:quoted
On Thu 04-02-16 23:22:18, Tetsuo Handa wrote:quoted
Michal Hocko wrote:quoted
From: Michal Hocko <mhocko@suse.com> When oom_reaper manages to unmap all the eligible vmas there shouldn't be much of the freable memory held by the oom victim left anymore so it makes sense to clear the TIF_MEMDIE flag for the victim and allow the OOM killer to select another task.Just a confirmation. Is it safe to clear TIF_MEMDIE without reaching do_exit() with regard to freezing_slow_path()? Since clearing TIF_MEMDIE from the OOM reaper confuses wait_event(oom_victims_wait, !atomic_read(&oom_victims)); in oom_killer_disable(), I'm worrying that the freezing operation continues before the OOM victim which escaped the __refrigerator() actually releases memory. Does this cause consistency problem?This is a good question! At first sight it seems this is not safe and we might need to make the oom_reaper freezable so that it doesn't wake up during suspend and interfere. Let me think about that.OK, I was thinking about it some more and it seems you are right here. oom_reaper as a kernel thread is not freezable automatically and so it might interfere after all the processes/kernel threads are considered frozen. Then it really might shut down TIF_MEMDIE too early and wake out oom_killer_disable. wait_event_freezable is not sufficient because the oom_reaper might running while the PM freezer is freezing tasks and it will miss it because it doesn't see it.
I'm not using PM freezer, but your answer is opposite to my guess. I thought try_to_freeze_tasks(false) is called by freeze_kernel_threads() after oom_killer_disable() succeeded, and try_to_freeze_tasks(false) will freeze both userspace tasks (including OOM victims which got TIF_MEMDIE cleared by the OOM reaper) and kernel threads (including the OOM reaper). Thus, I was guessing that clearing TIF_MEMDIE without reaching do_exit() is safe.
So I think we might need this. I am heading to vacation today and will be offline for the next week so I will prepare the full patch with the proper changelog after I get back:
I can't judge whether we need this set_freezable().
quoted hunk ↗ jump to hunk
diff --git a/mm/oom_kill.c b/mm/oom_kill.c index ca61e6cfae52..7e9953a64489 100644 --- a/mm/oom_kill.c +++ b/mm/oom_kill.c@@ -521,6 +521,8 @@ static void oom_reap_task(struct task_struct *tsk) static int oom_reaper(void *unused) { + set_freezable(); + while (true) { struct task_struct *tsk = NULL;-- 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>