[PATCH] mm: Check if any page in a pageblock is reserved before marking it MIGRATE_RESERVE

[PATCH] mm: Check if any page in a pageblock is reserved before marking it MIGRATE_RESERVE

[Arve Hjønnevåg]

This fixes a problem where the first pageblock got marked MIGRATE_RESERVE even
though it only had a few free pages. This in turn caused no contiguous memory
to be reserved and frequent kswapd wakeups that emptied the caches to get more
contiguous memory.

Signed-off-by: Arve Hjønnevåg <arve@android.com>
---
 mm/page_alloc.c |   16 +++++++++++++++-
 1 files changed, 15 insertions(+), 1 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index fb7df1d..46ade16 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -2860,6 +2860,20 @@ static inline unsigned long wait_table_bits(unsigned long size)
 #define LONG_ALIGN(x) (((x)+(sizeof(long))-1)&~((sizeof(long))-1))
 
 /*
+ * Check if a pageblock contains reserved pages
+ */
+static int pageblock_is_reserved(unsigned long start_pfn)
+{
+	unsigned long end_pfn = start_pfn + pageblock_nr_pages;
+	unsigned long pfn;
+
+	for (pfn = start_pfn; pfn < end_pfn; pfn++)
+		if (!pfn_valid_within(pfn) || PageReserved(pfn_to_page(pfn)))
+			return 1;
+	return 0;
+}
+
+/*
  * Mark a number of pageblocks as MIGRATE_RESERVE. The number
  * of blocks reserved is based on min_wmark_pages(zone). The memory within
  * the reserve will tend to store contiguous free pages. Setting min_free_kbytes
@@ -2898,7 +2912,7 @@ static void setup_zone_migrate_reserve(struct zone *zone)
 			continue;
 
 		/* Blocks with reserved pages will never free, skip them. */
-		if (PageReserved(page))
+		if (pageblock_is_reserved(pfn))
 			continue;
 
 		block_migratetype = get_pageblock_migratetype(page);
-- 
1.6.5.1

Re: [PATCH] mm: Check if any page in a pageblock is reserved before marking it MIGRATE_RESERVE

[Minchan Kim]

On Tue, Apr 6, 2010 at 12:09 PM, Arve Hjønnevåg <arve@android.com> wrote:
> This fixes a problem where the first pageblock got marked MIGRATE_RESERVE even
> though it only had a few free pages. This in turn caused no contiguous memory
> to be reserved and frequent kswapd wakeups that emptied the caches to get more
> contiguous memory.

It would be better to add following your description of previous mail thread.
It can help others understand it in future.

On Fri, Apr 02, 2010 at 05:59:00PM -0700, Arve Hj?nnev?g wrote:
...
"I think this happens by default on arm. The kernel starts at offset
0x8000 to leave room for boot parameters, and in recent kernel
versions (>~2.6.26-29) this memory is freed."


-- 
Kind regards,
Minchan Kim

Re: [PATCH] mm: Check if any page in a pageblock is reserved before marking it MIGRATE_RESERVE

[Mel Gorman]

On Mon, Apr 05, 2010 at 08:09:16PM -0700, Arve Hj?nnev?g wrote:
> This fixes a problem where the first pageblock got marked MIGRATE_RESERVE even
> though it only had a few free pages. This in turn caused no contiguous memory
> to be reserved and frequent kswapd wakeups that emptied the caches to get more
> contiguous memory.
> 
> Signed-off-by: Arve Hjønnevåg <arve@android.com>

I would have used pageblock_reserve_suitable because what you are really
checking is "is this page block suitable for use by MIGRATE_RESERVE?".
The definition was "is the first page PageReserved" and you are changing it to
"does the page block have any memory holes or PageReserved pages?"

No biggie though. Change it if you like before upstreaming. Either way.

Acked-by: Mel Gorman <mel@csn.ul.ie>


Thanks

