[PATCH v3 0/4] memory-hotplug: handle page race between allocation and isolation

[PATCH v3 0/4] memory-hotplug: handle page race between allocation and isolation

[Minchan Kim]

Memory hotplug has a subtle race problem so this patchset fixes the problem
(Look at [3/3] for detail and please confirm the problem before review
other patches in this series.)

 [1/4] is just clean up and help for [2/4].
 [2/4] keeps the migratetype information to freed page's index field
       and [3/4] uses the information.
 [3/4] fixes the race problem with [2/4]'s information.
 [4/4] enhance memory-hotremove operation success ratio

After applying [2/4], migratetype argument in __free_one_page
and free_one_page is redundant so we can remove it but I decide
to not touch them because it increases code size about 50 byte.

This patchset is based on mmotm-2012-09-06-16-46

Minchan Kim (4):
  use get_page_migratetype instead of page_private
  mm: remain migratetype in freed page
  memory-hotplug: bug fix race between isolation and allocation
  memory-hotplug: fix pages missed by race rather than failing

 include/linux/mm.h             |   12 ++++++++++++
 include/linux/page-isolation.h |    4 ++++
 mm/page_alloc.c                |   19 ++++++++++++-------
 mm/page_isolation.c            |   18 ++++++++++++++++--
 4 files changed, 44 insertions(+), 9 deletions(-)

-- 
1.7.9.5

[PATCH v3 1/4] use get_freepage_migratetype instead of page_private

[Minchan Kim]

page allocator uses set_page_private and page_private for handling
migratetype when it frees page. Let's replace them with [set|get]
_freepage_migratetype to make it more clear.

* from v2
  * Add Acked-by of Kame

* from v1
  * Change set_page_migratetype with set_freepage_migratetype
  * Add comment on set_freepage_migratetype

Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Signed-off-by: Minchan Kim <minchan@kernel.org>
---
 include/linux/mm.h  |   12 ++++++++++++
 mm/page_alloc.c     |   10 ++++++----
 mm/page_isolation.c |    2 +-
 3 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 0514fe9..84d1663f 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -237,6 +237,18 @@ struct inode;
 #define page_private(page)		((page)->private)
 #define set_page_private(page, v)	((page)->private = (v))
 
+/* It's valid only if the page is free path or free_list */
+static inline void set_freepage_migratetype(struct page *page, int migratetype)
+{
+	set_page_private(page, migratetype);
+}
+
+/* It's valid only if the page is free path or free_list */
+static inline int get_freepage_migratetype(struct page *page)
+{
+	return page_private(page);
+}
+
 /*
  * FIXME: take this include out, include page-flags.h in
  * files which need it (119 of them)
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index ba3100a..f5ba236 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -671,8 +671,10 @@ static void free_pcppages_bulk(struct zone *zone, int count,
 			/* must delete as __free_one_page list manipulates */
 			list_del(&page->lru);
 			/* MIGRATE_MOVABLE list may include MIGRATE_RESERVEs */
-			__free_one_page(page, zone, 0, page_private(page));
-			trace_mm_page_pcpu_drain(page, 0, page_private(page));
+			__free_one_page(page, zone, 0,
+				get_freepage_migratetype(page));
+			trace_mm_page_pcpu_drain(page, 0,
+				get_freepage_migratetype(page));
 		} while (--to_free && --batch_free && !list_empty(list));
 	}
 	__mod_zone_page_state(zone, NR_FREE_PAGES, count);
@@ -1134,7 +1136,7 @@ static int rmqueue_bulk(struct zone *zone, unsigned int order,
 			if (!is_migrate_cma(mt) && mt != MIGRATE_ISOLATE)
 				mt = migratetype;
 		}
-		set_page_private(page, mt);
+		set_freepage_migratetype(page, mt);
 		list = &page->lru;
 	}
 	__mod_zone_page_state(zone, NR_FREE_PAGES, -(i << order));
@@ -1301,7 +1303,7 @@ void free_hot_cold_page(struct page *page, int cold)
 		return;
 
 	migratetype = get_pageblock_migratetype(page);
