Re: [PATCH v2 8/9] mm/compaction: don't use higher order freepage than compaction aims at

[Joonsoo Kim]

On Fri, Sep 25, 2015 at 01:41:48PM +0200, Vlastimil Babka wrote:
> On 08/24/2015 04:19 AM, Joonsoo Kim wrote:
> > Purpose of compaction is to make high order page. To achive this purpose,
> > it is the best strategy that compaction migrates contiguous used pages
> > to fragmented unused freepages. Currently, freepage scanner don't
> > distinguish whether freepage is fragmented or not and blindly use
> > any freepage for migration target regardless of freepage's order.
> > 
> > Using higher order freepage than compaction aims at is not good because
> > what we do here is breaking high order freepage at somewhere and migrating
> > used pages from elsewhere to this broken high order freepages in order to
> > make new high order freepage. That is just position change of high order
> > freepage.
> > 
> > This is useless effort and doesn't help to make more high order freepages
> > because we can't be sure that migrating used pages makes high order
> > freepage. So, this patch makes freepage scanner only uses the ordered
> > freepage lower than compaction order.
> 
> How often does this happen? If there's a free page of the order we need, then 
> we
> are done compacting anyway, no? Or is this happening because of the current
> high-order watermark checking implementation? It would be interesting to 
> measure
> how often this skip would trigger. Also watermark checking should change with
> Mel's patchset and then this patch shouldn't be needed?

Yes, you are right. This would be happening because of the current
high-order watermakr checking implementation and Mel's patchset will
solve it and if it is merged, this patch isn't needed.

