On Tue, Oct 29, 2013 at 12:54 PM, Minchan Kim wrote: > Hello, > > On Mon, Oct 28, 2013 at 07:42:49PM +0800, zhang.mingjun@linaro.org wrote: > > From: Mingjun Zhang > > > > free_contig_range frees cma pages one by one and MIGRATE_CMA pages will > be > > used as MIGRATE_MOVEABLE pages in the pcp list, it causes unnecessary > > migration action when these pages reused by CMA. > > You are saying about the overhead but I'm not sure how much it is > because it wouldn't be frequent. Although it's frequent, migration is > already slow path and CMA migration is worse so I really wonder how much > pain is and how much this patch improve. > > Having said that, it makes CMA allocation policy consistent which > is that CMA migration type is last fallback to minimize number of migration > and code peice you are adding is already low hit path so that I think > it has no problem. > problem is when free_contig_range frees cma pages, page's migration type is MIGRATE_CMA! I don't know why free_contig_range free pages one by one, but in the end it calls free_hot_cold_page, so some of these MIGRATE_CMA pages will be used as MIGRATE_MOVEABLE, this break the CMA allocation policy and it's not the low hit path, it's really the hot path, in fact each time free_contig_range calls some of these CMA pages will stay on this pcp list. when filesytem needs a pagecache or page fault exception which alloc one page using alloc_pages(MOVABLE, 0) it will get the page from this pcp list, breaking the CMA fallback rules, that is CMA pages in pcp list using as page cache or annoymous page very easily. > > > > Signed-off-by: Mingjun Zhang > > --- > > mm/page_alloc.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/mm/page_alloc.c b/mm/page_alloc.c > > index 0ee638f..84b9d84 100644 > > --- a/mm/page_alloc.c > > +++ b/mm/page_alloc.c > > @@ -1362,7 +1362,8 @@ void free_hot_cold_page(struct page *page, int > cold) > > * excessively into the page allocator > > */ > > if (migratetype >= MIGRATE_PCPTYPES) { > > - if (unlikely(is_migrate_isolate(migratetype))) { > > + if (unlikely(is_migrate_isolate(migratetype)) > > + || is_migrate_cma(migratetype)) > > The concern is likely/unlikely usage is proper in this code peice. > If we don't use memory isolation, the code path is used for only > MIGRATE_RESERVE which is very rare allocation in normal workload. > > Even, in memory isolation environement, I'm not sure how many > CMA/HOTPLUG is used compared to normal alloc/free. > So, I think below is more proper? > > if (unlikely(migratetype >= MIGRATE_PCPTYPES)) { > if (is_migrate_isolate(migratetype) || is_migrate_cma(migratetype)) > > if CMA is enabled and alloc/free frequently, it will more likely migratetype >= MIGRATE_PCPTYPES I know it's an another topic but I'd like to disucss it in this time because > we will forget such trivial thing later, again. > > } > > > free_one_page(zone, page, 0, migratetype); > > goto out; > > } > > -- > > 1.7.9.5 > > > > -- > > To unsubscribe, send a message with 'unsubscribe linux-mm' in > > the body to majordomo@kvack.org. For more info on Linux MM, > > see: http://www.linux-mm.org/ . > > Don't email: email@kvack.org > > -- > Kind regards, > Minchan Kim >