-	set_page_private(page, migratetype);
+	set_freepage_migratetype(page, migratetype);
 	local_irq_save(flags);
 	if (unlikely(wasMlocked))
 		free_page_mlock(page);
diff --git a/mm/page_isolation.c b/mm/page_isolation.c
index 247d1f1..87a7929 100644
--- a/mm/page_isolation.c
+++ b/mm/page_isolation.c
@@ -196,7 +196,7 @@ __test_page_isolated_in_pageblock(unsigned long pfn, unsigned long end_pfn)
 		if (PageBuddy(page))
 			pfn += 1 << page_order(page);
 		else if (page_count(page) == 0 &&
-				page_private(page) == MIGRATE_ISOLATE)
+			get_freepage_migratetype(page) == MIGRATE_ISOLATE)
 			pfn += 1;
 		else
 			break;
-- 
1.7.9.5

Re: [PATCH v3 1/4] use get_freepage_migratetype instead of page_private

[Yasuaki Ishimatsu]

2012/09/07 9:39, Minchan Kim wrote:
> page allocator uses set_page_private and page_private for handling
> migratetype when it frees page. Let's replace them with [set|get]
> _freepage_migratetype to make it more clear.
> 
> * from v2
>    * Add Acked-by of Kame
> 
> * from v1
>    * Change set_page_migratetype with set_freepage_migratetype
>    * Add comment on set_freepage_migratetype
> 
> Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> Signed-off-by: Minchan Kim <minchan@kernel.org>

Reviewed-by: Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com>

Thanks,
Yasuaki Ishimatsu

> ---
>   include/linux/mm.h  |   12 ++++++++++++
>   mm/page_alloc.c     |   10 ++++++----
>   mm/page_isolation.c |    2 +-
>   3 files changed, 19 insertions(+), 5 deletions(-)
> 
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 0514fe9..84d1663f 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -237,6 +237,18 @@ struct inode;
>   #define page_private(page)		((page)->private)
>   #define set_page_private(page, v)	((page)->private = (v))
>   
> +/* It's valid only if the page is free path or free_list */
> +static inline void set_freepage_migratetype(struct page *page, int migratetype)
> +{
> +	set_page_private(page, migratetype);
> +}
> +
> +/* It's valid only if the page is free path or free_list */
> +static inline int get_freepage_migratetype(struct page *page)
> +{
> +	return page_private(page);
> +}
> +
>   /*
>    * FIXME: take this include out, include page-flags.h in
>    * files which need it (119 of them)
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index ba3100a..f5ba236 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -671,8 +671,10 @@ static void free_pcppages_bulk(struct zone *zone, int count,
>   			/* must delete as __free_one_page list manipulates */
>   			list_del(&page->lru);
>   			/* MIGRATE_MOVABLE list may include MIGRATE_RESERVEs */
> -			__free_one_page(page, zone, 0, page_private(page));
> -			trace_mm_page_pcpu_drain(page, 0, page_private(page));
> +			__free_one_page(page, zone, 0,
> +				get_freepage_migratetype(page));
> +			trace_mm_page_pcpu_drain(page, 0,
> +				get_freepage_migratetype(page));
>   		} while (--to_free && --batch_free && !list_empty(list));
>   	}
>   	__mod_zone_page_state(zone, NR_FREE_PAGES, count);
> @@ -1134,7 +1136,7 @@ static int rmqueue_bulk(struct zone *zone, unsigned int order,
>   			if (!is_migrate_cma(mt) && mt != MIGRATE_ISOLATE)
>   				mt = migratetype;
>   		}
> -		set_page_private(page, mt);
> +		set_freepage_migratetype(page, mt);
>   		list = &page->lru;
>   	}
>   	__mod_zone_page_state(zone, NR_FREE_PAGES, -(i << order));
> @@ -1301,7 +1303,7 @@ void free_hot_cold_page(struct page *page, int cold)
>   		return;
>   
>   	migratetype = get_pageblock_migratetype(page);
> -	set_page_private(page, migratetype);
> +	set_freepage_migratetype(page, migratetype);
>   	local_irq_save(flags);
>   	if (unlikely(wasMlocked))
>   		free_page_mlock(page);
> diff --git a/mm/page_isolation.c b/mm/page_isolation.c
> index 247d1f1..87a7929 100644
> --- a/mm/page_isolation.c
> +++ b/mm/page_isolation.c
> @@ -196,7 +196,7 @@ __test_page_isolated_in_pageblock(unsigned long pfn, unsigned long end_pfn)
>   		if (PageBuddy(page))
>   			pfn += 1 << page_order(page);
>   		else if (page_count(page) == 0 &&
> -				page_private(page) == MIGRATE_ISOLATE)
> +			get_freepage_migratetype(page) == MIGRATE_ISOLATE)
>   			pfn += 1;
>   		else
>   			break;
> 