> ---
>  mm/page_alloc.c |   16 +++++++++++++++-
>  1 files changed, 15 insertions(+), 1 deletions(-)
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index fb7df1d..46ade16 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -2860,6 +2860,20 @@ static inline unsigned long wait_table_bits(unsigned long size)
>  #define LONG_ALIGN(x) (((x)+(sizeof(long))-1)&~((sizeof(long))-1))
>  
>  /*
> + * Check if a pageblock contains reserved pages
> + */
> +static int pageblock_is_reserved(unsigned long start_pfn)
> +{
> +	unsigned long end_pfn = start_pfn + pageblock_nr_pages;
> +	unsigned long pfn;
> +
> +	for (pfn = start_pfn; pfn < end_pfn; pfn++)
> +		if (!pfn_valid_within(pfn) || PageReserved(pfn_to_page(pfn)))
> +			return 1;
> +	return 0;
> +}
> +
> +/*
>   * Mark a number of pageblocks as MIGRATE_RESERVE. The number
>   * of blocks reserved is based on min_wmark_pages(zone). The memory within
>   * the reserve will tend to store contiguous free pages. Setting min_free_kbytes
> @@ -2898,7 +2912,7 @@ static void setup_zone_migrate_reserve(struct zone *zone)
>  			continue;
>  
>  		/* Blocks with reserved pages will never free, skip them. */
> -		if (PageReserved(page))
> +		if (pageblock_is_reserved(pfn))
>  			continue;
>  
>  		block_migratetype = get_pageblock_migratetype(page);
> -- 
> 1.6.5.1
> 

-- 
Mel Gorman
Part-time Phd Student                          Linux Technology Center
University of Limerick                         IBM Dublin Software Lab

[PATCH] mm: Check if any page in a pageblock is reserved before marking it MIGRATE_RESERVE

[John Stultz]

From: Arve Hjønnevåg <arve@android.com>

This fixes a problem where the first pageblock got marked MIGRATE_RESERVE even
though it only had a few free pages. This in turn caused no contiguous memory
to be reserved and frequent kswapd wakeups that emptied the caches to get more
contiguous memory.

CC: Dave Hansen <dave@linux.vnet.ibm.com>
CC: Mel Gorman <mgorman@suse.de>
CC: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Arve Hjønnevåg <arve@android.com>
Acked-by: Mel Gorman <mel@csn.ul.ie>

