Minchan,

That sounds good to me. 

Thanks,
Wang Sheng-Hui


------------------ Original ------------------
From:  "Minchan Kim";<minchan@kernel.org>;
Date:  Wed, Jun 1, 2016 03:40 PM
To:  "Wang Sheng-Hui"<shhuiw@foxmail.com>; 
Cc:  "akpm"<akpm@linux-foundation.org>; "mgorman"<mgorman@techsingularity.net>; "iamjoonsoo.kim"<iamjoonsoo.kim@lge.com>; "linux-mm"<linux-mm@kvack.org>; "Vlastimil Babka"<vbabka@suse.cz>; 
Subject:  Re: Why __alloc_contig_migrate_range calls  migrate_prep() at first?



On Wed, Jun 01, 2016 at 11:42:29AM +0800, Wang Sheng-Hui wrote:
> Dear,
> 
> Sorry to trouble you.
> 
> I noticed cma_alloc would turn to  __alloc_contig_migrate_range for allocating pages.
> But  __alloc_contig_migrate_range calls  migrate_prep() at first, even if the requested page
> is single and free, lru_add_drain_all still run (called by  migrate_prep())?
> 
> Image a large chunk of free contig pages for CMA, various drivers may request a single page from
> the CMA area, we'll get  lru_add_drain_all run for each page.
> 
> Should we detect if the required pages are free before migrate_prep(), or detect at least for single 
> page allocation?

That makes sense to me.

How about calling migrate_prep once migrate_pages fails in the first trial?

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 9d666df5ef95..c504c1a623d2 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -6623,8 +6623,6 @@ static int __alloc_contig_migrate_range(struct compact_control *cc,
unsigned int tries = 0;
int ret = 0;

- migrate_prep();
-
while (pfn < end || !list_empty(&cc->migratepages)) {
if (fatal_signal_pending(current)) {
ret = -EINTR;
@@ -6650,6 +6648,8 @@ static int __alloc_contig_migrate_range(struct compact_control *cc,

ret = migrate_pages(&cc->migratepages, alloc_migrate_target,
 NULL, 0, cc->mode, MR_CMA);
+if (ret)
+ migrate_prep();
}
if (ret < 0) {
putback_movable_pages(&cc->migratepages);


> 
> ------------------
> Regards,
> Wang Sheng-HuiN‹§²æìr¸›zǧu©ž²Æ {­†éì¹»®&Þ–)îÆi¢žØ^n‡r¶‰šŽŠÝ¢j$½§$¢¸¢¹¨­è§~Š'.)îÄ