linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] mm: cma: free cma page to buddy instead of being cpu hot  page
@ 2013-10-28 11:42 zhang.mingjun
  2013-10-28 16:04 ` Laura Abbott
                   ` (2 more replies)
  0 siblings, 3 replies; 20+ messages in thread
From: zhang.mingjun @ 2013-10-28 11:42 UTC (permalink / raw)
  To: minchan, m.szyprowski, akpm, mgorman, haojian.zhuang, linux-mm,
	linux-kernel
  Cc: Mingjun Zhang

From: Mingjun Zhang <troy.zhangmingjun@linaro.org>

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.

Signed-off-by: Mingjun Zhang <troy.zhangmingjun@linaro.org>
---
 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))
 			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: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH] mm: cma: free cma page to buddy instead of being cpu hot  page
  2013-10-28 11:42 [PATCH] mm: cma: free cma page to buddy instead of being cpu hot page zhang.mingjun
@ 2013-10-28 16:04 ` Laura Abbott
  2013-10-29  4:54 ` Minchan Kim
  2013-10-29  9:33 ` Mel Gorman
  2 siblings, 0 replies; 20+ messages in thread
From: Laura Abbott @ 2013-10-28 16:04 UTC (permalink / raw)
  To: zhang.mingjun, minchan, m.szyprowski, akpm, mgorman,
	haojian.zhuang, linux-mm, linux-kernel
  Cc: Mingjun Zhang

On 10/28/2013 4:42 AM, zhang.mingjun@linaro.org wrote:
> From: Mingjun Zhang <troy.zhangmingjun@linaro.org>
>
> 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.
>
> Signed-off-by: Mingjun Zhang <troy.zhangmingjun@linaro.org>
> ---
>   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))
>   			free_one_page(zone, page, 0, migratetype);
>   			goto out;
>   		}
>


I submitted something very similar a while ago 
http://marc.info/?l=linaro-mm-sig&m=137645764208287&w=2 . Has the 
opinion on this patch changed?

Thanks,
Laura

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

--
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: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] mm: cma: free cma page to buddy instead of being cpu hot page
  2013-10-28 11:42 [PATCH] mm: cma: free cma page to buddy instead of being cpu hot page zhang.mingjun
  2013-10-28 16:04 ` Laura Abbott
@ 2013-10-29  4:54 ` Minchan Kim
  2013-10-29  6:41   ` KOSAKI Motohiro
  2013-10-29  7:00   ` Zhang Mingjun
  2013-10-29  9:33 ` Mel Gorman
  2 siblings, 2 replies; 20+ messages in thread
From: Minchan Kim @ 2013-10-29  4:54 UTC (permalink / raw)
  To: zhang.mingjun
  Cc: m.szyprowski, akpm, mgorman, haojian.zhuang, linux-mm,
	linux-kernel, Mingjun Zhang, KOSAKI Motohiro

Hello,

On Mon, Oct 28, 2013 at 07:42:49PM +0800, zhang.mingjun@linaro.org wrote:
> From: Mingjun Zhang <troy.zhangmingjun@linaro.org>
> 
> 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.

> 
> Signed-off-by: Mingjun Zhang <troy.zhangmingjun@linaro.org>
> ---
>  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))

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: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

-- 
Kind regards,
Minchan Kim

--
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: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] mm: cma: free cma page to buddy instead of being cpu hot page
  2013-10-29  4:54 ` Minchan Kim
@ 2013-10-29  6:41   ` KOSAKI Motohiro
  2013-10-29  7:00   ` Zhang Mingjun
  1 sibling, 0 replies; 20+ messages in thread
From: KOSAKI Motohiro @ 2013-10-29  6:41 UTC (permalink / raw)
  To: minchan, zhang.mingjun
  Cc: m.szyprowski, akpm, mgorman, haojian.zhuang, linux-mm,
	linux-kernel, troy.zhangmingjun

> 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))
> 
> 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.

I completely agree with this.


--
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: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] mm: cma: free cma page to buddy instead of being cpu hot page
  2013-10-29  4:54 ` Minchan Kim
  2013-10-29  6:41   ` KOSAKI Motohiro
@ 2013-10-29  7:00   ` Zhang Mingjun
  2013-10-29  7:25     ` Minchan Kim
  1 sibling, 1 reply; 20+ messages in thread
From: Zhang Mingjun @ 2013-10-29  7:00 UTC (permalink / raw)
  To: Minchan Kim
  Cc: Marek Szyprowski, akpm, Mel Gorman, Haojian Zhuang, linux-mm,
	linux-kernel, Mingjun Zhang, KOSAKI Motohiro

[-- Attachment #1: Type: text/plain, Size: 3488 bytes --]

On Tue, Oct 29, 2013 at 12:54 PM, Minchan Kim <minchan@kernel.org> wrote:

> Hello,
>
> On Mon, Oct 28, 2013 at 07:42:49PM +0800, zhang.mingjun@linaro.org wrote:
> > From: Mingjun Zhang <troy.zhangmingjun@linaro.org>
> >
> > 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 <troy.zhangmingjun@linaro.org>
> > ---
> >  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: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
>
> --
> Kind regards,
> Minchan Kim
>

[-- Attachment #2: Type: text/html, Size: 5048 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] mm: cma: free cma page to buddy instead of being cpu hot page
  2013-10-29  7:00   ` Zhang Mingjun
@ 2013-10-29  7:25     ` Minchan Kim
  2013-10-29 11:17       ` Zhang Mingjun
  0 siblings, 1 reply; 20+ messages in thread
From: Minchan Kim @ 2013-10-29  7:25 UTC (permalink / raw)
  To: Zhang Mingjun
  Cc: Marek Szyprowski, akpm, Mel Gorman, Haojian Zhuang, linux-mm,
	linux-kernel, Mingjun Zhang, KOSAKI Motohiro

On Tue, Oct 29, 2013 at 03:00:09PM +0800, Zhang Mingjun wrote:
> On Tue, Oct 29, 2013 at 12:54 PM, Minchan Kim <minchan@kernel.org> wrote:
> 
> > Hello,
> >
> > On Mon, Oct 28, 2013 at 07:42:49PM +0800, zhang.mingjun@linaro.org wrote:
> > > From: Mingjun Zhang <troy.zhangmingjun@linaro.org>
> > >
> > > 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?
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 <troy.zhangmingjun@linaro.org>
> > > ---
> > >  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?
If so, we should change it with following as?

if (migratetype >= MIGRATE_PCPTYPES) {
        if (is_migrate_cma(migratetype) || unlikely(is_migrate_isolate(migratetype)))

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: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
> >
> > --
> > Kind regards,
> > Minchan Kim
> >

-- 
Kind regards,
Minchan Kim

--
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: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] mm: cma: free cma page to buddy instead of being cpu hot page
  2013-10-28 11:42 [PATCH] mm: cma: free cma page to buddy instead of being cpu hot page zhang.mingjun
  2013-10-28 16:04 ` Laura Abbott
  2013-10-29  4:54 ` Minchan Kim
@ 2013-10-29  9:33 ` Mel Gorman
  2013-10-29 11:49   ` Zhang Mingjun
  2013-11-05 21:44   ` Andrew Morton
  2 siblings, 2 replies; 20+ messages in thread
From: Mel Gorman @ 2013-10-29  9:33 UTC (permalink / raw)
  To: zhang.mingjun
  Cc: minchan, m.szyprowski, akpm, haojian.zhuang, linux-mm,
	linux-kernel, Mingjun Zhang

On Mon, Oct 28, 2013 at 07:42:49PM +0800, zhang.mingjun@linaro.org wrote:
> From: Mingjun Zhang <troy.zhangmingjun@linaro.org>
> 
> 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.
> 
> Signed-off-by: Mingjun Zhang <troy.zhangmingjun@linaro.org>
> ---
>  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))
>  			free_one_page(zone, page, 0, migratetype);
>  			goto out;

This slightly impacts the page allocator free path for a marginal gain
on CMA which are relatively rare allocations. There is no obvious
benefit to this patch as I expect CMA allocations to flush the PCP lists
when a range of pages have been isolated and migrated. Is there any
measurable benefit to this patch?

-- 
Mel Gorman
SUSE Labs

--
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: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] mm: cma: free cma page to buddy instead of being cpu hot page
  2013-10-29  7:25     ` Minchan Kim
