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

[Vladimir Davydov]

On Tue, May 24, 2016 at 03:50:42PM +0200, Michal Hocko wrote:
> 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.

IMO the fact that a process, even an exiting one enters oom, is
abnormal, indicates that the system is misconfigured, and hence should
be reported to the admin.

> Those races is quite unlikely but not impossible.

If this case is unlikely, how can it spam the log?

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

I don't agree that a message about oom killing an exiting process is
noise, because that shouldn't happen on a properly configured system.
To me this racy check looks more like noise in the kernel code. By the
time we enter oom we should have scanned lru several times to find no
reclaimable pages. The system must be really sluggish. What's the point
in deceiving the admin by suppressing the warning?