Re: [PATCH 4/3] mm, oom: drop the last allocation attempt before out_of_memory

[Johannes Weiner]

On Thu, Jan 28, 2016 at 09:40:03PM +0100, Michal Hocko wrote:
> From: Michal Hocko <mhocko@xxxxxxxx>
> 
> __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@xxxxxxxx>

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.