> 
> > Signed-off-by: Joonsoo Kim 
> > ---
> >  mm/compaction.c | 15 +++
> >  1 file changed, 15 insertions(+)
> > 
> > diff --git a/mm/compaction.c b/mm/compaction.c
> > index ca4d6d1..e61ee77 100644
> > --- a/mm/compaction.c
> > +++ b/mm/compaction.c
> > @@ -455,6 +455,7 @@ static unsigned long isolate_freepages_block(struct 
> > compact_control *cc,
> > unsigned long flags = 0;
> > bool locked = false;
> > unsigned long blockpfn = *start_pfn;
> > +   unsigned long freepage_order;
> >  
> > cursor = pfn_to_page(blockpfn);
> >  
> > @@ -482,6 +483,20 @@ static unsigned long isolate_freepages_block(struct 
> > compact_control *cc,
> > if (!PageBuddy(page))
> > goto isolate_fail;
> >  
> > +   if (!strict && cc->order != -1) {
> > +   freepage_order = page_order_unsafe(page);
> > +
> > +   if (freepage_order > 0 && freepage_order < MAX_ORDER) {
> > +   /*
> > +* Do not use high order freepage for migration
> > +* taret. It would not be beneficial for
> > +* compaction success rate.
> > +*/
> > +   if (freepage_order >= cc->order)
> 
> It would be better to skip the whole freepage_order.

Okay.

Thanks.

> 
> > +   goto isolate_fail;
> > +   }
> > +   }
> > +
> > /*
> >  * If we already hold the lock, we can skip some rechecking.
> >  * Note that if we hold the lock now, checked_pageblock was
> > 
> 
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majord...@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: mailto:"d...@kvack.org;> em...@kvack.org 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2 8/9] mm/compaction: don't use higher order freepage than compaction aims at

[Joonsoo Kim]

On Fri, Sep 25, 2015 at 01:41:48PM +0200, Vlastimil Babka wrote:
> On 08/24/2015 04:19 AM, Joonsoo Kim wrote:
> > Purpose of compaction is to make high order page. To achive this purpose,
> > it is the best strategy that compaction migrates contiguous used pages
> > to fragmented unused freepages. Currently, freepage scanner don't
> > distinguish whether freepage is fragmented or not and blindly use
> > any freepage for migration target regardless of freepage's order.
> > 
> > Using higher order freepage than compaction aims at is not good because
> > what we do here is breaking high order freepage at somewhere and migrating
> > used pages from elsewhere to this broken high order freepages in order to
> > make new high order freepage. That is just position change of high order
> > freepage.
> > 
> > This is useless effort and doesn't help to make more high order freepages
> > because we can't be sure that migrating used pages makes high order
> > freepage. So, this patch makes freepage scanner only uses the ordered
> > freepage lower than compaction order.
> 
> How often does this happen? If there's a free page of the order we need, then 
> we
> are done compacting anyway, no? Or is this happening because of the current
> high-order watermark checking implementation? It would be interesting to 
> measure
> how often this skip would trigger. Also watermark checking should change with
> Mel's patchset and then this patch shouldn't be needed?

Yes, you are right. This would be happening because of the current
high-order watermakr checking implementation and Mel's patchset will
solve it and if it is merged, this patch isn't needed.

> 
> > Signed-off-by: Joonsoo Kim 
> > ---
> >  mm/compaction.c | 15 +++
> >  1 file changed, 15 insertions(+)
> > 
> > diff --git a/mm/compaction.c b/mm/compaction.c
> > index ca4d6d1..e61ee77 100644
> > --- a/mm/compaction.c
> > +++ b/mm/compaction.c
> > @@ -455,6 +455,7 @@ static unsigned long isolate_freepages_block(struct 
> > compact_control *cc,
> > unsigned long flags = 0;
> > bool locked = false;
> > unsigned long blockpfn = *start_pfn;
> > +   unsigned long freepage_order;
> >  
> > cursor = pfn_to_page(blockpfn);
> >  
> > @@ -482,6 +483,20 @@ static unsigned long isolate_freepages_block(struct 
> > compact_control *cc,
> > if (!PageBuddy(page))
> > goto isolate_fail;
> >  
> > +   if (!strict && cc->order != -1) {
> > +   freepage_order = page_order_unsafe(page);
> > +
> > +   if (freepage_order > 0 && freepage_order < MAX_ORDER) {
> > +   /*
> > +* Do not use high order freepage for migration
> > +* taret. It would not be beneficial for
> > +* compaction success rate.
> > +*/
> > +   if (freepage_order >= cc->order)
> 
> It would be better to skip the whole freepage_order.

Okay.

Thanks.

> 
> > +   goto isolate_fail;
> > +   }
> > +   }
> > +
> > /*
> >  * If we already hold the lock, we can skip some rechecking.
> >  * Note that if we hold the lock now, checked_pageblock was
> > 
> 
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majord...@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: mailto:"d...@kvack.org;> em...@kvack.org 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2 8/9] mm/compaction: don't use higher order freepage than compaction aims at

[Vlastimil Babka]

On 08/24/2015 04:19 AM, Joonsoo Kim wrote:
> Purpose of compaction is to make high order page. To achive this purpose,
> it is the best strategy that compaction migrates contiguous used pages
> to fragmented unused freepages. Currently, freepage scanner don't
> distinguish whether freepage is fragmented or not and blindly use
> any freepage for migration target regardless of freepage's order.
> 
> Using higher order freepage than compaction aims at is not good because
> what we do here is breaking high order freepage at somewhere and migrating
> used pages from elsewhere to this broken high order freepages in order to
> make new high order freepage. That is just position change of high order
> freepage.
> 
> This is useless effort and doesn't help to make more high order freepages
> because we can't be sure that migrating used pages makes high order
> freepage. So, this patch makes freepage scanner only uses the ordered
> freepage lower than compaction order.

How often does this happen? If there's a free page of the order we need, then we
are done compacting anyway, no? Or is this happening because of the current
high-order watermark checking implementation? It would be interesting to measure
how often this skip would trigger. Also watermark checking should change with
Mel's patchset and then this patch shouldn't be needed?

> Signed-off-by: Joonsoo Kim 
> ---
>  mm/compaction.c | 15 +++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/mm/compaction.c b/mm/compaction.c
> index ca4d6d1..e61ee77 100644
> --- a/mm/compaction.c
> +++ b/mm/compaction.c
> @@ -455,6 +455,7 @@ static unsigned long isolate_freepages_block(struct 
> compact_control *cc,
>   unsigned long flags = 0;
>   bool locked = false;
>   unsigned long blockpfn = *start_pfn;
> + unsigned long freepage_order;
>  
>   cursor = pfn_to_page(blockpfn);
>  
> @@ -482,6 +483,20 @@ static unsigned long isolate_freepages_block(struct 
> compact_control *cc,
>   if (!PageBuddy(page))
>   goto isolate_fail;
>  
> + if (!strict && cc->order != -1) {
> + freepage_order = page_order_unsafe(page);
> +
> + if (freepage_order > 0 && freepage_order < MAX_ORDER) {
> + /*
> +  * Do not use high order freepage for migration
> +  * taret. It would not be beneficial for
> +  * compaction success rate.
> +  */
> + if (freepage_order >= cc->order)

