Re: [PATCH 3/3] mm, oom: protect !costly allocations some more

[Michal Hocko]

On Tue 08-03-16 15:34:37, Vlastimil Babka wrote:
> On 03/08/2016 02:42 PM, Michal Hocko wrote:
> > From: Michal Hocko <mhocko@xxxxxxxx>
> > 
> > should_reclaim_retry will give up retries for higher order allocations
> > if none of the eligible zones has any requested or higher order pages
> > available even if we pass the watermak check for order-0. This is done
> > because there is no guarantee that the reclaimable and currently free
> > pages will form the required order.
> > 
> > This can, however, lead to situations were the high-order request (e.g.
> > order-2 required for the stack allocation during fork) will trigger
> > OOM too early - e.g. after the first reclaim/compaction round. Such a
> > system would have to be highly fragmented and there is no guarantee
> > further reclaim/compaction attempts would help but at least make sure
> > that the compaction was active before we go OOM and keep retrying even
> > if should_reclaim_retry tells us to oom if the last compaction round
> > was either inactive (deferred, skipped or bailed out early due to
> > contention) or it told us to continue.
> > 
> > Signed-off-by: Michal Hocko <mhocko@xxxxxxxx>
> > ---
> >  include/linux/compaction.h |  5 +++++
> >  mm/page_alloc.c            | 53 ++++++++++++++++++++++++++++++++--------------
> >  2 files changed, 42 insertions(+), 16 deletions(-)
> > 
> > diff --git a/include/linux/compaction.h b/include/linux/compaction.h
> > index b167801187e7..49e04326dcb8 100644
> > --- a/include/linux/compaction.h
> > +++ b/include/linux/compaction.h
> > @@ -14,6 +14,11 @@ enum compact_result {
> >  	/* compaction should continue to another pageblock */
> >  	COMPACT_CONTINUE,
> >  	/*
> > +	 * whoever is calling compaction should retry because it was either
> > +	 * not active or it tells us there is more work to be done.
> > +	 */
> > +	COMPACT_SHOULD_RETRY = COMPACT_CONTINUE,
> 
> Hmm, I'm not sure about this. AFAIK compact_zone() doesn't ever return
> COMPACT_CONTINUE, and thus try_to_compact_pages() also doesn't. This
> overloading of CONTINUE only applies to compaction_suitable(). But the
> value that should_compact_retry() is testing comes only from
> try_to_compact_pages(). So this is not wrong, but perhaps a bit misleading?

Well the idea was that I wanted to cover all the _possible_ cases where
compaction might want to tell us "please try again even when the last
round wasn't really successful". COMPACT_CONTINUE might not be returned
right now but we can come up with that in the future. It sounds like a
sensible feedback to me. But maybe there would be a better name for such
a feedback. I confess this is a bit oom-rework centric name...

Also I find it better to hide details behind a more generic name.

I am open to suggestions here, of course.
-- 
Michal Hocko
SUSE Labs