linux-kernel - Re: [PATCH 4/3] mm, oom: drop the last allocation attempt before out_of

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: <20160128213634.GA4903@cmpxchg.org>
Date:	Thu, 28 Jan 2016 16:36:34 -0500
From:	Johannes Weiner <hannes@...xchg.org>
To:	Michal Hocko <mhocko@...nel.org>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Mel Gorman <mgorman@...e.de>,
	David Rientjes <rientjes@...gle.com>,
	Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>,
	Hillf Danton <hillf.zj@...baba-inc.com>,
	KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>,
	linux-mm@...ck.org, LKML <linux-kernel@...r.kernel.org>,
	Michal Hocko <mhocko@...e.com>
Subject: Re: [PATCH 4/3] mm, oom: drop the last allocation attempt before
 out_of_memory

On Thu, Jan 28, 2016 at 09:40:03PM +0100, Michal Hocko wrote:
> From: Michal Hocko <mhocko@...e.com>
> 
> __alloc_pages_may_oom has been doing get_page_from_freelist with
> ALLOC_WMARK_HIGH target before going out_of_memory and invoking the oom
> killer. This has two reasons as explained by Andrea:
> "
> : the reason for the high wmark is to reduce the likelihood of livelocks
> : and be sure to invoke the OOM killer, if we're still under pressure
> : and reclaim just failed. The high wmark is used to be sure the failure
> : of reclaim isn't going to be ignored. If using the min wmark like
> : you propose there's risk of livelock or anyway of delayed OOM killer
> : invocation.
> :
> : The reason for doing one last wmark check (regardless of the wmark
> : used) before invoking the oom killer, was just to be sure another OOM
> : killer invocation hasn't already freed a ton of memory while we were
> : stuck in reclaim. A lot of free memory generated by the OOM killer,
> : won't make a parallel reclaim more likely to succeed, it just creates
> : free memory, but reclaim only succeeds when it finds "freeable" memory
> : and it makes progress in converting it to free memory. So for the
> : purpose of this last check, the high wmark would work fine as lots of
> : free memory would have been generated in such case.
> "
> 
> This is no longer a concern after "mm, oom: rework oom detection"
> because should_reclaim_retry performs the water mark check right before
> __alloc_pages_may_oom is invoked. Remove the last moment allocation
> request as it just makes the code more confusing and doesn't really
> serve any purpose because a success is basically impossible otherwise
> should_reclaim_retry would force the reclaim to retry. So this is
> merely a code cleanup rather than a functional change.
> 
> Signed-off-by: Michal Hocko <mhocko@...e.com>

The check has to happen while holding the OOM lock, otherwise we'll
end up killing much more than necessary when there are many racing
allocations.

Please drop this patch.