@ 2013-10-29 11:17       ` Zhang Mingjun
  0 siblings, 0 replies; 20+ messages in thread
From: Zhang Mingjun @ 2013-10-29 11:17 UTC (permalink / raw)
  To: Minchan Kim
  Cc: Marek Szyprowski, akpm, Mel Gorman, Haojian Zhuang, linux-mm,
	linux-kernel, Mingjun Zhang, KOSAKI Motohiro

[-- Attachment #1: Type: text/plain, Size: 4834 bytes --]

On Tue, Oct 29, 2013 at 3:25 PM, Minchan Kim <minchan@kernel.org> 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 <minchan@kernel.org>
> wrote:
> >
> > > Hello,
> > >
> > > On Mon, Oct 28, 2013 at 07:42:49PM +0800, zhang.mingjun@linaro.orgwrote:
> > > > From: Mingjun Zhang <troy.zhangmingjun@linaro.org>
> > > >
> > > > 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 <troy.zhangmingjun@linaro.org>
> > > > ---
> > > >  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: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
> > >
> > > --
> > > Kind regards,
> > > Minchan Kim
> > >
>
> --
> Kind regards,
> Minchan Kim
>

[-- Attachment #2: Type: text/html, Size: 7033 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] mm: cma: free cma page to buddy instead of being cpu hot page
  2013-10-29  9:33 ` Mel Gorman
@ 2013-10-29 11:49   ` Zhang Mingjun
  2013-10-29 12:27     ` Mel Gorman
  2013-11-05 21:44   ` Andrew Morton
  1 sibling, 1 reply; 20+ messages in thread
From: Zhang Mingjun @ 2013-10-29 11:49 UTC (permalink / raw)
  To: Mel Gorman
  Cc: Minchan Kim, Marek Szyprowski, akpm, Haojian Zhuang, linux-mm,
	linux-kernel, troy.zhangmingjun

[-- Attachment #1: Type: text/plain, Size: 1977 bytes --]

On Tue, Oct 29, 2013 at 5:33 PM, Mel Gorman <mgorman@suse.de> wrote:

> On Mon, Oct 28, 2013 at 07:42:49PM +0800, zhang.mingjun@linaro.org wrote:
> > From: Mingjun Zhang <troy.zhangmingjun@linaro.org>
> >
> > 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.
> >
> > Signed-off-by: Mingjun Zhang <troy.zhangmingjun@linaro.org>
> > ---
> >  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))
> >                       free_one_page(zone, page, 0, migratetype);
> >                       goto out;
>
> This slightly impacts the page allocator free path for a marginal gain
> on CMA which are relatively rare allocations. There is no obvious
> benefit to this patch as I expect CMA allocations to flush the PCP lists
>
how about keeping the migrate type of CMA page block as MIGRATE_ISOLATED
after
the alloc_contig_range , and undo_isolate_page_range at the end of
free_contig_range?
of course, it will waste the memory outside of the alloc range but in the
pageblocks.

> when a range of pages have been isolated and migrated. Is there any
> measurable benefit to this patch?
>
> after applying this patch, the video player on my platform works more
fluent,
and the driver of video decoder on my test platform using cma alloc/free
frequently.

> -- Mel Gorman
> SUSE Labs
>

[-- Attachment #2: Type: text/html, Size: 3110 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] mm: cma: free cma page to buddy instead of being cpu hot page
  2013-10-29 11:49   ` Zhang Mingjun
@ 2013-10-29 12:27     ` Mel Gorman
  2013-10-29 15:02       ` Zhang Mingjun
  0 siblings, 1 reply; 20+ messages in thread
From: Mel Gorman @ 2013-10-29 12:27 UTC (permalink / raw)
  To: Zhang Mingjun
  Cc: Minchan Kim, Marek Szyprowski, akpm, Haojian Zhuang, linux-mm,
	linux-kernel, troy.zhangmingjun

On Tue, Oct 29, 2013 at 07:49:30PM +0800, Zhang Mingjun wrote:
> On Tue, Oct 29, 2013 at 5:33 PM, Mel Gorman <mgorman@suse.de> wrote:
> 
> > On Mon, Oct 28, 2013 at 07:42:49PM +0800, zhang.mingjun@linaro.org wrote:
> > > From: Mingjun Zhang <troy.zhangmingjun@linaro.org>
> > >
> > > 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.
> > >
> > > Signed-off-by: Mingjun Zhang <troy.zhangmingjun@linaro.org>
> > > ---
> > >  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))
> > >                       free_one_page(zone, page, 0, migratetype);
> > >                       goto out;
> >
> > This slightly impacts the page allocator free path for a marginal gain
> > on CMA which are relatively rare allocations. There is no obvious
> > benefit to this patch as I expect CMA allocations to flush the PCP lists
> >
> how about keeping the migrate type of CMA page block as MIGRATE_ISOLATED
> after
> the alloc_contig_range , and undo_isolate_page_range at the end of
> free_contig_range?

It would move the cost to the CMA paths so I would complain less. Bear
in mind as well that forcing everything to go through free_one_page()
means that every free goes through the zone lock. I doubt you have any
machine large enough but it is possible for simultaneous CMA allocations
to now contend on the zone lock that would have been previously fine.
Hence, I'm interesting in knowing the underlying cause of the problem you
are experiencing.

> of course, it will waste the memory outside of the alloc range but in the
> pageblocks.
> 

I would hope/expect that the loss would only last for the duration of
the allocation attempt and a small amount of memory.

> > when a range of pages have been isolated and migrated. Is there any
> > measurable benefit to this patch?
> >
> after applying this patch, the video player on my platform works more
> fluent,

fluent almost always refers to ones command of a spoken language. I do
not see how a video player can be fluent in anything. What is measurably
better?

For example, are allocations faster? If so, why? What cost from another
path is removed as a result of this patch? If the cost is in the PCP
flush then can it be checked if the PCP flush was unnecessary and called
unconditionally even though all the pages were freed already? We had
problems in the past where drain_all_pages() or similar were called
unnecessarily causing long sync stalls related to IPIs. I'm wondering if
we are seeing a similar problem here.

Maybe the problem is the complete opposite. Are allocations failing
because there are PCP pages in the way? In that case, it real fix might
be to insert a PCP flush if the allocation is failing due to per-cpu
pages.

> and the driver of video decoder on my test platform using cma alloc/free
> frequently.
> 

CMA allocations are almost never used outside of these contexts. While I
appreciate that embedded use is important I'm reluctant to see an impact
in fast paths unless there is a good reason for every other use case. I
also am a bit unhappy to see CMA allocations making the zone->lock
hotter than necessary even if no embedded use case it likely to
experience the problem in the short-term.

-- 
Mel Gorman
SUSE Labs

--
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: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] mm: cma: free cma page to buddy instead of being cpu hot page
  2013-10-29 12:27     ` Mel Gorman
@ 2013-10-29 15:02       ` Zhang Mingjun
  2013-10-29 16:14         ` Laura Abbott
  2013-10-30  2:55         ` Minchan Kim
  0 siblings, 2 replies; 20+ messages in thread
