On Tue, Oct 29, 2013 at 12:54 PM, Minchan Kim <minchan@xxxxxxxxxx> wrote:
Hello,
You are saying about the overhead but I'm not sure how much it is
On Mon, Oct 28, 2013 at 07:42:49PM +0800, zhang.mingjun@xxxxxxxxxx wrote:
> From: Mingjun Zhang <troy.zhangmingjun@xxxxxxxxxx>
>
> 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.
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,
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.
>The concern is likely/unlikely usage is proper in this code peice.
> Signed-off-by: Mingjun Zhang <troy.zhangmingjun@xxxxxxxxxx>
> ---
> 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))
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@xxxxxxxxx. For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href="" href="mailto:dont@xxxxxxxxx">dont@xxxxxxxxx"> email@xxxxxxxxx </a>
--
Kind regards,
Minchan Kim