[PATCH v3 2/4] mm: remain migratetype in freed page

[Minchan Kim]

The page allocator caches the pageblock information in page->private while
it is in the PCP freelists but this is overwritten with the order of the
page when freed to the buddy allocator. This patch stores the migratetype
of the page in the page->index field so that it is available at all times
when the page remain in free_list.

This patch adds a new call site in __free_pages_ok so it might be
overhead a bit but it's for high order allocation.
So I believe damage isn't hurt.

* from v2
  * Add Acked-by of Kame

* from v1
  * Fix move_freepages's migratetype - Mel
  * Add more kind explanation in description - Mel

Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Signed-off-by: Minchan Kim <minchan@kernel.org>
---
 include/linux/mm.h |    4 ++--
 mm/page_alloc.c    |    7 +++++--
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 84d1663f..68f9e8d 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -240,13 +240,13 @@ struct inode;
 /* It's valid only if the page is free path or free_list */
 static inline void set_freepage_migratetype(struct page *page, int migratetype)
 {
-	set_page_private(page, migratetype);
+	page->index = migratetype;
 }
 
 /* It's valid only if the page is free path or free_list */
 static inline int get_freepage_migratetype(struct page *page)
 {
-	return page_private(page);
+	return page->index;
 }
 
 /*
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index f5ba236..8531fa3 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -723,6 +723,7 @@ static void __free_pages_ok(struct page *page, unsigned int order)
 {
 	unsigned long flags;
 	int wasMlocked = __TestClearPageMlocked(page);
+	int migratetype;
 
 	if (!free_pages_prepare(page, order))
 		return;
@@ -731,8 +732,9 @@ static void __free_pages_ok(struct page *page, unsigned int order)
 	if (unlikely(wasMlocked))
 		free_page_mlock(page);
 	__count_vm_events(PGFREE, 1 << order);
-	free_one_page(page_zone(page), page, order,
-					get_pageblock_migratetype(page));
+	migratetype = get_pageblock_migratetype(page);
+	set_freepage_migratetype(page, migratetype);
+	free_one_page(page_zone(page), page, order, migratetype);
 	local_irq_restore(flags);
 }
 
@@ -952,6 +954,7 @@ static int move_freepages(struct zone *zone,
 		order = page_order(page);
 		list_move(&page->lru,
 			  &zone->free_area[order].free_list[migratetype]);
+		set_freepage_migratetype(page, migratetype);
 		page += 1 << order;
 		pages_moved += 1 << order;
 	}
-- 
1.7.9.5

Re: [PATCH v3 2/4] mm: remain migratetype in freed page

[Yasuaki Ishimatsu]

2012/09/07 9:39, Minchan Kim wrote:
> The page allocator caches the pageblock information in page->private while
> it is in the PCP freelists but this is overwritten with the order of the
> page when freed to the buddy allocator. This patch stores the migratetype
> of the page in the page->index field so that it is available at all times
> when the page remain in free_list.
> 
> This patch adds a new call site in __free_pages_ok so it might be
> overhead a bit but it's for high order allocation.
> So I believe damage isn't hurt.
> 
> * from v2
>    * Add Acked-by of Kame
> 
> * from v1
>    * Fix move_freepages's migratetype - Mel
>    * Add more kind explanation in description - Mel
> 
> Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> Signed-off-by: Minchan Kim <minchan@kernel.org>

Reviewed-by: Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com>

Thanks,
Yasuaki Ishimatsu

> ---
>   include/linux/mm.h |    4 ++--
>   mm/page_alloc.c    |    7 +++++--
>   2 files changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 84d1663f..68f9e8d 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -240,13 +240,13 @@ struct inode;
>   /* It's valid only if the page is free path or free_list */
>   static inline void set_freepage_migratetype(struct page *page, int migratetype)
>   {
> -	set_page_private(page, migratetype);
> +	page->index = migratetype;
>   }
>   
>   /* It's valid only if the page is free path or free_list */
>   static inline int get_freepage_migratetype(struct page *page)
>   {
> -	return page_private(page);
> +	return page->index;
>   }
>   
>   /*
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index f5ba236..8531fa3 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -723,6 +723,7 @@ static void __free_pages_ok(struct page *page, unsigned int order)
>   {
>   	unsigned long flags;
>   	int wasMlocked = __TestClearPageMlocked(page);
> +	int migratetype;
>   
>   	if (!free_pages_prepare(page, order))
>   		return;
> @@ -731,8 +732,9 @@ static void __free_pages_ok(struct page *page, unsigned int order)
>   	if (unlikely(wasMlocked))
>   		free_page_mlock(page);
>   	__count_vm_events(PGFREE, 1 << order);
> -	free_one_page(page_zone(page), page, order,
> -					get_pageblock_migratetype(page));
> +	migratetype = get_pageblock_migratetype(page);
> +	set_freepage_migratetype(page, migratetype);
> +	free_one_page(page_zone(page), page, order, migratetype);
>   	local_irq_restore(flags);
>   }
>   
> @@ -952,6 +954,7 @@ static int move_freepages(struct zone *zone,
>   		order = page_order(page);
>   		list_move(&page->lru,
>   			  &zone->free_area[order].free_list[migratetype]);
> +		set_freepage_migratetype(page, migratetype);
>   		page += 1 << order;
>   		pages_moved += 1 << order;
>   	}
> 

[PATCH v3 3/4] memory-hotplug: bug fix race between isolation and allocation

[Minchan Kim]

Like below, memory-hotplug makes race between page-isolation
and page-allocation so it can hit BUG_ON in __offline_isolated_pages.

	CPU A					CPU B

start_isolate_page_range
set_migratetype_isolate
spin_lock_irqsave(zone->lock)

				free_hot_cold_page(Page A)
				/* without zone->lock */
				migratetype = get_pageblock_migratetype(Page A);
				/*
				 * Page could be moved into MIGRATE_MOVABLE
				 * of per_cpu_pages
				 */
				list_add_tail(&page->lru, &pcp->lists[migratetype]);