From: Zhang Mingjun @ 2013-10-29 15:02 UTC (permalink / raw)
  To: Mel Gorman
  Cc: Minchan Kim, Marek Szyprowski, akpm, Haojian Zhuang, linux-mm,
	linux-kernel, troy.zhangmingjun

[-- Attachment #1: Type: text/plain, Size: 4575 bytes --]

On Tue, Oct 29, 2013 at 8:27 PM, Mel Gorman <mgorman@suse.de> wrote:

> On Tue, Oct 29, 2013 at 07:49:30PM +0800, Zhang Mingjun wrote:
> > On Tue, Oct 29, 2013 at 5:33 PM, Mel Gorman <mgorman@suse.de> wrote:
> >
> > > On Mon, Oct 28, 2013 at 07:42:49PM +0800, zhang.mingjun@linaro.orgwrote:
> > > > From: Mingjun Zhang <troy.zhangmingjun@linaro.org>
> > > >
> > > > 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.
> > > >
> > > > Signed-off-by: Mingjun Zhang <troy.zhangmingjun@linaro.org>
> > > > ---
> > > >  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))
> > > >                       free_one_page(zone, page, 0, migratetype);
> > > >                       goto out;
> > >
> > > This slightly impacts the page allocator free path for a marginal gain
> > > on CMA which are relatively rare allocations. There is no obvious
> > > benefit to this patch as I expect CMA allocations to flush the PCP
> lists
> > >
> > how about keeping the migrate type of CMA page block as MIGRATE_ISOLATED
> > after
> > the alloc_contig_range , and undo_isolate_page_range at the end of
> > free_contig_range?
>
> It would move the cost to the CMA paths so I would complain less. Bear
> in mind as well that forcing everything to go through free_one_page()
> means that every free goes through the zone lock. I doubt you have any
> machine large enough but it is possible for simultaneous CMA allocations
> to now contend on the zone lock that would have been previously fine.
> Hence, I'm interesting in knowing the underlying cause of the problem you
> are experiencing.
>
> my platform uses CMA but disabled CMA's migration func by del MIGRATE_CMA
in fallbacks[MIGRATE_MOVEABLE]. But I find CMA pages can still used by
pagecache or page fault page request from PCP list and cma allocation has to
migrate these page. So I want to free these cma pages to buddy directly not
PCP..

> of course, it will waste the memory outside of the alloc range but in the
> > pageblocks.
> >
>
> I would hope/expect that the loss would only last for the duration of
> the allocation attempt and a small amount of memory.
>
> > > when a range of pages have been isolated and migrated. Is there any
> > > measurable benefit to this patch?
> > >
> > after applying this patch, the video player on my platform works more
> > fluent,
>
> fluent almost always refers to ones command of a spoken language. I do
> not see how a video player can be fluent in anything. What is measurably
> better?
>
> For example, are allocations faster? If so, why? What cost from another
> path is removed as a result of this patch? If the cost is in the PCP
> flush then can it be checked if the PCP flush was unnecessary and called
> unconditionally even though all the pages were freed already? We had
> problems in the past where drain_all_pages() or similar were called
> unnecessarily causing long sync stalls related to IPIs. I'm wondering if
> we are seeing a similar problem here.
>
> Maybe the problem is the complete opposite. Are allocations failing
> because there are PCP pages in the way? In that case, it real fix might
> be to insert a  if the allocation is failing due to per-cpu
> pages.
>
problem is not the allocation failing, but the unexpected cma migration
slows
down the allocation.

>
> > and the driver of video decoder on my test platform using cma alloc/free
> > frequently.
> >
>
> CMA allocations are almost never used outside of these contexts. While I
> appreciate that embedded use is important I'm reluctant to see an impact
> in fast paths unless there is a good reason for every other use case. I
> also am a bit unhappy to see CMA allocations making the zone->lock
> hotter than necessary even if no embedded use case it likely to
> experience the problem in the short-term.
>
> --
> Mel Gorman
> SUSE Labs
>

