linux-kernel - Re: [PATCH 4/4] 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]

Date:	Thu, 9 Jul 2015 12:05:41 +0200
From:	Michal Hocko <mhocko@...nel.org>
To:	David Rientjes <rientjes@...gle.com>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	Jakob Unterwurzacher <jakobunt@...il.com>, linux-mm@...ck.org,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 4/4] oom: split out forced OOM killer

On Wed 08-07-15 16:41:23, David Rientjes wrote:
> On Wed, 8 Jul 2015, Michal Hocko wrote:
> 
> > From: Michal Hocko <mhocko@...e.cz>
> > 
> > The forced OOM killing is currently wired into out_of_memory() call
> > even though their objective is different which makes the code ugly
> > and harder to follow. 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). All of them are
> > either relying on explicit force_kill check or indirectly by checking
> > current->mm which is always NULL for sysrq+f. This is not nice, hard
> > to follow and error prone.
> > 
> > Let's pull forced OOM killer code out into a separate function
> > (force_out_of_memory) which is really trivial now.
> > 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.
> > 
> > Signed-off-by: Michal Hocko <mhocko@...e.cz>
> 
> It's really absurd that we have to go through this over and over and that 
> your patches are actually being merged into -mm just because you don't get 
> the point.
> 
> We have no need for a force_out_of_memory() function.  None whatsoever.  

The reasons are explained in the changelog and I do not see a single
argument against any of them.

> Keeping oc->force_kill around is just more pointless space on a very deep 
> stack and I'm tired of fixing stack overflows.

This just doesn't make any sense. oc->force_kill vs oc->order =
-1 replacement is completely independent on this patch and can be
implemented on top of it if you really insist.

> I'm certainly not going to 
> introduce others because you think it looks cleaner in the code when 
> memory compaction does the exact same thing by using cc->order == -1 to 
> mean explicit compaction.
> 
> This is turning into a complete waste of time.

You know what? I am tired of your complete immunity to any arguments and
the way how you are pushing more hacks into an already cluttered code.

out_of_memory is a giant mess wrt. to force killing and you can see
at least two different bugs being there just because of the code
obfuscation. If this is the state that you want to keep, I do not
care. I wanted to fix real issues and do a clean up on top. You seem to
do anything to block that. I just give up.
-- 
Michal Hocko
SUSE Labs
--
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/