set_pageblock_isolate
move_freepages_block
drain_all_pages

				/* Page A could be in MIGRATE_MOVABLE of free_list. */

check_pages_isolated
__test_page_isolated_in_pageblock
/*
 * We can't catch freed page which
 * is free_list[MIGRATE_MOVABLE]
 */
if (PageBuddy(page A))
	pfn += 1 << page_order(page A);

				/* So, Page A could be allocated */

__offline_isolated_pages
/*
 * BUG_ON hit or offline page
 * which is used by someone
 */
BUG_ON(!PageBuddy(page A));

This patch checks page's migratetype in freelist in __test_page_isolated_in_pageblock.
So now __test_page_isolated_in_pageblock can check the page caused by above race and
can fail of memory offlining.

* from v2
  * Add Acked-by of Kame

Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Signed-off-by: Minchan Kim <minchan@kernel.org>
---
 mm/page_isolation.c |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/mm/page_isolation.c b/mm/page_isolation.c
index 87a7929..7ba7405 100644
--- a/mm/page_isolation.c
+++ b/mm/page_isolation.c
@@ -193,8 +193,11 @@ __test_page_isolated_in_pageblock(unsigned long pfn, unsigned long end_pfn)
 			continue;
 		}
 		page = pfn_to_page(pfn);
-		if (PageBuddy(page))
+		if (PageBuddy(page)) {
+			if (get_freepage_migratetype(page) != MIGRATE_ISOLATE)
+				break;
 			pfn += 1 << page_order(page);
+		}
 		else if (page_count(page) == 0 &&
 			get_freepage_migratetype(page) == MIGRATE_ISOLATE)
 			pfn += 1;
