linux-kernel - Re: [PATCH] mm: oom_kill_process: do not abort if the victim is exiting

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20160524135042.GK8259@dhcp22.suse.cz>
Date:	Tue, 24 May 2016 15:50:42 +0200
From:	Michal Hocko <mhocko@...nel.org>
To:	Vladimir Davydov <vdavydov@...tuozzo.com>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>,
	David Rientjes <rientjes@...gle.com>,
	Johannes Weiner <hannes@...xchg.org>, linux-mm@...ck.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] mm: oom_kill_process: do not abort if the victim is
 exiting

On Tue 24-05-16 15:24:02, Vladimir Davydov wrote:
> After selecting an oom victim, we first check if it's already exiting
> and if it is, we don't bother killing tasks sharing its mm. We do try to
> reap its mm though, but we abort if any of the processes sharing it is
> still alive. This might result in oom deadlock if an exiting task got
> stuck trying to acquire a lock held by another task sharing the same mm
> which needs memory to continue: if oom killer happens to keep selecting
> the stuck task, we won't even try to kill other processes or reap the
> mm.

I plan to extend task_will_free_mem to catch this case because we will
need it for other changes.

> The check in question was first introduced by commit 50ec3bbffbe8 ("oom:
> handle current exiting"). Initially it worked in conjunction with
> another check in select_bad_process() which forced selecting exiting
> task. The goal of that patch was selecting the current task on oom if it
> was exiting. Now, we select the current task if it's exiting or was
> killed anyway. And the check in select_bad_process() was removed by
> commit 6a618957ad17 ("mm: oom_kill: don't ignore oom score on exiting
> tasks"), because it prevented oom reaper. This just makes the remaining
> hunk in oom_kill_process pointless.

It is not really pointless. The original intention was to not spam the
log and alarm the administrator when in fact the memory hog is exiting
already and will free the memory. Those races is quite unlikely but not
impossible. The original check was much more optimistic as you said
above we have even removed one part of this heuristic. We can still end
up selecting an exiting task which is stuck and we could invoke the oom
reaper for it without excessive oom report. I agree that the current
check is still little bit optimistic but processes sharing the mm
(CLONE_VM without CLONE_THREAD/CLONE_SIGHAND) are really rare so I
wouldn't bother with them with a high priority.

That being said I would prefer to keep the check for now. After the
merge windlow closes I will send other oom enhancements which I have
half baked locally and that should make task_will_free_mem much more
reliable and the check would serve as a last resort to reduce oom noise.
-- 
Michal Hocko
SUSE Labs