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.1506091542120.30516@chino.kir.corp.google.com>
Date:	Tue, 9 Jun 2015 15:45:35 -0700 (PDT)
From:	David Rientjes <rientjes@...gle.com>
To:	Michal Hocko <mhocko@...e.cz>
cc:	Austin S Hemmelgarn <ahferroin7@...il.com>,
	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, 9 Jun 2015, Michal Hocko wrote:

> > Yes, and that's why I believe we should pursue that direction without the 
> > associated "cleanup" that adds 35 lines of code to supress a panic.  In 
> > other words, there's no reason to combine a patch that suppresses the 
> > panic even with panic_on_oom, which I support, and a "cleanup" that I 
> > believe just obfuscates the code.
> > 
> > It's a one-liner change: just test for force_kill and suppress the panic; 
> > we don't need 35 new lines that create even more unique entry paths.
> 
> I completely detest yet another check in out_of_memory. And there is
> even no reason to do that. Forced kill and genuine oom have different
> objectives and combining those two just makes the code harder to read
> (one has to go to check the syrq callback to realize that the forced
> path is triggered from the workqueue context and that current->mm !=
> NULL check will prevent some heuristics. This is just too ugly to
> live). So why the heck are you pushing for keeping everything in a
> single path?
> 

Perhaps if you renamed "force_kill" to "sysrq" it would make more sense to 
you?

I don't think the oom killer needs multiple entry points that duplicates 
code and adds more than twice the lines it removes.  It would make sense 
if that was an optimization in a hot path, or a warm path, or even a 
luke-warm path, but not an icy cold path like the oom killer.  
check_panic_on_oom() can simply do

	if (sysrq)
		return;

It's not hard and it's very clear.  We don't need 35 more lines of code to 
do this.
--
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/