On Tue, Oct 29, 2013 at 3:25 PM, Minchan Kim wrote: > On Tue, Oct 29, 2013 at 03:00:09PM +0800, Zhang Mingjun wrote: > > 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.orgwrote: > > > > 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. > > > It seems you misunderstood me. My English was poor? > sorry, it's my fault, my poor english. > I already said that I agree with you. > Your patch has no impact with hot path and makes CMA allocation policy > consistent so that there is no objection. > > > > > > > > > > > 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 > > Until now, I didn't notice there is such workload. Do you have such real > workload? > yes, my test platform using cma for video decoder, it alloc/free cma frequently. If so, we should change it with following as? > > if (migratetype >= MIGRATE_PCPTYPES) { > if (is_migrate_cma(migratetype) || > unlikely(is_migrate_isolate(migratetype))) > ok. > > Because assumption is you insist that there is lots of alloc/free for CMA. > But since we have had unlikely on memory-hotplug check, it would be less > than CMA. > > > > > > > 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 > > > > > -- > Kind regards, > Minchan Kim >