linux-kernel - Re: [PATCH] oom: split out forced OOM killer

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: <alpine.DEB.2.10.1506041557070.16555@chino.kir.corp.google.com>
Date:	Thu, 4 Jun 2015 15:59:09 -0700 (PDT)
From:	David Rientjes <rientjes@...gle.com>
To:	Michal Hocko <mhocko@...e.cz>
cc:	Andrew Morton <akpm@...ux-foundation.org>, linux-mm@...ck.org,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] oom: split out forced OOM killer

On Tue, 2 Jun 2015, Michal Hocko wrote:

> OOM killer might be triggered externally via sysrq+f. This is supposed
> to kill a task no matter what e.g. a task is selected even though there
> is an OOM victim on the way to exit. This is a big hammer for an admin
> to help to resolve a memory short condition when the system is not able
> to cope with it on its own in a reasonable time frame (e.g. when the
> system is trashing or the OOM killer cannot make sufficient progress).
> 
> The forced OOM killing is currently wired into out_of_memory()
> call which is kind of ugly because generic out_of_memory path
> has to deal with configuration settings and heuristics which
> are completely irrelevant to the forced OOM killer (e.g.
> sysctl_oom_kill_allocating_task or OOM killer prevention for already
> dying tasks). Some of those will not apply to sysrq because the handler
> runs from the worker context.
> check_panic_on_oom on the other hand will work and that is kind of
> unexpected because sysrq+f should be usable to kill a mem hog whether
> the global OOM policy is to panic or not.
> It also doesn't make much sense to panic the system when no task cannot
> be killed because admin has a separate sysrq for that purpose.
> 
> Let's pull forced OOM killer code out into a separate function
> (force_out_of_memory) which is really trivial now. Also extract the core
> of oom_kill_process into __oom_kill_process which doesn't do any
> OOM prevention heuristics.
> As a bonus we can clearly state that this is a forced OOM killer in the
> OOM message which is helpful to distinguish it from the regular OOM
> killer.
> 

I'm not sure what the benefit of this is, and it's adding more code.  
Having multiple pathways and requirements, such as constrained_alloc(), to 
oom kill a process isn't any clearer, in my opinion.  It also isn't 
intended to be optimized since the oom killer called from the page 
allocator and from sysrq aren't fastpaths.  To me, this seems like only a 
source code level change and doesn't make anything more clear but rather 
adds more code and obfuscates the entry path.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/