It would be better to skip the whole freepage_order.

> + goto isolate_fail;
> + }
> + }
> +
>   /*
>* If we already hold the lock, we can skip some rechecking.
>* Note that if we hold the lock now, checked_pageblock was
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2 8/9] mm/compaction: don't use higher order freepage than compaction aims at

[Vlastimil Babka]

On 08/24/2015 04:19 AM, Joonsoo Kim wrote:
> Purpose of compaction is to make high order page. To achive this purpose,
> it is the best strategy that compaction migrates contiguous used pages
> to fragmented unused freepages. Currently, freepage scanner don't
> distinguish whether freepage is fragmented or not and blindly use
> any freepage for migration target regardless of freepage's order.
> 
> Using higher order freepage than compaction aims at is not good because
> what we do here is breaking high order freepage at somewhere and migrating
> used pages from elsewhere to this broken high order freepages in order to
> make new high order freepage. That is just position change of high order
> freepage.
> 
> This is useless effort and doesn't help to make more high order freepages
> because we can't be sure that migrating used pages makes high order
> freepage. So, this patch makes freepage scanner only uses the ordered
> freepage lower than compaction order.

How often does this happen? If there's a free page of the order we need, then we
are done compacting anyway, no? Or is this happening because of the current
high-order watermark checking implementation? It would be interesting to measure
how often this skip would trigger. Also watermark checking should change with
Mel's patchset and then this patch shouldn't be needed?

> Signed-off-by: Joonsoo Kim 
> ---
>  mm/compaction.c | 15 +++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/mm/compaction.c b/mm/compaction.c
> index ca4d6d1..e61ee77 100644
> --- a/mm/compaction.c
> +++ b/mm/compaction.c
> @@ -455,6 +455,7 @@ static unsigned long isolate_freepages_block(struct 
> compact_control *cc,
>   unsigned long flags = 0;
>   bool locked = false;
>   unsigned long blockpfn = *start_pfn;
> + unsigned long freepage_order;
>  
>   cursor = pfn_to_page(blockpfn);
>  
> @@ -482,6 +483,20 @@ static unsigned long isolate_freepages_block(struct 
> compact_control *cc,
>   if (!PageBuddy(page))
>   goto isolate_fail;
>  
> + if (!strict && cc->order != -1) {
> + freepage_order = page_order_unsafe(page);
> +
> + if (freepage_order > 0 && freepage_order < MAX_ORDER) {
> + /*
> +  * Do not use high order freepage for migration
> +  * taret. It would not be beneficial for
> +  * compaction success rate.
> +  */
> + if (freepage_order >= cc->order)

It would be better to skip the whole freepage_order.

