On Fri 29-01-16 16:56:45, Andrea Arcangeli wrote:

On Fri, Jan 29, 2016 at 03:38:06PM +0100, Michal Hocko wrote:

quoted

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.

quoted

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.

quoted

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>