Re: why do we do ALLOC_WMARK_HIGH before going out_of_memory

[Michal Hocko <mhocko@suse.cz>]

On Fri 29-01-16 16:56:45, Andrea Arcangeli wrote:
> On Fri, Jan 29, 2016 at 03:38:06PM +0100, Michal Hocko wrote:
> > That would require the oom victim to release the memory and drop
> > TIF_MEMDIE before we go out_of_memory again. And that might happen
> > anytime whether we are holding oom_trylock or not because it doesn't
> > synchronize the exit path. So we are basically talking about:
> > 
> > should_alloc_retry
> > [1]
> > get_page_from_freelist(ALLOC_WMARK_HIGH)
> > [2]
> > out_of_memory
> > 
> > and the race window for 1 is much smaller than 2 because [2] is quite
> 
> [1] is before should_alloc_retry is set. It covers the entire reclaim.
> 
> > costly operation. I wonder if this last moment request ever succeeds. I
> > have run my usual oom flood tests and it hasn't shown up a single time.
> 
> For this check to make a difference, you need a lot of small programs
> all hitting OOM at the same time.

That is essentially my oom flood testing program doing. Spawning
hundreds of paralell anon mem eaters.

> Perhaps the trylock on the oom_lock
> tends to hide the race like you suggested earlier but it doesn't sound
> accurate if we proceed to oom kill without checking the high wmark at all
> before killing another task after a random reclaim failure.

The thing is that the reclaim would have to reclaim consistently after
the rework.

> Also note there's no CPU to save here, this is a very slow path,
> anything that can increase accuracy and avoid OOM kill false
> positives (at practical zero CPU cost like here) sounds worth it.

Sure my idea wasn't to save the CPU. The code was just a head scratcher
and an attempt for a clean up. We have more places where we keep some
heuristics just because we have them since ever and it is hard to judge
what effect they have exactly.

> > That being said I do not care that much. I just find this confusing and
> > basically pointless because the whole thing is racy by definition and we
> > are trying to cover a smaller window. I would understand if we did such
> > a last attempt right before we are going to kill a selected victim. This
> > would cover much larger race window.
> 
> The high wmark itself is still an arbitrary value so yes, it isn't
> perfect, but the whole OOM killing is an heuristic, so tiny race
> window to me sounds better than huge race window.
> 
> Moving this check down inside out_of_memory to reduce the window even
> further is quite a different proposition than removing the check.
> 
> Currently we're doing this check after holding the oom_lock, back in
> 2.6.x it was more more racy, now thanks to the oom_lock it's way more
> reliable. If you want to increase reliability further I sure agree,
> but removing the check would drop reliability instead so I don't see
> how it could be preferable.
> 
> We can increase reliability further if we'd move this high wmark check
> after select_bad_process() returned a task (and not -1UL) to be sure
> all TIF_MEMDIE tasks already were flushed out, before checking the
> high wmark. Just it would complicate the code and that's probably why
> it wasn't done.

This would be mixing two layers which is not nice. Johannes was
proposing a different approach [1] which sounds much better to me.

[1] http://lkml.kernel.org/r/20160128235110.GA5805@cmpxchg.org
-- 
Michal Hocko
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>