> + goto isolate_fail;
> + }
> + }
> +
>   /*
>* If we already hold the lock, we can skip some rechecking.
>* Note that if we hold the lock now, checked_pageblock was
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v2 8/9] mm/compaction: don't use higher order freepage than compaction aims at

[Joonsoo Kim]

Purpose of compaction is to make high order page. To achive this purpose,
it is the best strategy that compaction migrates contiguous used pages
to fragmented unused freepages. Currently, freepage scanner don't
distinguish whether freepage is fragmented or not and blindly use
any freepage for migration target regardless of freepage's order.

Using higher order freepage than compaction aims at is not good because
what we do here is breaking high order freepage at somewhere and migrating
used pages from elsewhere to this broken high order freepages in order to
make new high order freepage. That is just position change of high order
freepage.

This is useless effort and doesn't help to make more high order freepages
because we can't be sure that migrating used pages makes high order
freepage. So, this patch makes freepage scanner only uses the ordered
freepage lower than compaction order.

Signed-off-by: Joonsoo Kim 
---
 mm/compaction.c | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/mm/compaction.c b/mm/compaction.c
index ca4d6d1..e61ee77 100644
--- a/mm/compaction.c
+++ b/mm/compaction.c
@@ -455,6 +455,7 @@ static unsigned long isolate_freepages_block(struct 
compact_control *cc,
unsigned long flags = 0;
bool locked = false;
unsigned long blockpfn = *start_pfn;
+   unsigned long freepage_order;
 
cursor = pfn_to_page(blockpfn);
 
@@ -482,6 +483,20 @@ static unsigned long isolate_freepages_block(struct 
compact_control *cc,
if (!PageBuddy(page))
goto isolate_fail;
 
+   if (!strict && cc->order != -1) {
+   freepage_order = page_order_unsafe(page);
+
+   if (freepage_order > 0 && freepage_order < MAX_ORDER) {
+   /*
+* Do not use high order freepage for migration
+* taret. It would not be beneficial for
+* compaction success rate.
+*/
+   if (freepage_order >= cc->order)
+   goto isolate_fail;
+   }
+   }
+
/*
 * If we already hold the lock, we can skip some rechecking.
 * Note that if we hold the lock now, checked_pageblock was
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v2 8/9] mm/compaction: don't use higher order freepage than compaction aims at

[Joonsoo Kim]

Purpose of compaction is to make high order page. To achive this purpose,
it is the best strategy that compaction migrates contiguous used pages
to fragmented unused freepages. Currently, freepage scanner don't
distinguish whether freepage is fragmented or not and blindly use
any freepage for migration target regardless of freepage's order.

Using higher order freepage than compaction aims at is not good because
what we do here is breaking high order freepage at somewhere and migrating
used pages from elsewhere to this broken high order freepages in order to
make new high order freepage. That is just position change of high order
freepage.

This is useless effort and doesn't help to make more high order freepages
because we can't be sure that migrating used pages makes high order
freepage. So, this patch makes freepage scanner only uses the ordered
freepage lower than compaction order.

Signed-off-by: Joonsoo Kim iamjoonsoo@lge.com
---
 mm/compaction.c | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/mm/compaction.c b/mm/compaction.c
index ca4d6d1..e61ee77 100644
--- a/mm/compaction.c
+++ b/mm/compaction.c
@@ -455,6 +455,7 @@ static unsigned long isolate_freepages_block(struct 
compact_control *cc,
unsigned long flags = 0;
bool locked = false;
unsigned long blockpfn = *start_pfn;
+   unsigned long freepage_order;
 
cursor = pfn_to_page(blockpfn);
 
@@ -482,6 +483,20 @@ static unsigned long isolate_freepages_block(struct 
compact_control *cc,
if (!PageBuddy(page))
goto isolate_fail;
 
+   if (!strict  cc-order != -1) {
+   freepage_order = page_order_unsafe(page);
+
+   if (freepage_order  0  freepage_order  MAX_ORDER) {
+   /*
+* Do not use high order freepage for migration
+* taret. It would not be beneficial for
+* compaction success rate.
+*/
+   if (freepage_order = cc-order)
+   goto isolate_fail;
+   }
+   }
+
/*
 * If we already hold the lock, we can skip some rechecking.
 * Note that if we hold the lock now, checked_pageblock was
-- 
1.9.1

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/