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

[Michal Hocko]

On Thu 28-01-16 16:36:34, Johannes Weiner wrote:
> 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.

My testing shows that this doesn't trigger even during oom flood
testing. So I am not really convinced it does anything useful.

> Please drop this patch.

Sure I do not insist...
-- 
Michal Hocko
SUSE Labs