linux-kernel - Re: [patch for-4.7] mm, compaction: prevent VM_BUG

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: <20160706014109.GC23627@js1304-P5Q-DELUXE>
Date:	Wed, 6 Jul 2016 10:41:09 +0900
From:	Joonsoo Kim <iamjoonsoo.kim@....com>
To:	David Rientjes <rientjes@...gle.com>
Cc:	Vlastimil Babka <vbabka@...e.cz>,
	Andrew Morton <akpm@...ux-foundation.org>,
	linux-kernel@...r.kernel.org, linux-mm@...ck.org, hughd@...gle.com,
	mgorman@...hsingularity.net, minchan@...nel.org,
	stable@...r.kernel.org
Subject: Re: [patch for-4.7] mm, compaction: prevent VM_BUG_ON when
 terminating freeing scanner

On Tue, Jul 05, 2016 at 02:01:29PM -0700, David Rientjes wrote:
> On Thu, 30 Jun 2016, Vlastimil Babka wrote:
> 
> > >  Note: I really dislike the low watermark check in split_free_page() and
> > >  consider it poor software engineering.  The function should split a free
> > >  page, nothing more.  Terminating memory compaction because of a low
> > >  watermark check when we're simply trying to migrate memory seems like an
> > >  arbitrary heuristic.  There was an objection to removing it in the first
> > >  proposed patch, but I think we should really consider removing that
> > >  check so this is simpler.
> > 
> > There's a patch changing it to min watermark (you were CC'd on the series). We
> > could argue whether it belongs to split_free_page() or some wrapper of it, but
> > I don't think removing it completely should be done. If zone is struggling
> > with order-0 pages, a functionality for making higher-order pages shouldn't
> > make it even worse. It's also not that arbitrary, even if we succeeded the
> > migration and created a high-order page, the higher-order allocation would
> > still fail due to watermark checks. Worse, __compact_finished() would keep
> > telling the compaction to continue, creating an even longer lag, which is also
> > against your recent patches.
> > 
> 
> I'm suggesting we shouldn't check any zone watermark in split_free_page(): 
> that function should just split the free page.
> 
> I don't find our current watermark checks to determine if compaction is 
> worthwhile to be invalid, but I do think that we should avoid checking or 
> acting on any watermark in isolate_freepages() itself.  We could do more 
> effective checking in __compact_finished() to determine if we should 
> terminate compaction, but the freeing scanner feels like the wrong place 
> to do it -- it's also expensive to check while gathering free pages for 
> memory that we have already successfully isolated as part of the 
> iteration.
> 
> Do you have any objection to this fix for 4.7?
> 
> Joonson and/or Minchan, does this address the issue that you reported?

Unfortunately, I have no test case to trigger it. But, I think that
this patch will address it. Anyway, I commented one problem on this
patch in other e-mail so please fix it.

Thanks.