[This patch was submitted and acked a little over a year ago
(see: http://lkml.org/lkml/2010/4/6/172 ), but never seemingly
made it upstream. Resending for comments. -jstultz]

Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 mm/page_alloc.c |   16 +++++++++++++++-
 1 files changed, 15 insertions(+), 1 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index ed87f3b..209d9bf 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -3288,6 +3288,20 @@ static inline unsigned long wait_table_bits(unsigned long size)
 #define LONG_ALIGN(x) (((x)+(sizeof(long))-1)&~((sizeof(long))-1))
 
 /*
+ * Check if a pageblock contains reserved pages
+ */
+static int pageblock_is_reserved(unsigned long start_pfn)
+{
+	unsigned long end_pfn = start_pfn + pageblock_nr_pages;
+	unsigned long pfn;
+
+	for (pfn = start_pfn; pfn < end_pfn; pfn++)
+		if (PageReserved(pfn_to_page(pfn)))
+			return 1;
+	return 0;
+}
+
+/*
  * Mark a number of pageblocks as MIGRATE_RESERVE. The number
  * of blocks reserved is based on min_wmark_pages(zone). The memory within
  * the reserve will tend to store contiguous free pages. Setting min_free_kbytes
@@ -3326,7 +3340,7 @@ static void setup_zone_migrate_reserve(struct zone *zone)
 			continue;
 
 		/* Blocks with reserved pages will never free, skip them. */
-		if (PageReserved(page))
+		if (pageblock_is_reserved(pfn))
 			continue;
 
 		block_migratetype = get_pageblock_migratetype(page);
-- 
1.7.3.2.146.gca209

Re: [PATCH] mm: Check if any page in a pageblock is reserved before marking it MIGRATE_RESERVE

[Dave Hansen]

On Thu, 2011-04-21 at 18:34 -0700, John Stultz wrote:
> This fixes a problem where the first pageblock got marked MIGRATE_RESERVE even
> though it only had a few free pages. This in turn caused no contiguous memory
> to be reserved and frequent kswapd wakeups that emptied the caches to get more
> contiguous memory.

This looks sane to me.

Acked-by: Dave Hansen <dave@linux.vnet.ibm.com> 

-- Dave

Re: [PATCH] mm: Check if any page in a pageblock is reserved before marking it MIGRATE_RESERVE

[Mel Gorman]

On Thu, Apr 21, 2011 at 06:34:03PM -0700, John Stultz wrote:
> From: Arve Hjønnevåg <arve@android.com>
> 
> This fixes a problem where the first pageblock got marked MIGRATE_RESERVE even
> though it only had a few free pages. This in turn caused no contiguous memory
> to be reserved and frequent kswapd wakeups that emptied the caches to get more
> contiguous memory.
> 
> CC: Dave Hansen <dave@linux.vnet.ibm.com>
> CC: Mel Gorman <mgorman@suse.de>
> CC: Andrew Morton <akpm@linux-foundation.org>
> Signed-off-by: Arve Hjønnevåg <arve@android.com>
> Acked-by: Mel Gorman <mel@csn.ul.ie>
> 
> [This patch was submitted and acked a little over a year ago
> (see: http://lkml.org/lkml/2010/4/6/172 ), but never seemingly
> made it upstream. Resending for comments. -jstultz]
> 
> Signed-off-by: John Stultz <john.stultz@linaro.org>

Whoops, should have spotted it slipped through. FWIW, I'm still happy
with my Ack being stuck onto it.

Thanks.

-- 
Mel Gorman
SUSE Labs

Re: [PATCH] mm: Check if any page in a pageblock is reserved before marking it MIGRATE_RESERVE

[KOSAKI Motohiro]

> On Thu, Apr 21, 2011 at 06:34:03PM -0700, John Stultz wrote:
> > From: Arve Hjønnevåg <arve@android.com>
> > 
> > This fixes a problem where the first pageblock got marked MIGRATE_RESERVE even
> > though it only had a few free pages. This in turn caused no contiguous memory
> > to be reserved and frequent kswapd wakeups that emptied the caches to get more
> > contiguous memory.
> > 
> > CC: Dave Hansen <dave@linux.vnet.ibm.com>
> > CC: Mel Gorman <mgorman@suse.de>
> > CC: Andrew Morton <akpm@linux-foundation.org>
> > Signed-off-by: Arve Hjønnevåg <arve@android.com>
> > Acked-by: Mel Gorman <mel@csn.ul.ie>
> > 
> > [This patch was submitted and acked a little over a year ago
> > (see: http://lkml.org/lkml/2010/4/6/172 ), but never seemingly
> > made it upstream. Resending for comments. -jstultz]
> > 
> > Signed-off-by: John Stultz <john.stultz@linaro.org>
> 
> Whoops, should have spotted it slipped through. FWIW, I'm still happy
> with my Ack being stuck onto it.

Hehe, No.

You acked another patch at last year and John taked up old one. Sigh.
Look,  correct one has pfn_valid_within(). 
	http://lkml.org/lkml/2010/4/6/172

And, Minchan suggested to add more explanation to the description. Then, I think
following is desiable one.



Subject: [PATCH] mm: Check if any page in a pageblock is reserved before marking it MIGRATE_RESERVE
From: Arve Hjonnevag <arve@android.com>

This fixes a problem where the first pageblock got marked MIGRATE_RESERVE even
though it only had a few free pages. eg, On current ARM port, The kernel starts
at offset 0x8000 to leave room for boot parameters, and the memory is freed later.

This in turn caused no contiguous memory to be reserved and frequent kswapd
wakeups that emptied the caches to get more contiguous memory.

Unfortunatelly, ARM need order-2 allocation for pgd (see arm/mm/pgd.c#pgd_alloc()).
Therefore the issue is not minor nor easy avoidable.

CC: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Arve Hjonnevag <arve@android.com>
Acked-by: Mel Gorman <mel@csn.ul.ie>
Acked-by: Dave Hansen <dave@linux.vnet.ibm.com>
Signed-off-by: John Stultz <john.stultz@linaro.org>
Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com> [added a
few explanation]
---
 mm/page_alloc.c |   16 +++++++++++++++-
 1 files changed, 15 insertions(+), 1 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 1d5c189..10d9fa7 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -3282,6 +3282,20 @@ static inline unsigned long wait_table_bits(unsigned long size)
 #define LONG_ALIGN(x) (((x)+(sizeof(long))-1)&~((sizeof(long))-1))
 
 /*
+ * Check if a pageblock contains reserved pages
+ */
+static int pageblock_is_reserved(unsigned long start_pfn)
+{
+	unsigned long end_pfn = start_pfn + pageblock_nr_pages;
+	unsigned long pfn;
+
+	for (pfn = start_pfn; pfn < end_pfn; pfn++)
+		if (!pfn_valid_within(pfn) || PageReserved(pfn_to_page(pfn)))
+			return 1;
+	return 0;
+}
+
+/*
  * Mark a number of pageblocks as MIGRATE_RESERVE. The number
  * of blocks reserved is based on min_wmark_pages(zone). The memory within
  * the reserve will tend to store contiguous free pages. Setting min_free_kbytes
@@ -3320,7 +3334,7 @@ static void setup_zone_migrate_reserve(struct zone *zone)
 			continue;
 
 		/* Blocks with reserved pages will never free, skip them. */
-		if (PageReserved(page))
+		if (pageblock_is_reserved(pfn))
 			continue;
 
 		block_migratetype = get_pageblock_migratetype(page);
-- 
1.7.3.1

Re: [PATCH] mm: Check if any page in a pageblock is reserved before marking it MIGRATE_RESERVE

[Mel Gorman]

On Tue, Apr 26, 2011 at 06:49:39PM +0900, KOSAKI Motohiro wrote:
> > On Thu, Apr 21, 2011 at 06:34:03PM -0700, John Stultz wrote:
> > > From: Arve Hjønnevåg <arve@android.com>
> > > 
> > > This fixes a problem where the first pageblock got marked MIGRATE_RESERVE even
> > > though it only had a few free pages. This in turn caused no contiguous memory
> > > to be reserved and frequent kswapd wakeups that emptied the caches to get more
> > > contiguous memory.
> > > 
> > > CC: Dave Hansen <dave@linux.vnet.ibm.com>
> > > CC: Mel Gorman <mgorman@suse.de>
> > > CC: Andrew Morton <akpm@linux-foundation.org>
> > > Signed-off-by: Arve Hjønnevåg <arve@android.com>
> > > Acked-by: Mel Gorman <mel@csn.ul.ie>
> > > 
> > > [This patch was submitted and acked a little over a year ago
> > > (see: http://lkml.org/lkml/2010/4/6/172 ), but never seemingly
> > > made it upstream. Resending for comments. -jstultz]
> > > 
> > > Signed-off-by: John Stultz <john.stultz@linaro.org>
> > 
> > Whoops, should have spotted it slipped through. FWIW, I'm still happy
> > with my Ack being stuck onto it.
> 
> Hehe, No.
> 
> You acked another patch at last year and John taked up old one. Sigh.
> Look,  correct one has pfn_valid_within(). 
> 	http://lkml.org/lkml/2010/4/6/172
> 

Bah, you're right thanks for catching that. A pfn_valid_within check is
indeed required, particularly on ARM where there can be holes punched within
pageblock boundaries. Thanks

> 
> Subject: [PATCH] mm: Check if any page in a pageblock is reserved before marking it MIGRATE_RESERVE
> From: Arve Hjonnevag <arve@android.com>
> 
> This fixes a problem where the first pageblock got marked MIGRATE_RESERVE even
> though it only had a few free pages. eg, On current ARM port, The kernel starts
> at offset 0x8000 to leave room for boot parameters, and the memory is freed later.
> 
> This in turn caused no contiguous memory to be reserved and frequent kswapd
> wakeups that emptied the caches to get more contiguous memory.
> 
> Unfortunatelly, ARM need order-2 allocation for pgd (see arm/mm/pgd.c#pgd_alloc()).
> Therefore the issue is not minor nor easy avoidable.
> 
> CC: Andrew Morton <akpm@linux-foundation.org>
> Signed-off-by: Arve Hjonnevag <arve@android.com>
> Acked-by: Mel Gorman <mel@csn.ul.ie>
> Acked-by: Dave Hansen <dave@linux.vnet.ibm.com>
> Signed-off-by: John Stultz <john.stultz@linaro.org>
> Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com> [added a
> few explanation]
> ---
>  mm/page_alloc.c |   16 +++++++++++++++-
>  1 files changed, 15 insertions(+), 1 deletions(-)
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 1d5c189..10d9fa7 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -3282,6 +3282,20 @@ static inline unsigned long wait_table_bits(unsigned long size)
>  #define LONG_ALIGN(x) (((x)+(sizeof(long))-1)&~((sizeof(long))-1))
>  
>  /*
> + * Check if a pageblock contains reserved pages
> + */
> +static int pageblock_is_reserved(unsigned long start_pfn)
> +{
> +	unsigned long end_pfn = start_pfn + pageblock_nr_pages;
> +	unsigned long pfn;
> +
> +	for (pfn = start_pfn; pfn < end_pfn; pfn++)
> +		if (!pfn_valid_within(pfn) || PageReserved(pfn_to_page(pfn)))
> +			return 1;
> +	return 0;
> +}
> +
> +/*
>   * Mark a number of pageblocks as MIGRATE_RESERVE. The number
>   * of blocks reserved is based on min_wmark_pages(zone). The memory within
>   * the reserve will tend to store contiguous free pages. Setting min_free_kbytes
> @@ -3320,7 +3334,7 @@ static void setup_zone_migrate_reserve(struct zone *zone)
>  			continue;
>  
>  		/* Blocks with reserved pages will never free, skip them. */
> -		if (PageReserved(page))
> +		if (pageblock_is_reserved(pfn))
>  			continue;
>  
>  		block_migratetype = get_pageblock_migratetype(page);

-- 
Mel Gorman
SUSE Labs

Re: [PATCH] mm: Check if any page in a pageblock is reserved before marking it MIGRATE_RESERVE

[John Stultz]

On Tue, 2011-04-26 at 18:49 +0900, KOSAKI Motohiro wrote:
> > On Thu, Apr 21, 2011 at 06:34:03PM -0700, John Stultz wrote:
> > > From: Arve Hjønnevåg <arve@android.com>
> > > 
> > > This fixes a problem where the first pageblock got marked MIGRATE_RESERVE even
> > > though it only had a few free pages. This in turn caused no contiguous memory
> > > to be reserved and frequent kswapd wakeups that emptied the caches to get more
> > > contiguous memory.
> > > 
> > > CC: Dave Hansen <dave@linux.vnet.ibm.com>
> > > CC: Mel Gorman <mgorman@suse.de>
> > > CC: Andrew Morton <akpm@linux-foundation.org>
> > > Signed-off-by: Arve Hjønnevåg <arve@android.com>
> > > Acked-by: Mel Gorman <mel@csn.ul.ie>
> > > 
> > > [This patch was submitted and acked a little over a year ago
> > > (see: http://lkml.org/lkml/2010/4/6/172 ), but never seemingly
> > > made it upstream. Resending for comments. -jstultz]
> > > 
> > > Signed-off-by: John Stultz <john.stultz@linaro.org>
> > 
> > Whoops, should have spotted it slipped through. FWIW, I'm still happy
> > with my Ack being stuck onto it.
> 
> Hehe, No.
> 
> You acked another patch at last year and John taked up old one. Sigh.
> Look,  correct one has pfn_valid_within(). 
> 	http://lkml.org/lkml/2010/4/6/172

Oh yikes! Many thanks for noticing that detail! Indeed, I started with
the patch in the Android tree, and didn't notice the difference in the
discussion I linked to. My apologies.

> And, Minchan suggested to add more explanation to the description. Then, I think
> following is desiable one.

Thanks so much again!
-john