From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pd0-f169.google.com (mail-pd0-f169.google.com [209.85.192.169]) by kanga.kvack.org (Postfix) with ESMTP id 5B5886B0031 for ; Tue, 29 Oct 2013 07:17:35 -0400 (EDT) Received: by mail-pd0-f169.google.com with SMTP id q10so8412180pdj.14 for ; Tue, 29 Oct 2013 04:17:35 -0700 (PDT) Received: from psmtp.com ([74.125.245.131]) by mx.google.com with SMTP id mi5si15574520pab.251.2013.10.29.04.17.27 for ; Tue, 29 Oct 2013 04:17:28 -0700 (PDT) Received: by mail-wg0-f51.google.com with SMTP id l18so8088914wgh.30 for ; Tue, 29 Oct 2013 04:17:25 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <20131029072511.GA6030@bbox> References: <1382960569-6564-1-git-send-email-zhang.mingjun@linaro.org> <20131029045430.GE17038@bbox> <20131029072511.GA6030@bbox> Date: Tue, 29 Oct 2013 19:17:25 +0800 Message-ID: Subject: Re: [PATCH] mm: cma: free cma page to buddy instead of being cpu hot page From: Zhang Mingjun Content-Type: multipart/alternative; boundary=089e01227dfe7d96d704e9df5c9c Sender: owner-linux-mm@kvack.org List-ID: To: Minchan Kim Cc: Marek Szyprowski , akpm@linux-foundation.org, Mel Gorman , Haojian Zhuang , linux-mm@kvack.org, linux-kernel@vger.kernel.org, Mingjun Zhang , KOSAKI Motohiro --089e01227dfe7d96d704e9df5c9c Content-Type: text/plain; charset=ISO-8859-1 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 > --089e01227dfe7d96d704e9df5c9c Content-Type: text/html; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable
On T= ue, Oct 29, 2013 at 3:25 PM, Minchan Kim <minchan@kernel.org> wrote:
On T= ue, 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 un= necessary
> > > migration action when these pages reused by CMA.
> >
> > You are saying about the overhead but I'm not sure how much i= t 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 h= ow 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 t= hink
> > it has no problem.
> >
> problem is when free_contig_range frees cma pages, page's migratio= n type is
> MIGRATE_CMA!
> I don't know why free_contig_range free pages one by one, but in t= he end it
> calls free_hot_cold_page,
> so some of these MIGRATE_CMA pages will be used as MIGRATE_MOVEABLE, t= his
> break the CMA
> allocation policy and it's not the low hit path, it's really t= he 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 o= ne
> page using alloc_pages(MOVABLE, 0)
> it will get the page from this pcp list, breaking the CMA fallback rul= es,
> 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>
> > > ---
> > > =A0mm/page_alloc.c | =A0 =A03 ++-
> > > =A01 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 *p= age, int
> > cold)
> > > =A0 =A0 =A0 =A0* excessively into the page allocator
> > > =A0 =A0 =A0 =A0*/
> > > =A0 =A0 =A0 if (migratetype >=3D MIGRATE_PCPTYPES) {
> > > - =A0 =A0 =A0 =A0 =A0 =A0 if (unlikely(is_migrate_isolate(mi= gratetype))) {
> > > + =A0 =A0 =A0 =A0 =A0 =A0 if (unlikely(is_migrate_isolate(mi= gratetype))
> > > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 || 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 o= nly
> > 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 >=3D MIGRATE_PCPTYPES)) {
> > =A0 =A0 =A0 =A0 if (is_migrate_isolate(migratetype) || is_migrate= _cma(migratetype))
> >
> > if CMA is enabled and alloc/free frequently, it will more likely<= br> > migratetype >=3D MIGRATE_PCPTYPES

Until now, I didn't notice there is such workload. Do you h= ave such real workload?
yes, my test platform using cm= a for video decoder, it alloc/free cma frequently.

If so, we should change it with following as?

if (migratetype >=3D MIGRATE_PCPTYPES) {
=A0 =A0 =A0 =A0 if (is_migrate_cma(migratetype) || unlikely(is_migrate_isol= ate(migratetype)))
ok.

Because assumption is you insist that there is lots of alloc/free for CMA.<= br> But since we have had unlikely on memory-hotplug check, it would be less th= an CMA.



>
> I know it's an another topic but I'd like to disucss it in thi= s time because
> > we will forget such trivial thing later, again.
> >
> > }
> >
> > > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 free_one_page(zo= ne, page, 0, migratetype);
> > > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 goto out;
> > > =A0 =A0 =A0 =A0 =A0 =A0 =A0 }
> > > --
> > > 1.7.9.5
> > >
> > > --
> > > To unsubscribe, send a message with 'unsubscribe linux-m= m' in
> > > the body to majordomo= @kvack.org. =A0For more info on Linux MM,
> > > see: = http://www.linux-mm.org/ .
> > > Don't email: <a href=3Dmailto:"dont@kvack.org"> email@kvack.org </a>
> >
> > --
> > Kind regards,
> > Minchan Kim
> >

--
Kind regards,
Minchan Kim

--089e01227dfe7d96d704e9df5c9c-- -- 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