[-- Attachment #2: Type: text/html, Size: 6128 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] mm: cma: free cma page to buddy instead of being cpu hot page
  2013-10-29 15:02       ` Zhang Mingjun
@ 2013-10-29 16:14         ` Laura Abbott
  2013-10-30  5:40           ` Minchan Kim
  2013-10-30  2:55         ` Minchan Kim
  1 sibling, 1 reply; 20+ messages in thread
From: Laura Abbott @ 2013-10-29 16:14 UTC (permalink / raw)
  To: Zhang Mingjun, Mel Gorman
  Cc: Minchan Kim, Marek Szyprowski, akpm, Haojian Zhuang, linux-mm,
	linux-kernel, troy.zhangmingjun

On 10/29/2013 8:02 AM, Zhang Mingjun wrote:

>     It would move the cost to the CMA paths so I would complain less. Bear
>     in mind as well that forcing everything to go through free_one_page()
>     means that every free goes through the zone lock. I doubt you have any
>     machine large enough but it is possible for simultaneous CMA allocations
>     to now contend on the zone lock that would have been previously fine.
>     Hence, I'm interesting in knowing the underlying cause of the
>     problem you
>     are experiencing.
>
> my platform uses CMA but disabled CMA's migration func by del MIGRATE_CMA
> in fallbacks[MIGRATE_MOVEABLE]. But I find CMA pages can still used by
> pagecache or page fault page request from PCP list and cma allocation has to
> migrate these page. So I want to free these cma pages to buddy directly
> not PCP..
>
>      > of course, it will waste the memory outside of the alloc range
>     but in the
>      > pageblocks.
>      >
>
>     I would hope/expect that the loss would only last for the duration of
>     the allocation attempt and a small amount of memory.
>
>      > > when a range of pages have been isolated and migrated. Is there any
>      > > measurable benefit to this patch?
>      > >
>      > after applying this patch, the video player on my platform works more
>      > fluent,
>
>     fluent almost always refers to ones command of a spoken language. I do
>     not see how a video player can be fluent in anything. What is measurably
>     better?
>
>     For example, are allocations faster? If so, why? What cost from another
>     path is removed as a result of this patch? If the cost is in the PCP
>     flush then can it be checked if the PCP flush was unnecessary and called
>     unconditionally even though all the pages were freed already? We had
>     problems in the past where drain_all_pages() or similar were called
>     unnecessarily causing long sync stalls related to IPIs. I'm wondering if
>     we are seeing a similar problem here.
>
>     Maybe the problem is the complete opposite. Are allocations failing
>     because there are PCP pages in the way? In that case, it real fix might
>     be to insert a  if the allocation is failing due to per-cpu
>     pages.
>
> problem is not the allocation failing, but the unexpected cma migration
> slows
> down the allocation.
>
>
>      > and the driver of video decoder on my test platform using cma
>     alloc/free
>      > frequently.
>      >
>
>     CMA allocations are almost never used outside of these contexts. While I
>     appreciate that embedded use is important I'm reluctant to see an impact
>     in fast paths unless there is a good reason for every other use case. I
>     also am a bit unhappy to see CMA allocations making the zone->lock
>     hotter than necessary even if no embedded use case it likely to
>     experience the problem in the short-term.
>
>     --
>     Mel Gorman
>     SUSE Labs
>
>

We've had a similar patch in our tree for a year and a half because of 
CMA migration failures, not just for a speedup in allocation time. I 
understand that CMA is not the fast case or the general use case but the 
problem is that the cost of CMA failure is very high (complete failure 
of the feature using CMA). Putting CMA on the PCP lists means they may 
be picked up by users who temporarily make the movable pages unmovable 
(page cache etc.) which prevents the allocation from succeeding. The 
problem still exists even if the CMA pages are not on the PCP list but 
the window gets slightly smaller.

This really highlights one of the biggest issues with CMA today. Movable 
pages make return -EBUSY for any number of reasons. For non-CMA pages 
this is mostly fine, another movable page may be substituted for the 
movable page that is busy. CMA is a restricted range though so any 
failure in that range is very costly because CMA regions are generally 
sized exactly for the use cases at hand which means there is very little 
extra space for retries.

To make CMA actually usable, we've had to go through and add in 
hacks/quirks that prevent CMA from being allocated in any path which may 
prevent migration. I've been mixed on if this is the right path or if 
the definition of MIGRATE_CMA needs to be changed to be more restrictive 
(can't prevent migration).

Thanks,
Laura
-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

--
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: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] mm: cma: free cma page to buddy instead of being cpu hot page
  2013-10-29 15:02       ` Zhang Mingjun
  2013-10-29 16:14         ` Laura Abbott
@ 2013-10-30  2:55         ` Minchan Kim
  1 sibling, 0 replies; 20+ messages in thread
From: Minchan Kim @ 2013-10-30  2:55 UTC (permalink / raw)
  To: Zhang Mingjun
  Cc: Mel Gorman, Marek Szyprowski, akpm, Haojian Zhuang, linux-mm,
	linux-kernel, troy.zhangmingjun

Hello,

On Tue, Oct 29, 2013 at 11:02:30PM +0800, Zhang Mingjun wrote:
> On Tue, Oct 29, 2013 at 8:27 PM, Mel Gorman <mgorman@suse.de> wrote:
> 
> > On Tue, Oct 29, 2013 at 07:49:30PM +0800, Zhang Mingjun wrote:
> > > On Tue, Oct 29, 2013 at 5:33 PM, Mel Gorman <mgorman@suse.de> wrote:
> > >
> > > > On Mon, Oct 28, 2013 at 07:42:49PM +0800, zhang.mingjun@linaro.orgwrote:
> > > > > From: Mingjun Zhang <troy.zhangmingjun@linaro.org>
> > > > >
> > > > > 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.
> > > > >
> > > > > Signed-off-by: Mingjun Zhang <troy.zhangmingjun@linaro.org>
> > > > > ---
> > > > >  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))
> > > > >                       free_one_page(zone, page, 0, migratetype);
> > > > >                       goto out;
> > > >
> > > > This slightly impacts the page allocator free path for a marginal gain
> > > > on CMA which are relatively rare allocations. There is no obvious
> > > > benefit to this patch as I expect CMA allocations to flush the PCP
> > lists
> > > >
> > > how about keeping the migrate type of CMA page block as MIGRATE_ISOLATED
> > > after
> > > the alloc_contig_range , and undo_isolate_page_range at the end of
> > > free_contig_range?
> >
> > It would move the cost to the CMA paths so I would complain less. Bear
> > in mind as well that forcing everything to go through free_one_page()
> > means that every free goes through the zone lock. I doubt you have any
> > machine large enough but it is possible for simultaneous CMA allocations
> > to now contend on the zone lock that would have been previously fine.
> > Hence, I'm interesting in knowing the underlying cause of the problem you
> > are experiencing.
> >
> > my platform uses CMA but disabled CMA's migration func by del MIGRATE_CMA
> in fallbacks[MIGRATE_MOVEABLE]. But I find CMA pages can still used by

In that case, why do you want to use CMA?
It's almost same with resreved memory.

> pagecache or page fault page request from PCP list and cma allocation has to
> migrate these page. So I want to free these cma pages to buddy directly not
> PCP..

I know your goal and understand current problem it could make more number
of migration but how often it happens so that if we apply your patch,
how much is the gain? For example, you can get a number followin as.

1. old

getting XXXM contiguos memory area: 100ms
the number of migration : 200

2. new

getting XXXM contiguos memory area: 10ms
the number of migration : 0

It seems Mel want it and I'd like to see it to convince.
Of course, Andrew might want it, too.

> 
> > of course, it will waste the memory outside of the alloc range but in the
> > > pageblocks.
> > >

We need to know fundamental problem and number before you go with any method
so that we could judge if you approach is good.
Please add more detail explanation about current status in description.
It would be better to include more statistic data.

> >
> > I would hope/expect that the loss would only last for the duration of
> > the allocation attempt and a small amount of memory.
> >
> > > > when a range of pages have been isolated and migrated. Is there any
> > > > measurable benefit to this patch?
> > > >
> > > after applying this patch, the video player on my platform works more
> > > fluent,
> >
> > fluent almost always refers to ones command of a spoken language. I do
> > not see how a video player can be fluent in anything. What is measurably
> > better?
> >
> > For example, are allocations faster? If so, why? What cost from another
> > path is removed as a result of this patch? If the cost is in the PCP
> > flush then can it be checked if the PCP flush was unnecessary and called
> > unconditionally even though all the pages were freed already? We had
> > problems in the past where drain_all_pages() or similar were called
> > unnecessarily causing long sync stalls related to IPIs. I'm wondering if
> > we are seeing a similar problem here.
> >
> > Maybe the problem is the complete opposite. Are allocations failing
> > because there are PCP pages in the way? In that case, it real fix might
> > be to insert a  if the allocation is failing due to per-cpu
> > pages.
> >
> problem is not the allocation failing, but the unexpected cma migration
> slows
> down the allocation.

Okay, So how many? Need number. # of migration and time due to it.

> 
> >
> > > and the driver of video decoder on my test platform using cma alloc/free
> > > frequently.
> > >
> >
> > CMA allocations are almost never used outside of these contexts. While I
> > appreciate that embedded use is important I'm reluctant to see an impact
> > in fast paths unless there is a good reason for every other use case. I
> > also am a bit unhappy to see CMA allocations making the zone->lock
> > hotter than necessary even if no embedded use case it likely to
> > experience the problem in the short-term.
> >
> > --
> > Mel Gorman
> > SUSE Labs
> >

-- 
Kind regards,
Minchan Kim

--
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: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] mm: cma: free cma page to buddy instead of being cpu hot page
  2013-10-29 16:14         ` Laura Abbott
@ 2013-10-30  5:40           ` Minchan Kim
  2013-10-31  2:14             ` Laura Abbott
  2013-12-23 12:38             ` Wanpeng Li
  0 siblings, 2 replies; 20+ messages in thread
From: Minchan Kim @ 2013-10-30  5:40 UTC (permalink / raw)
  To: Laura Abbott
  Cc: Zhang Mingjun, Mel Gorman, Marek Szyprowski, akpm,
	Haojian Zhuang, linux-mm, linux-kernel, troy.zhangmingjun

Hello,

On Tue, Oct 29, 2013 at 09:14:03AM -0700, Laura Abbott wrote:
> On 10/29/2013 8:02 AM, Zhang Mingjun wrote:
> 
> >    It would move the cost to the CMA paths so I would complain less. Bear
> >    in mind as well that forcing everything to go through free_one_page()
> >    means that every free goes through the zone lock. I doubt you have any
> >    machine large enough but it is possible for simultaneous CMA allocations
> >    to now contend on the zone lock that would have been previously fine.
> >    Hence, I'm interesting in knowing the underlying cause of the
> >    problem you
> >    are experiencing.
> >
> >my platform uses CMA but disabled CMA's migration func by del MIGRATE_CMA
> >in fallbacks[MIGRATE_MOVEABLE]. But I find CMA pages can still used by
> >pagecache or page fault page request from PCP list and cma allocation has to
> >migrate these page. So I want to free these cma pages to buddy directly
> >not PCP..
> >
> >     > of course, it will waste the memory outside of the alloc range
> >    but in the
> >     > pageblocks.
> >     >
> >
> >    I would hope/expect that the loss would only last for the duration of
> >    the allocation attempt and a small amount of memory.
> >
> >     > > when a range of pages have been isolated and migrated. Is there any
> >     > > measurable benefit to this patch?
> >     > >
> >     > after applying this patch, the video player on my platform works more
> >     > fluent,
> >
> >    fluent almost always refers to ones command of a spoken language. I do
> >    not see how a video player can be fluent in anything. What is measurably
> >    better?
> >
> >    For example, are allocations faster? If so, why? What cost from another
> >    path is removed as a result of this patch? If the cost is in the PCP
> >    flush then can it be checked if the PCP flush was unnecessary and called
> >    unconditionally even though all the pages were freed already? We had
> >    problems in the past where drain_all_pages() or similar were called
> >    unnecessarily causing long sync stalls related to IPIs. I'm wondering if
> >    we are seeing a similar problem here.
> >
> >    Maybe the problem is the complete opposite. Are allocations failing
> >    because there are PCP pages in the way? In that case, it real fix might
> >    be to insert a  if the allocation is failing due to per-cpu
> >    pages.
> >
> >problem is not the allocation failing, but the unexpected cma migration
> >slows
> >down the allocation.
> >
> >
> >     > and the driver of video decoder on my test platform using cma
> >    alloc/free
> >     > frequently.
> >     >
> >
> >    CMA allocations are almost never used outside of these contexts. While I
> >    appreciate that embedded use is important I'm reluctant to see an impact
> >    in fast paths unless there is a good reason for every other use case. I
> >    also am a bit unhappy to see CMA allocations making the zone->lock
> >    hotter than necessary even if no embedded use case it likely to
> >    experience the problem in the short-term.
> >
> >    --
> >    Mel Gorman
> >    SUSE Labs
> >
> >
> 
> We've had a similar patch in our tree for a year and a half because
> of CMA migration failures, not just for a speedup in allocation
> time. I understand that CMA is not the fast case or the general use
> case but the problem is that the cost of CMA failure is very high
> (complete failure of the feature using CMA). Putting CMA on the PCP
> lists means they may be picked up by users who temporarily make the
> movable pages unmovable (page cache etc.) which prevents the
> allocation from succeeding. The problem still exists even if the CMA
> pages are not on the PCP list but the window gets slightly smaller.

I understand that I have seen many people want to use CMA have tweaked
their system to work well and although they do best effort, it doesn't
work well because CMA doesn't gaurantee to succeed in getting free
space since there are lots of hurdle. (get_user_pages, AIO ring buffer,
buffer cache, short of free memory for migration, no swap and so on).
Even, someone want to allocate CMA space with speedy. SIGH.

Yeah, at the moment, CMA is really SUCK.

> 
> This really highlights one of the biggest issues with CMA today.
> Movable pages make return -EBUSY for any number of reasons. For
> non-CMA pages this is mostly fine, another movable page may be
> substituted for the movable page that is busy. CMA is a restricted
> range though so any failure in that range is very costly because CMA
> regions are generally sized exactly for the use cases at hand which
> means there is very little extra space for retries.
> 
> To make CMA actually usable, we've had to go through and add in
> hacks/quirks that prevent CMA from being allocated in any path which
> may prevent migration. I've been mixed on if this is the right path
> or if the definition of MIGRATE_CMA needs to be changed to be more
> restrictive (can't prevent migration).

Fundamental problem is that every subsystem could grab a page anytime
and they doesn't gaurantee to release it soonish or within time CMA
user want so it turns out non-determisitic mess which just hook into
core MM system here and there.

Sometime, I see some people try to solve it case by case with ad-hoc
approach. I guess it would be never ending story as kernel evolves.

I suggest that we could make new wheel with frontswap/cleancache stuff.
The idea is that pages in frontswap/cleancache are evicted from kernel
POV so that we can gaurantee that there is no chance to grab a page
in CMA area and we could remove lots of hook from core MM which just
complicated MM without benefit.

As benefit, cleancache pages could drop easily so it would be fast
to get free space but frontswap cache pages should be move into somewhere.
If there are enough free pages, it could be migrated out there. Optionally
we could compress them. Otherwise, we could pageout them into backed device.
Yeah, it could be slow than migration but at least, we could estimate the time
by storage speed ideally so we could have tunable knob. If someone want
fast CMA, he could control it with ratio of cleancache:frontswap.
IOW, higher frontswap page ratio is, slower the speed would be.
Important thing is admin could have tuned control knob and it gaurantees to
get CMA free space with deterministic time.

As drawback, if we fail to tune the ratio, memeory efficieny would be
bad so that it ends up thrashing but you guys is saying we have been
used CMA without movable fallback which means that it's already static
reserved memory and it's never CMA so you already have lost memory
efficiency and even fail to get a space so I think it's good trade-off
for embedded people.

If anyone has interest the idea, I will move into that.
If it sounds crazy idea, feel free to ignore, please.

Thanks.

> 
> Thanks,
> Laura
> -- 
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> hosted by The Linux Foundation
> 
> --
> 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: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

-- 
Kind regards,
Minchan Kim

--
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: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] mm: cma: free cma page to buddy instead of being cpu hot page
  2013-10-30  5:40           ` Minchan Kim
@ 2013-10-31  2:14             ` Laura Abbott
  2013-11-01  1:49               ` Minchan Kim
  2013-12-23 12:38             ` Wanpeng Li
  1 sibling, 1 reply; 20+ messages in thread
From: Laura Abbott @ 2013-10-31  2:14 UTC (permalink / raw)
  To: Minchan Kim
  Cc: Zhang Mingjun, Mel Gorman, Marek Szyprowski, akpm,
	Haojian Zhuang, linux-mm, linux-kernel, troy.zhangmingjun

On 10/29/2013 10:40 PM, Minchan Kim wrote:
>>
>> We've had a similar patch in our tree for a year and a half because
>> of CMA migration failures, not just for a speedup in allocation
>> time. I understand that CMA is not the fast case or the general use
>> case but the problem is that the cost of CMA failure is very high
>> (complete failure of the feature using CMA). Putting CMA on the PCP
>> lists means they may be picked up by users who temporarily make the
>> movable pages unmovable (page cache etc.) which prevents the
>> allocation from succeeding. The problem still exists even if the CMA
>> pages are not on the PCP list but the window gets slightly smaller.
>
> I understand that I have seen many people want to use CMA have tweaked
> their system to work well and although they do best effort, it doesn't
> work well because CMA doesn't gaurantee to succeed in getting free
> space since there are lots of hurdle. (get_user_pages, AIO ring buffer,
> buffer cache, short of free memory for migration, no swap and so on).
> Even, someone want to allocate CMA space with speedy. SIGH.
>
> Yeah, at the moment, CMA is really SUCK.
>

Yes, without hacks there are enough issues where it makes more sense to 
turn off CMA in some cases.

>>
>> This really highlights one of the biggest issues with CMA today.
>> Movable pages make return -EBUSY for any number of reasons. For
>> non-CMA pages this is mostly fine, another movable page may be
>> substituted for the movable page that is busy. CMA is a restricted
>> range though so any failure in that range is very costly because CMA
>> regions are generally sized exactly for the use cases at hand which
>> means there is very little extra space for retries.
>>
>> To make CMA actually usable, we've had to go through and add in
>> hacks/quirks that prevent CMA from being allocated in any path which
>> may prevent migration. I've been mixed on if this is the right path
>> or if the definition of MIGRATE_CMA needs to be changed to be more
>> restrictive (can't prevent migration).
>
> Fundamental problem is that every subsystem could grab a page anytime
> and they doesn't gaurantee to release it soonish or within time CMA
> user want so it turns out non-determisitic mess which just hook into
> core MM system here and there.
>
> Sometime, I see some people try to solve it case by case with ad-hoc
> approach. I guess it would be never ending story as kernel evolves.
>
> I suggest that we could make new wheel with frontswap/cleancache stuff.
> The idea is that pages in frontswap/cleancache are evicted from kernel
> POV so that we can gaurantee that there is no chance to grab a page
> in CMA area and we could remove lots of hook from core MM which just
> complicated MM without benefit.
>
> As benefit, cleancache pages could drop easily so it would be fast
> to get free space but frontswap cache pages should be move into somewhere.
> If there are enough free pages, it could be migrated out there. Optionally
> we could compress them. Otherwise, we could pageout them into backed device.
> Yeah, it could be slow than migration but at least, we could estimate the time
> by storage speed ideally so we could have tunable knob. If someone want
> fast CMA, he could control it with ratio of cleancache:frontswap.
> IOW, higher frontswap page ratio is, slower the speed would be.
> Important thing is admin could have tuned control knob and it gaurantees to
> get CMA free space with deterministic time.
>

Before CMA was available, we attempted to do something similar with 
carved out memory where we hooked up the carveout to tmem and 
cleancache. The feature never really went anywhere because we saw 
impacts on file system benchmarks (too much time copying data to 
carveout memory). The feature has long been deprecated and we never 
debugged too far. Admittedly, this was many kernel versions ago and with 
a backport of various patches to an older kernel so I'd take our results 
with a grain of salt.

> As drawback, if we fail to tune the ratio, memeory efficieny would be
> bad so that it ends up thrashing but you guys is saying we have been
> used CMA without movable fallback which means that it's already static
> reserved memory and it's never CMA so you already have lost memory
> efficiency and even fail to get a space so I think it's good trade-off
> for embedded people.
>

Agreed. It really should be a policy decision how much effort to put 
into getting CMA pages. There isn't a nice way to do this now.

> If anyone has interest the idea, I will move into that.
> If it sounds crazy idea, feel free to ignore, please.
>

I'm interested in the idea with the warning noted above.

Thanks,
Laura

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

--
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: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] mm: cma: free cma page to buddy instead of being cpu hot page
  2013-10-31  2:14             ` Laura Abbott
@ 2013-11-01  1:49               ` Minchan Kim
  0 siblings, 0 replies; 20+ messages in thread
From: Minchan Kim @ 2013-11-01  1:49 UTC (permalink / raw)
  To: Laura Abbott
  Cc: Zhang Mingjun, Mel Gorman, Marek Szyprowski, akpm,
	Haojian Zhuang, linux-mm, linux-kernel, troy.zhangmingjun

Hello,

On Wed, Oct 30, 2013 at 07:14:45PM -0700, Laura Abbott wrote:
> On 10/29/2013 10:40 PM, Minchan Kim wrote:
> >>
> >>We've had a similar patch in our tree for a year and a half because
> >>of CMA migration failures, not just for a speedup in allocation
> >>time. I understand that CMA is not the fast case or the general use
> >>case but the problem is that the cost of CMA failure is very high
> >>(complete failure of the feature using CMA). Putting CMA on the PCP
> >>lists means they may be picked up by users who temporarily make the
> >>movable pages unmovable (page cache etc.) which prevents the
> >>allocation from succeeding. The problem still exists even if the CMA
> >>pages are not on the PCP list but the window gets slightly smaller.
> >
> >I understand that I have seen many people want to use CMA have tweaked
> >their system to work well and although they do best effort, it doesn't
> >work well because CMA doesn't gaurantee to succeed in getting free
> >space since there are lots of hurdle. (get_user_pages, AIO ring buffer,
> >buffer cache, short of free memory for migration, no swap and so on).
> >Even, someone want to allocate CMA space with speedy. SIGH.
> >
> >Yeah, at the moment, CMA is really SUCK.
> >
> 
> Yes, without hacks there are enough issues where it makes more sense
> to turn off CMA in some cases.
> 
> >>
> >>This really highlights one of the biggest issues with CMA today.
> >>Movable pages make return -EBUSY for any number of reasons. For
> >>non-CMA pages this is mostly fine, another movable page may be
> >>substituted for the movable page that is busy. CMA is a restricted
> >>range though so any failure in that range is very costly because CMA
> >>regions are generally sized exactly for the use cases at hand which
> >>means there is very little extra space for retries.
> >>
> >>To make CMA actually usable, we've had to go through and add in
> >>hacks/quirks that prevent CMA from being allocated in any path which
> >>may prevent migration. I've been mixed on if this is the right path
> >>or if the definition of MIGRATE_CMA needs to be changed to be more
> >>restrictive (can't prevent migration).
> >
> >Fundamental problem is that every subsystem could grab a page anytime
> >and they doesn't gaurantee to release it soonish or within time CMA
> >user want so it turns out non-determisitic mess which just hook into
> >core MM system here and there.
> >
> >Sometime, I see some people try to solve it case by case with ad-hoc
> >approach. I guess it would be never ending story as kernel evolves.
> >
> >I suggest that we could make new wheel with frontswap/cleancache stuff.
> >The idea is that pages in frontswap/cleancache are evicted from kernel
> >POV so that we can gaurantee that there is no chance to grab a page
> >in CMA area and we could remove lots of hook from core MM which just
> >complicated MM without benefit.
> >
> >As benefit, cleancache pages could drop easily so it would be fast
> >to get free space but frontswap cache pages should be move into somewhere.
> >If there are enough free pages, it could be migrated out there. Optionally
> >we could compress them. Otherwise, we could pageout them into backed device.
> >Yeah, it could be slow than migration but at least, we could estimate the time
> >by storage speed ideally so we could have tunable knob. If someone want
> >fast CMA, he could control it with ratio of cleancache:frontswap.
> >IOW, higher frontswap page ratio is, slower the speed would be.
> >Important thing is admin could have tuned control knob and it gaurantees to
> >get CMA free space with deterministic time.
> >
> 
> Before CMA was available, we attempted to do something similar with
> carved out memory where we hooked up the carveout to tmem and
> cleancache. The feature never really went anywhere because we saw
> impacts on file system benchmarks (too much time copying data to
> carveout memory). The feature has long been deprecated and we never
> debugged too far. Admittedly, this was many kernel versions ago and
> with a backport of various patches to an older kernel so I'd take
> our results with a grain of salt.

Did you use only cleancache?
If so, I guess the problem is from cleancache which could store used-once
pages easily due to lack of function to identify such pages so that copy
from frontend to backend would be totally overhead.
AFAIR, there was some patch to remove such overhead from Dan who is author.
He wanted to detect it by seeing evicted page was LRU active list of VM.
https://lkml.org/lkml/2012/1/25/300
Maybe, we could enhance that part.

> 
> >As drawback, if we fail to tune the ratio, memeory efficieny would be
> >bad so that it ends up thrashing but you guys is saying we have been
> >used CMA without movable fallback which means that it's already static
> >reserved memory and it's never CMA so you already have lost memory
> >efficiency and even fail to get a space so I think it's good trade-off
> >for embedded people.
> >
> 
> Agreed. It really should be a policy decision how much effort to put
> into getting CMA pages. There isn't a nice way to do this now.
> 
> >If anyone has interest the idea, I will move into that.
> >If it sounds crazy idea, feel free to ignore, please.
> >
> 
> I'm interested in the idea with the warning noted above.

Okay, I will start the work if other going works are done.
I will send a patch with Ccing you if the prototype is done.
Of course, you could kick off firstly. :)

Thanks for the interest!

-- 
Kind regards,
Minchan Kim

--
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: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] mm: cma: free cma page to buddy instead of being cpu hot page
  2013-10-29  9:33 ` Mel Gorman
  2013-10-29 11:49   ` Zhang Mingjun
@ 2013-11-05 21:44   ` Andrew Morton
  2013-11-06  6:43     ` Minchan Kim
  1 sibling, 1 reply; 20+ messages in thread
From: Andrew Morton @ 2013-11-05 21:44 UTC (permalink / raw)
  To: Mel Gorman
  Cc: zhang.mingjun, minchan, m.szyprowski, haojian.zhuang, linux-mm,
	linux-kernel, Mingjun Zhang

On Tue, 29 Oct 2013 09:33:23 +0000 Mel Gorman <mgorman@suse.de> wrote:

> On Mon, Oct 28, 2013 at 07:42:49PM +0800, zhang.mingjun@linaro.org wrote:
> > From: Mingjun Zhang <troy.zhangmingjun@linaro.org>
> > 
> > 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.
> > 
> > Signed-off-by: Mingjun Zhang <troy.zhangmingjun@linaro.org>
> > ---
> >  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))
> >  			free_one_page(zone, page, 0, migratetype);
> >  			goto out;
> 
> This slightly impacts the page allocator free path for a marginal gain
> on CMA which are relatively rare allocations. There is no obvious
> benefit to this patch as I expect CMA allocations to flush the PCP lists
> when a range of pages have been isolated and migrated. Is there any
> measurable benefit to this patch?

The added overhead is pretty small - just a comparison of a local with
a constant.  And that cost is not incurred for MIGRATE_UNMOVABLE,
MIGRATE_RECLAIMABLE and MIGRATE_MOVABLE, which are the common cases
(yes?).

This thread is a bit straggly and inconclusive, but it sounds to me
that the benefit to CMA users is quite large and the cost to others is
small, so I'm inclined to run with the original patch.  Someone stop me
if that's wrong.

(we could speed up some of the migratetype tests if the MIGRATE_foo
constants were converted to bitfields.  The above test becomes "if
(migratetype & (MIGRATE_CMA|MIGRATE_ISOLATE))").

(why is is_migrate_cma() implemented as a macro in mmzone.h while
is_migrate_isolate() is an inline in page-isolation.h?)

--
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: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] mm: cma: free cma page to buddy instead of being cpu hot page
  2013-11-05 21:44   ` Andrew Morton
@ 2013-11-06  6:43     ` Minchan Kim
  2013-11-06 23:41       ` Andrew Morton
  0 siblings, 1 reply; 20+ messages in thread
From: Minchan Kim @ 2013-11-06  6:43 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Mel Gorman, zhang.mingjun, m.szyprowski, haojian.zhuang,
	linux-mm, linux-kernel, Mingjun Zhang

Hello Andrew,

On Tue, Nov 05, 2013 at 01:44:48PM -0800, Andrew Morton wrote:
> On Tue, 29 Oct 2013 09:33:23 +0000 Mel Gorman <mgorman@suse.de> wrote:
> 
> > On Mon, Oct 28, 2013 at 07:42:49PM +0800, zhang.mingjun@linaro.org wrote:
> > > From: Mingjun Zhang <troy.zhangmingjun@linaro.org>
> > > 
> > > 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.
> > > 
> > > Signed-off-by: Mingjun Zhang <troy.zhangmingjun@linaro.org>
> > > ---
> > >  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))
> > >  			free_one_page(zone, page, 0, migratetype);
> > >  			goto out;
> > 
> > This slightly impacts the page allocator free path for a marginal gain
> > on CMA which are relatively rare allocations. There is no obvious
> > benefit to this patch as I expect CMA allocations to flush the PCP lists
> > when a range of pages have been isolated and migrated. Is there any
> > measurable benefit to this patch?
> 
> The added overhead is pretty small - just a comparison of a local with
> a constant.  And that cost is not incurred for MIGRATE_UNMOVABLE,
> MIGRATE_RECLAIMABLE and MIGRATE_MOVABLE, which are the common cases
> (yes?).