-- 
1.7.9.5

Re: [PATCH v3 3/4] memory-hotplug: bug fix race between isolation and allocation

[Yasuaki Ishimatsu]

2012/09/07 9:39, Minchan Kim wrote:
> Like below, memory-hotplug makes race between page-isolation
> and page-allocation so it can hit BUG_ON in __offline_isolated_pages.
> 
> 	CPU A					CPU B
> 
> start_isolate_page_range
> set_migratetype_isolate
> spin_lock_irqsave(zone->lock)
> 
> 				free_hot_cold_page(Page A)
> 				/* without zone->lock */
> 				migratetype = get_pageblock_migratetype(Page A);
> 				/*
> 				 * Page could be moved into MIGRATE_MOVABLE
> 				 * of per_cpu_pages
> 				 */
> 				list_add_tail(&page->lru, &pcp->lists[migratetype]);
> 
> set_pageblock_isolate
> move_freepages_block
> drain_all_pages
> 
> 				/* Page A could be in MIGRATE_MOVABLE of free_list. */
> 
> check_pages_isolated
> __test_page_isolated_in_pageblock
> /*
>   * We can't catch freed page which
>   * is free_list[MIGRATE_MOVABLE]
>   */
> if (PageBuddy(page A))
> 	pfn += 1 << page_order(page A);
> 
> 				/* So, Page A could be allocated */
> 
> __offline_isolated_pages
> /*
>   * BUG_ON hit or offline page
>   * which is used by someone
>   */
> BUG_ON(!PageBuddy(page A));
> 
> This patch checks page's migratetype in freelist in __test_page_isolated_in_pageblock.
> So now __test_page_isolated_in_pageblock can check the page caused by above race and
> can fail of memory offlining.
> 
> * from v2
>    * Add Acked-by of Kame
> 
> Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> Signed-off-by: Minchan Kim <minchan@kernel.org>

Reviewed-by: Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com>

Thanks,
Yasuaki Ishimatsu

> ---
>   mm/page_isolation.c |    5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/page_isolation.c b/mm/page_isolation.c
> index 87a7929..7ba7405 100644
> --- a/mm/page_isolation.c
> +++ b/mm/page_isolation.c
> @@ -193,8 +193,11 @@ __test_page_isolated_in_pageblock(unsigned long pfn, unsigned long end_pfn)
>   			continue;
>   		}
>   		page = pfn_to_page(pfn);
> -		if (PageBuddy(page))
> +		if (PageBuddy(page)) {
> +			if (get_freepage_migratetype(page) != MIGRATE_ISOLATE)
> +				break;
>   			pfn += 1 << page_order(page);
> +		}
>   		else if (page_count(page) == 0 &&
>   			get_freepage_migratetype(page) == MIGRATE_ISOLATE)
>   			pfn += 1;
> 

[PATCH v3 4/4] memory-hotplug: fix pages missed by race rather than failng

[Minchan Kim]

If race between allocation and isolation in memory-hotplug offline
happens, some pages could be in MIGRATE_MOVABLE of free_list although
the pageblock's migratetype of the page is MIGRATE_ISOLATE.

The race could be detected by get_freepage_migratetype
in __test_page_isolated_in_pageblock. If it is detected, now EBUSY
gets bubbled all the way up and the hotplug operations fails.

But better idea is instead of returning and failing memory-hotremove,
move the free page to the correct list at the time it is detected.
It could enhance memory-hotremove operation success ratio although
the race is really rare.

Suggested-by: Mel Gorman <mgorman@suse.de>
Signed-off-by: Minchan Kim <minchan@kernel.org>
---
 include/linux/page-isolation.h |    4 ++++
 mm/page_alloc.c                |    2 +-
 mm/page_isolation.c            |   15 +++++++++++++--
 3 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/include/linux/page-isolation.h b/include/linux/page-isolation.h
index 105077a..fca8c0a 100644
--- a/include/linux/page-isolation.h
+++ b/include/linux/page-isolation.h
@@ -6,6 +6,10 @@ bool has_unmovable_pages(struct zone *zone, struct page *page, int count);
 void set_pageblock_migratetype(struct page *page, int migratetype);
 int move_freepages_block(struct zone *zone, struct page *page,
 				int migratetype);
