Re: [PATCH] [RFC] CMA: clear buffer-head lru before page migration

[Andrew Morton <akpm@linux-foundation.org>]

On Mon, 14 Jul 2014 16:02:25 +0900 Joonsoo Kim <iamjoonsoo.kim@lge.com> wrote:

> On Tue, Jul 08, 2014 at 06:46:31PM +0200, Michal Nazarewicz wrote:
> > On Mon, Jul 07 2014, Andrew Morton <akpm@linux-foundation.org> wrote:
> > > What I proposed is that CMA call invalidate_bh_lrus() right at the
> > > outset.  Something along the lines of
> > >
> > > --- a/mm/page_alloc.c~a
> > > +++ a/mm/page_alloc.c
> > > @@ -6329,6 +6329,14 @@ int alloc_contig_range(unsigned long sta
> > >  	};
> > >  	INIT_LIST_HEAD(&cc.migratepages);
> > >  
> > > +#ifdef CONFIG_CMA
> > > +	/*
> > > +	 * Comment goes here
> > > +	 */
> > > +	if (migratetype == MIGRATE_CMA)
> > > +		invalidate_bh_lrus();
> > > +#endif
> > > +
> > 
> > This seems reasonable, except I think it should go after
> > start_isolate_page_range call because otherwise there's no guarantee
> > that someone won't grab those pages back.
> > 
> > Also to avoid the #ifdef perhaps we want this as well:
> 
> I think that we just want to remove ifdef CONFIG_CMA on above code
> snippet, because invalidate_bh_lrus() would also help user of
> alloc_contig_range() with MIGRATE_MOVABLE.

That's what I believe also.  I pinged Mel and Johannes off-list and Mel
said "I hit it, the invalidation cost wasn't worth it for a THP alloc".

So hm.  I do think it's worth additional investigation but some careful
testing would be needed to demonstrate that it's worthwhile.  If the
invalidation cost is hurting then perhaps additional logic will be
needed to perform the invalidation only as a last-resort thing.