True but bloat code might affect icache so we should be careful.
And what Mel has a concern is about zone->lock, which would be more contended.
I agree his opinion.

In addition, I think the gain is marginal because normally CMA is big range
so free_contig_range in dma release path will fill per_cpu_pages with freed pages
easily so it could drain per_cpu_pages frequently so race which steal page from
per_cpu_pages is not big, I guess.

Morever, we could change free_contig_range with batch_free_page which would
be useful for other cases if they want to free many number of pages
all at once.

The bottom line is we need *number and real scenario* for that.

If it's really needed, after merging this patch, we could enhance it with
batch_free_page so we could solve Mel's concern, too.

> 
> This thread is a bit straggly and inconclusive, but it sounds to me
> that the benefit to CMA users is quite large and the cost to others is
> small, so I'm inclined to run with the original patch.  Someone stop me
> if that's wrong.

I want you to stop until we see the number.

> 
> (we could speed up some of the migratetype tests if the MIGRATE_foo
> constants were converted to bitfields.  The above test becomes "if
> (migratetype & (MIGRATE_CMA|MIGRATE_ISOLATE))").
> 
> (why is is_migrate_cma() implemented as a macro in mmzone.h while
> is_migrate_isolate() is an inline in page-isolation.h?)

Just preference?
I like inline than macro and that's why is_migrate_isolate was inline.

> 
> --
> 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: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

-- 
Kind regards,
Minchan Kim

--
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: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] mm: cma: free cma page to buddy instead of being cpu hot page
  2013-11-06  6:43     ` Minchan Kim
@ 2013-11-06 23:41       ` Andrew Morton
  0 siblings, 0 replies; 20+ messages in thread
