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]
Date:	Mon, 14 Jul 2014 13:37:14 -0700
From:	Andrew Morton <akpm@...ux-foundation.org>
To:	Joonsoo Kim <iamjoonsoo.kim@....com>
Cc:	Michal Nazarewicz <mina86@...a86.com>, Gioh Kim <gioh.kim@....com>,
	Laura Abbott <lauraa@...eaurora.org>,
	Marek Szyprowski <m.szyprowski@...sung.com>,
	Alexander Viro <viro@...iv.linux.org.uk>,
	linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
	이건호 <gunho.lee@....com>,
	Gi-Oh Kim <gurugio@...il.com>,
	Johannes Weiner <hannes@...xchg.org>,
	Mel Gorman <mel@....ul.ie>
Subject: Re: [PATCH] [RFC] CMA: clear buffer-head lru before page migration

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

> On Tue, Jul 08, 2014 at 06:46:31PM +0200, Michal Nazarewicz wrote:
> > On Mon, Jul 07 2014, Andrew Morton <akpm@...ux-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.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists