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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20160311130647.GO27701@dhcp22.suse.cz>
Date:	Fri, 11 Mar 2016 14:06:47 +0100
From:	Michal Hocko <mhocko@...nel.org>
To:	Hugh Dickins <hughd@...gle.com>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>,
	Vlastimil Babka <vbabka@...e.cz>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Johannes Weiner <hannes@...xchg.org>,
	Mel Gorman <mgorman@...e.de>,
	David Rientjes <rientjes@...gle.com>,
	Tetsuo Handa <penguin-kernel@...ove.sakura.ne.jp>,
	Hillf Danton <hillf.zj@...baba-inc.com>,
	KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>,
	Joonsoo Kim <js1304@...il.com>, linux-mm@...ck.org,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 3/3] mm, oom: protect !costly allocations some more

On Fri 11-03-16 04:17:30, Hugh Dickins wrote:
> On Wed, 9 Mar 2016, Michal Hocko wrote:
> > Joonsoo has pointed out that this attempt is still not sufficient
> > becasuse we might have invoked only a single compaction round which
> > is might be not enough. I fully agree with that. Here is my take on
> > that. It is again based on the number of retries loop.
> > 
> > I was also playing with an idea of doing something similar to the
> > reclaim retry logic:
> > 	if (order) {
> > 		if (compaction_made_progress(compact_result)
> > 			no_compact_progress = 0;
> > 		else if (compaction_failed(compact_result)
> > 			no_compact_progress++;
> > 	}
> > but it is compaction_failed() part which is not really
> > straightforward to define. Is it COMPACT_NO_SUITABLE_PAGE
> > resp. COMPACT_NOT_SUITABLE_ZONE sufficient? compact_finished and
> > compaction_suitable however hide this from compaction users so it
> > seems like we can never see it.
> > 
> > Maybe we can update the feedback mechanism from the compaction but
> > retries count seems reasonably easy to understand and pragmatic. If
> > we cannot form a order page after we tried for N times then it really
> > doesn't make much sense to continue and we are oom for this order. I am
> > holding my breath to hear from Hugh on this, though.
> 
> Never a wise strategy.  But I just got around to it tonight.
> 
> I do believe you've nailed it with this patch!  Thank you!

That's a great news! Thanks for testing.

> I've applied 1/3, 2/3 and this (ah, it became the missing 3/3 later on)
> on top of 4.5.0-rc5-mm1 (I think there have been a couple of mmotms since,
> but I've not got to them yet): so far it is looking good on all machines.
> 
> After a quick go with the simple make -j20 in tmpfs, which survived
> a cycle on the laptop, I've switched back to my original tougher load,
> and that's going well so far: no sign of any OOMs.  But I've interrupted
> on the laptop to report back to you now, then I'll leave it running
> overnight.

OK, let's wait for the rest of the tests but I find it really optimistic
considering how easily you could trigger the issue previously. Anyway
I hope for your Tested-by after you are reasonably confident your loads
are behaving well.

[...]
> > diff --git a/include/linux/compaction.h b/include/linux/compaction.h
> > index b167801187e7..7d028ccf440a 100644
> > --- a/include/linux/compaction.h
> > +++ b/include/linux/compaction.h
> > @@ -61,6 +61,12 @@ extern void compaction_defer_reset(struct zone *zone, int order,
> >  				bool alloc_success);
> >  extern bool compaction_restarting(struct zone *zone, int order);
> >  
> > +static inline bool compaction_made_progress(enum compact_result result)
> > +{
> > +	return (compact_result > COMPACT_SKIPPED &&
> > +				compact_result < COMPACT_NO_SUITABLE_PAGE)
> 
> That line didn't build at all:
> 
>         return result > COMPACT_SKIPPED && result < COMPACT_NO_SUITABLE_PAGE;

those last minute changes... Sorry about that. Fixed.

Thanks!
-- 
Michal Hocko
SUSE Labs