+int move_freepages(struct zone *zone,
+			  struct page *start_page, struct page *end_page,
+			  int migratetype);
+
 /*
  * Changes migrate type in [start_pfn, end_pfn) to be MIGRATE_ISOLATE.
  * If specified range includes migrate types other than MOVABLE or CMA,
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 8531fa3..5a4c4d8 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -918,7 +918,7 @@ static int fallbacks[MIGRATE_TYPES][4] = {
  * Note that start_page and end_pages are not aligned on a pageblock
  * boundary. If alignment is required, use move_freepages_block()
  */
-static int move_freepages(struct zone *zone,
+int move_freepages(struct zone *zone,
 			  struct page *start_page, struct page *end_page,
 			  int migratetype)
 {
diff --git a/mm/page_isolation.c b/mm/page_isolation.c
index 7ba7405..a42fa8d 100644
--- a/mm/page_isolation.c
+++ b/mm/page_isolation.c
@@ -194,8 +194,19 @@ __test_page_isolated_in_pageblock(unsigned long pfn, unsigned long end_pfn)
 		}
 		page = pfn_to_page(pfn);
 		if (PageBuddy(page)) {
-			if (get_freepage_migratetype(page) != MIGRATE_ISOLATE)
-				break;
+			/*
+			 * If race between isolatation and allocation happens,
+			 * some free pages could be in MIGRATE_MOVABLE list
+			 * although pageblock's migratation type of the page
+			 * is MIGRATE_ISOLATE. Catch it and move the page into
+			 * MIGRATE_ISOLATE list.
+			 */
+			if (get_freepage_migratetype(page) != MIGRATE_ISOLATE) {
+				struct page *end_page = page +
+						(1 << page_order(page)) - 1;
+				move_freepages(page_zone(page), page, end_page,
+						MIGRATE_ISOLATE);
+			}
 			pfn += 1 << page_order(page);
 		}
 		else if (page_count(page) == 0 &&
-- 
1.7.9.5

Re: [PATCH v3 4/4] memory-hotplug: fix pages missed by race rather than failng

[Kamezawa Hiroyuki]

(2012/09/07 9:39), Minchan Kim wrote:
> If race between allocation and isolation in memory-hotplug offline
> happens, some pages could be in MIGRATE_MOVABLE of free_list although
> the pageblock's migratetype of the page is MIGRATE_ISOLATE.
> 
> The race could be detected by get_freepage_migratetype
> in __test_page_isolated_in_pageblock. If it is detected, now EBUSY
> gets bubbled all the way up and the hotplug operations fails.
> 
> But better idea is instead of returning and failing memory-hotremove,
> move the free page to the correct list at the time it is detected.
> It could enhance memory-hotremove operation success ratio although
> the race is really rare.
> 
> Suggested-by: Mel Gorman <mgorman@suse.de>
> Signed-off-by: Minchan Kim <minchan@kernel.org>

Sounds reasonable. Thank you very much !

Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>

Re: [PATCH v3 0/4] memory-hotplug: handle page race between allocation and isolation

[Mel Gorman]

On Fri, Sep 07, 2012 at 09:39:28AM +0900, Minchan Kim wrote:
> Memory hotplug has a subtle race problem so this patchset fixes the problem
> (Look at [3/3] for detail and please confirm the problem before review
> other patches in this series.)
> 
>  [1/4] is just clean up and help for [2/4].
>  [2/4] keeps the migratetype information to freed page's index field
>        and [3/4] uses the information.
>  [3/4] fixes the race problem with [2/4]'s information.
>  [4/4] enhance memory-hotremove operation success ratio
> 
> After applying [2/4], migratetype argument in __free_one_page
> and free_one_page is redundant so we can remove it but I decide
> to not touch them because it increases code size about 50 byte.
> 
> This patchset is based on mmotm-2012-09-06-16-46
> 

Nothing jumped out and poked me in the eye so for the series;

Acked-by: Mel Gorman <mgorman@suse.de>

Thanks!

-- 
Mel Gorman
SUSE Labs