From: Andrew Morton @ 2013-11-06 23:41 UTC (permalink / raw)
  To: Minchan Kim
  Cc: Mel Gorman, zhang.mingjun, m.szyprowski, haojian.zhuang,
	linux-mm, linux-kernel, Mingjun Zhang

On Wed, 6 Nov 2013 15:43:02 +0900 Minchan Kim <minchan@kernel.org> wrote:

> > The added overhead is pretty small - just a comparison of a local with
> > a constant.  And that cost is not incurred for MIGRATE_UNMOVABLE,
> > MIGRATE_RECLAIMABLE and MIGRATE_MOVABLE, which are the common cases
> > (yes?).
> 
> True but bloat code might affect icache so we should be careful.
> And what Mel has a concern is about zone->lock, which would be more contended.
> I agree his opinion.
> 
> In addition, I think the gain is marginal because normally CMA is big range
> so free_contig_range in dma release path will fill per_cpu_pages with freed pages
> easily so it could drain per_cpu_pages frequently so race which steal page from
> per_cpu_pages is not big, I guess.
> 
> Morever, we could change free_contig_range with batch_free_page which would
> be useful for other cases if they want to free many number of pages
> all at once.
> 
> The bottom line is we need *number and real scenario* for that.

Well yes, quantitative results are always good to have with a patch like
this.

