[PATCH] oom, oom_reaper: Do not enqueue task if it is on the oom_reaper_list head

[PATCH] oom, oom_reaper: Do not enqueue task if it is on the oom_reaper_list head

[Michal Hocko]

From: Michal Hocko <mhocko@suse.com>

bb29902a7515 ("oom, oom_reaper: protect oom_reaper_list using simpler
way") has simplified the check for tasks already enqueued for the oom
reaper by checking tsk->oom_reaper_list != NULL. This check is not
sufficient because the tsk might be the head of the queue without any
other tasks queued and then we would simply lockup looping on the same
task. Fix the condition by checking for the head as well.

Fixes: bb29902a7515 ("oom, oom_reaper: protect oom_reaper_list using simpler way")
Signed-off-by: Michal Hocko <mhocko@suse.com>
---
Hi,
I have just noticed this after I started consolidating other oom_reaper
related changes I have here locally. I should have caught this during
the review already and I really feel ashamed I haven't because this is
really a trivial bug that should be obvious see...

 mm/oom_kill.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/mm/oom_kill.c b/mm/oom_kill.c
index b34d279a7ee6..86349586eacb 100644
--- a/mm/oom_kill.c
+++ b/mm/oom_kill.c
@@ -547,7 +547,11 @@ static int oom_reaper(void *unused)
 
 static void wake_oom_reaper(struct task_struct *tsk)
 {
-	if (!oom_reaper_th || tsk->oom_reaper_list)
+	if (!oom_reaper_th)
+		return;
+
+	/* tsk is already queued? */
+	if (tsk == oom_reaper_list || tsk->oom_reaper_list)
 		return;
 
 	get_task_struct(tsk);
-- 
2.7.0

--
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>

Re: [PATCH] oom, oom_reaper: Do not enqueue task if it is on the oom_reaper_list head

[Tetsuo Handa]

Michal Hocko wrote:
> From: Michal Hocko <mhocko@suse.com>
> 
> bb29902a7515 ("oom, oom_reaper: protect oom_reaper_list using simpler
> way") has simplified the check for tasks already enqueued for the oom
> reaper by checking tsk->oom_reaper_list != NULL. This check is not
> sufficient because the tsk might be the head of the queue without any
> other tasks queued and then we would simply lockup looping on the same
> task. Fix the condition by checking for the head as well.

Indeed, oom_reaper_list is initially NULL.

> 
> Fixes: bb29902a7515 ("oom, oom_reaper: protect oom_reaper_list using simpler way")
> Signed-off-by: Michal Hocko <mhocko@suse.com>
Acked-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>

> ---
> Hi,
> I have just noticed this after I started consolidating other oom_reaper
> related changes I have here locally. I should have caught this during
> the review already and I really feel ashamed I haven't because this is
> really a trivial bug that should be obvious see...

--
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>