On Mon, 14 Jul 2014 16:02:25 +0900 Joonsoo Kim <iamjoonsoo.kim@xxxxxxx> wrote:Adding invalidate_bh_lrus() between start_isolate_page_range and
On Tue, Jul 08, 2014 at 06:46:31PM +0200, Michal Nazarewicz wrote:
On Mon, Jul 07 2014, Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> 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.