It doesn't actually compile (missing a "}"), which doesn't inspire
confidence.  I'll make the patch go away for now

--
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: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] mm: cma: free cma page to buddy instead of being cpu hot page
  2013-10-30  5:40           ` Minchan Kim
  2013-10-31  2:14             ` Laura Abbott
@ 2013-12-23 12:38             ` Wanpeng Li
  1 sibling, 0 replies; 20+ messages in thread
From: Wanpeng Li @ 2013-12-23 12:38 UTC (permalink / raw)
  To: Minchan Kim
  Cc: Laura Abbott, Zhang Mingjun, Mel Gorman, Marek Szyprowski, akpm,
	Haojian Zhuang, linux-mm, linux-kernel, troy.zhangmingjun

On Wed, Oct 30, 2013 at 02:40:06PM +0900, Minchan Kim wrote:
>Hello,
>
>On Tue, Oct 29, 2013 at 09:14:03AM -0700, Laura Abbott wrote:
>> On 10/29/2013 8:02 AM, Zhang Mingjun wrote:
>> 
>> >    It would move the cost to the CMA paths so I would complain less. Bear
>> >    in mind as well that forcing everything to go through free_one_page()
>> >    means that every free goes through the zone lock. I doubt you have any
>> >    machine large enough but it is possible for simultaneous CMA allocations
>> >    to now contend on the zone lock that would have been previously fine.
>> >    Hence, I'm interesting in knowing the underlying cause of the
>> >    problem you
>> >    are experiencing.
>> >
>> >my platform uses CMA but disabled CMA's migration func by del MIGRATE_CMA
>> >in fallbacks[MIGRATE_MOVEABLE]. But I find CMA pages can still used by
>> >pagecache or page fault page request from PCP list and cma allocation has to
>> >migrate these page. So I want to free these cma pages to buddy directly
>> >not PCP..
>> >
>> >     > of course, it will waste the memory outside of the alloc range
>> >    but in the
>> >     > pageblocks.
>> >     >
>> >
>> >    I would hope/expect that the loss would only last for the duration of
>> >    the allocation attempt and a small amount of memory.
>> >
>> >     > > when a range of pages have been isolated and migrated. Is there any
>> >     > > measurable benefit to this patch?
>> >     > >
>> >     > after applying this patch, the video player on my platform works more
>> >     > fluent,
>> >
>> >    fluent almost always refers to ones command of a spoken language. I do
>> >    not see how a video player can be fluent in anything. What is measurably
>> >    better?
>> >
>> >    For example, are allocations faster? If so, why? What cost from another
>> >    path is removed as a result of this patch? If the cost is in the PCP
>> >    flush then can it be checked if the PCP flush was unnecessary and called
>> >    unconditionally even though all the pages were freed already? We had
>> >    problems in the past where drain_all_pages() or similar were called
>> >    unnecessarily causing long sync stalls related to IPIs. I'm wondering if
>> >    we are seeing a similar problem here.
>> >
>> >    Maybe the problem is the complete opposite. Are allocations failing
>> >    because there are PCP pages in the way? In that case, it real fix might
>> >    be to insert a  if the allocation is failing due to per-cpu
>> >    pages.
>> >
>> >problem is not the allocation failing, but the unexpected cma migration
>> >slows
>> >down the allocation.
>> >
>> >
>> >     > and the driver of video decoder on my test platform using cma
>> >    alloc/free
>> >     > frequently.
>> >     >
>> >
>> >    CMA allocations are almost never used outside of these contexts. While I
>> >    appreciate that embedded use is important I'm reluctant to see an impact
>> >    in fast paths unless there is a good reason for every other use case. I
>> >    also am a bit unhappy to see CMA allocations making the zone->lock
>> >    hotter than necessary even if no embedded use case it likely to
>> >    experience the problem in the short-term.
>> >
>> >    --
>> >    Mel Gorman
>> >    SUSE Labs
>> >
>> >
>> 
>> We've had a similar patch in our tree for a year and a half because
>> of CMA migration failures, not just for a speedup in allocation
>> time. I understand that CMA is not the fast case or the general use
>> case but the problem is that the cost of CMA failure is very high
>> (complete failure of the feature using CMA). Putting CMA on the PCP
>> lists means they may be picked up by users who temporarily make the
>> movable pages unmovable (page cache etc.) which prevents the
>> allocation from succeeding. The problem still exists even if the CMA
>> pages are not on the PCP list but the window gets slightly smaller.
>
>I understand that I have seen many people want to use CMA have tweaked
>their system to work well and although they do best effort, it doesn't
>work well because CMA doesn't gaurantee to succeed in getting free
>space since there are lots of hurdle. (get_user_pages, AIO ring buffer,
>buffer cache, short of free memory for migration, no swap and so on).
>Even, someone want to allocate CMA space with speedy. SIGH.
>
>Yeah, at the moment, CMA is really SUCK.
>
>> 
>> This really highlights one of the biggest issues with CMA today.
>> Movable pages make return -EBUSY for any number of reasons. For
>> non-CMA pages this is mostly fine, another movable page may be
>> substituted for the movable page that is busy. CMA is a restricted
>> range though so any failure in that range is very costly because CMA
>> regions are generally sized exactly for the use cases at hand which
>> means there is very little extra space for retries.
>> 
>> To make CMA actually usable, we've had to go through and add in
>> hacks/quirks that prevent CMA from being allocated in any path which
>> may prevent migration. I've been mixed on if this is the right path
>> or if the definition of MIGRATE_CMA needs to be changed to be more
>> restrictive (can't prevent migration).
>
>Fundamental problem is that every subsystem could grab a page anytime
>and they doesn't gaurantee to release it soonish or within time CMA
>user want so it turns out non-determisitic mess which just hook into
>core MM system here and there.
>
>Sometime, I see some people try to solve it case by case with ad-hoc
>approach. I guess it would be never ending story as kernel evolves.
>
>I suggest that we could make new wheel with frontswap/cleancache stuff.
>The idea is that pages in frontswap/cleancache are evicted from kernel
>POV so that we can gaurantee that there is no chance to grab a page
>in CMA area and we could remove lots of hook from core MM which just
>complicated MM without benefit.
>
>As benefit, cleancache pages could drop easily so it would be fast
>to get free space but frontswap cache pages should be move into somewhere.
>If there are enough free pages, it could be migrated out there. Optionally
>we could compress them. Otherwise, we could pageout them into backed device.
>Yeah, it could be slow than migration but at least, we could estimate the time
>by storage speed ideally so we could have tunable knob. If someone want
>fast CMA, he could control it with ratio of cleancache:frontswap.
>IOW, higher frontswap page ratio is, slower the speed would be.
>Important thing is admin could have tuned control knob and it gaurantees to
>get CMA free space with deterministic time.
>
>As drawback, if we fail to tune the ratio, memeory efficieny would be
>bad so that it ends up thrashing but you guys is saying we have been
>used CMA without movable fallback which means that it's already static
>reserved memory and it's never CMA so you already have lost memory
>efficiency and even fail to get a space so I think it's good trade-off
>for embedded people.
>
>If anyone has interest the idea, I will move into that.
>If it sounds crazy idea, feel free to ignore, please.
>

Interesting. ;-)

Regards,
Wanpeng Li 

>Thanks.
>
>> 
>> Thanks,
>> Laura
>> -- 
>> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
>> hosted by The Linux Foundation
>> 
>> --
>> 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: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
>
>-- 
>Kind regards,
>Minchan Kim
>
>--
>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: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

--
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: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2013-12-23 12:38 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-10-28 11:42 [PATCH] mm: cma: free cma page to buddy instead of being cpu hot page zhang.mingjun
2013-10-28 16:04 ` Laura Abbott
2013-10-29  4:54 ` Minchan Kim
2013-10-29  6:41   ` KOSAKI Motohiro
2013-10-29  7:00   ` Zhang Mingjun
2013-10-29  7:25     ` Minchan Kim
2013-10-29 11:17       ` Zhang Mingjun
2013-10-29  9:33 ` Mel Gorman
2013-10-29 11:49   ` Zhang Mingjun
2013-10-29 12:27     ` Mel Gorman
2013-10-29 15:02       ` Zhang Mingjun
2013-10-29 16:14         ` Laura Abbott
2013-10-30  5:40           ` Minchan Kim
2013-10-31  2:14             ` Laura Abbott
2013-11-01  1:49               ` Minchan Kim
2013-12-23 12:38             ` Wanpeng Li
2013-10-30  2:55         ` Minchan Kim
2013-11-05 21:44   ` Andrew Morton
2013-11-06  6:43     ` Minchan Kim
2013-11-06 23:41       ` Andrew Morton

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).