linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] memory_hotplug: always initialize pageblock bitmap.
@ 2008-05-09  6:06 Heiko Carstens
  2008-05-09  6:39 ` KAMEZAWA Hiroyuki
  0 siblings, 1 reply; 9+ messages in thread
From: Heiko Carstens @ 2008-05-09  6:06 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Andy Whitcroft, Dave Hansen, Gerald Schaefer, linux-kernel, linux-mm

From: Heiko Carstens <heiko.carstens@de.ibm.com>

Trying to online a new memory section that was added via memory hotplug
sometimes results in crashes when the new pages are added via
__free_page. Reason for that is that the pageblock bitmap isn't
initialized and hence contains random stuff.
That means that get_pageblock_migratetype() returns also random stuff
and therefore

	list_add(&page->lru,
		 &zone->free_area[order].free_list[migratetype]);

in __free_one_page() tries to do a list_add to something that isn't
even necessarily a list.
This is only an issue for memory sections that get added after boot
time since all previously present memory sections allocate their
pageblock bitmaps via the bootmem allocator which in turn initializes
just everything it returns.

Cc: Andy Whitcroft <apw@shadowen.org>
Cc: Dave Hansen <haveblue@us.ibm.com>
Cc: Gerald Schaefer <gerald.schaefer@de.ibm.com>
Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
---
 mm/sparse.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux-2.6/mm/sparse.c
===================================================================
--- linux-2.6.orig/mm/sparse.c
+++ linux-2.6/mm/sparse.c
@@ -244,7 +244,7 @@ unsigned long usemap_size(void)
 #ifdef CONFIG_MEMORY_HOTPLUG
 static unsigned long *__kmalloc_section_usemap(void)
 {
-	return kmalloc(usemap_size(), GFP_KERNEL);
+	return kzalloc(usemap_size(), GFP_KERNEL);
 }
 #endif /* CONFIG_MEMORY_HOTPLUG */
 

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

* Re: [PATCH] memory_hotplug: always initialize pageblock bitmap.
  2008-05-09  6:06 [PATCH] memory_hotplug: always initialize pageblock bitmap Heiko Carstens
@ 2008-05-09  6:39 ` KAMEZAWA Hiroyuki
  2008-05-09  6:45   ` Heiko Carstens
  2008-05-10 12:45   ` Heiko Carstens
  0 siblings, 2 replies; 9+ messages in thread
From: KAMEZAWA Hiroyuki @ 2008-05-09  6:39 UTC (permalink / raw)
  To: Heiko Carstens
  Cc: Andrew Morton, Andy Whitcroft, Dave Hansen, Gerald Schaefer,
	linux-kernel, linux-mm

On Fri, 9 May 2008 08:06:09 +0200
Heiko Carstens <heiko.carstens@de.ibm.com> wrote:

> From: Heiko Carstens <heiko.carstens@de.ibm.com>
> 
> Trying to online a new memory section that was added via memory hotplug
> sometimes results in crashes when the new pages are added via
> __free_page. Reason for that is that the pageblock bitmap isn't
> initialized and hence contains random stuff.

Hmm, curious. In my understanding, memmap_init_zone() initializes it.

 __add_pages()
	-> __add_section()
		-> sparse-add_one_section() // allocate usemap
		-> __add_zone()
			-> memmap_init_zone() // reset pageblock's bitmap 

Can't memmap_init_zone() does proper initialization ?
........................
Ah, ok. I see. grow_zone_span() is not called at __add_zone(), then,
memmap_init_zone() doesn't initialize usemap because memmap is not in zone's
range.

Recently, I added a check "zone's start_pfn < pfn < zone's end"
to memmap_init_zone()'s usemap initialization for !SPARSEMEM case bug FIX.
(and I think the fix itself is sane.)

How about calling grow_pgdat_span()/grow_zone_span() from __add_zone() ?

Thanks,
-Kame


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

* Re: [PATCH] memory_hotplug: always initialize pageblock bitmap.
  2008-05-09  6:39 ` KAMEZAWA Hiroyuki
@ 2008-05-09  6:45   ` Heiko Carstens
  2008-05-10 12:45   ` Heiko Carstens
  1 sibling, 0 replies; 9+ messages in thread
From: Heiko Carstens @ 2008-05-09  6:45 UTC (permalink / raw)
  To: KAMEZAWA Hiroyuki
  Cc: Andrew Morton, Andy Whitcroft, Dave Hansen, Gerald Schaefer,
	linux-kernel, linux-mm

On Fri, May 09, 2008 at 03:39:10PM +0900, KAMEZAWA Hiroyuki wrote:
> On Fri, 9 May 2008 08:06:09 +0200
> Heiko Carstens <heiko.carstens@de.ibm.com> wrote:
> 
> > From: Heiko Carstens <heiko.carstens@de.ibm.com>
> > 
> > Trying to online a new memory section that was added via memory hotplug
> > sometimes results in crashes when the new pages are added via
> > __free_page. Reason for that is that the pageblock bitmap isn't
> > initialized and hence contains random stuff.
> 
> Hmm, curious. In my understanding, memmap_init_zone() initializes it.
> 
>  __add_pages()
> 	-> __add_section()
> 		-> sparse-add_one_section() // allocate usemap
> 		-> __add_zone()
> 			-> memmap_init_zone() // reset pageblock's bitmap 
> 
> Can't memmap_init_zone() does proper initialization ?

Well, it just _sets_ some bits. But nobody has initialized the bitmap
before to zero. It doesn't reset the pageblock's bitmap as your
comment would indicate.

> ........................
> Ah, ok. I see. grow_zone_span() is not called at __add_zone(), then,
> memmap_init_zone() doesn't initialize usemap because memmap is not in zone's
> range.
> 
> Recently, I added a check "zone's start_pfn < pfn < zone's end"
> to memmap_init_zone()'s usemap initialization for !SPARSEMEM case bug FIX.
> (and I think the fix itself is sane.)
> 
> How about calling grow_pgdat_span()/grow_zone_span() from __add_zone() ?

Dunno.. just fixed a few bugs to get it working.. somehow.. ;)

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

* Re: [PATCH] memory_hotplug: always initialize pageblock bitmap.
  2008-05-09  6:39 ` KAMEZAWA Hiroyuki
  2008-05-09  6:45   ` Heiko Carstens
@ 2008-05-10 12:45   ` Heiko Carstens
  2008-05-12  1:55     ` KAMEZAWA Hiroyuki
  1 sibling, 1 reply; 9+ messages in thread
From: Heiko Carstens @ 2008-05-10 12:45 UTC (permalink / raw)
  To: KAMEZAWA Hiroyuki
  Cc: Andrew Morton, Andy Whitcroft, Dave Hansen, Gerald Schaefer,
	linux-kernel, linux-mm

On Fri, May 09, 2008 at 03:39:10PM +0900, KAMEZAWA Hiroyuki wrote:
> On Fri, 9 May 2008 08:06:09 +0200
> Heiko Carstens <heiko.carstens@de.ibm.com> wrote:
> 
> > From: Heiko Carstens <heiko.carstens@de.ibm.com>
> > 
> > Trying to online a new memory section that was added via memory hotplug
> > sometimes results in crashes when the new pages are added via
> > __free_page. Reason for that is that the pageblock bitmap isn't
> > initialized and hence contains random stuff.
> 
> Hmm, curious. In my understanding, memmap_init_zone() initializes it.
> 
>  __add_pages()
> 	-> __add_section()
> 		-> sparse-add_one_section() // allocate usemap
> 		-> __add_zone()
> 			-> memmap_init_zone() // reset pageblock's bitmap 
> 
> Can't memmap_init_zone() does proper initialization ?
> ........................
> Ah, ok. I see. grow_zone_span() is not called at __add_zone(), then,
> memmap_init_zone() doesn't initialize usemap because memmap is not in zone's
> range.

Now I got it. So my patch was incorrect, it just worked around the crash.

> Recently, I added a check "zone's start_pfn < pfn < zone's end"
> to memmap_init_zone()'s usemap initialization for !SPARSEMEM case bug FIX.
> (and I think the fix itself is sane.)

Oh, you broke memory hot-add on -stable ;)

> How about calling grow_pgdat_span()/grow_zone_span() from __add_zone() ?

Like this?

Note that this patch is on top of the other patch I already sent, but
reverts it...

This works for me. A final version (if this is acceptable) should move
the grow* functions also.

---
 mm/memory_hotplug.c |   28 +++++++++++++++++++---------
 mm/page_alloc.c     |    3 +--
 2 files changed, 20 insertions(+), 11 deletions(-)

Index: linux-2.6/mm/memory_hotplug.c
===================================================================
--- linux-2.6.orig/mm/memory_hotplug.c
+++ linux-2.6/mm/memory_hotplug.c
@@ -159,17 +159,33 @@ void register_page_bootmem_info_node(str
 }
 #endif /* !CONFIG_SPARSEMEM_VMEMMAP */
 
+static void grow_zone_span(struct zone *zone, unsigned long start_pfn,
+			   unsigned long end_pfn);
+static void grow_pgdat_span(struct pglist_data *pgdat,
+			    unsigned long start_pfn, unsigned long end_pfn);
+
 static int __add_zone(struct zone *zone, unsigned long phys_start_pfn)
 {
 	struct pglist_data *pgdat = zone->zone_pgdat;
 	int nr_pages = PAGES_PER_SECTION;
 	int nid = pgdat->node_id;
 	int zone_type;
+	unsigned long flags;
 
 	zone_type = zone - pgdat->node_zones;
-	if (!zone->wait_table)
-		return init_currently_empty_zone(zone, phys_start_pfn,
-						 nr_pages, MEMMAP_HOTPLUG);
+	if (!zone->wait_table) {
+		int ret;
+
+		ret = init_currently_empty_zone(zone, phys_start_pfn,
+						nr_pages, MEMMAP_HOTPLUG);
+		if (ret)
+			return ret;
+	}
+	pgdat_resize_lock(zone->zone_pgdat, &flags);
+	grow_zone_span(zone, phys_start_pfn, phys_start_pfn + nr_pages);
+	grow_pgdat_span(zone->zone_pgdat, phys_start_pfn,
+			phys_start_pfn + nr_pages);
+	pgdat_resize_unlock(zone->zone_pgdat, &flags);
 	memmap_init_zone(nr_pages, nid, zone_type,
 			 phys_start_pfn, MEMMAP_HOTPLUG);
 	return 0;
@@ -363,7 +379,6 @@ static int online_pages_range(unsigned l
 
 int online_pages(unsigned long pfn, unsigned long nr_pages)
 {
-	unsigned long flags;
 	unsigned long onlined_pages = 0;
 	struct zone *zone;
 	int need_zonelists_rebuild = 0;
@@ -391,11 +406,6 @@ int online_pages(unsigned long pfn, unsi
 	 * memory_block->state_mutex.
 	 */
 	zone = page_zone(pfn_to_page(pfn));
-	pgdat_resize_lock(zone->zone_pgdat, &flags);
-	grow_zone_span(zone, pfn, pfn + nr_pages);
-	grow_pgdat_span(zone->zone_pgdat, pfn, pfn + nr_pages);
-	pgdat_resize_unlock(zone->zone_pgdat, &flags);
-
 	/*
 	 * If this zone is not populated, then it is not in zonelist.
 	 * This means the page allocator ignores this zone.
Index: linux-2.6/mm/page_alloc.c
===================================================================
--- linux-2.6.orig/mm/page_alloc.c
+++ linux-2.6/mm/page_alloc.c
@@ -2862,8 +2862,6 @@ __meminit int init_currently_empty_zone(
 
 	zone->zone_start_pfn = zone_start_pfn;
 
-	memmap_init(size, pgdat->node_id, zone_idx(zone), zone_start_pfn);
-
 	zone_init_free_lists(zone);
 
 	return 0;
@@ -3433,6 +3431,7 @@ static void __paginginit free_area_init_
 		ret = init_currently_empty_zone(zone, zone_start_pfn,
 						size, MEMMAP_EARLY);
 		BUG_ON(ret);
+		memmap_init(size, nid, j, zone_start_pfn);
 		zone_start_pfn += size;
 	}
 }

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

* Re: [PATCH] memory_hotplug: always initialize pageblock bitmap.
  2008-05-10 12:45   ` Heiko Carstens
@ 2008-05-12  1:55     ` KAMEZAWA Hiroyuki
  2008-05-12  9:19       ` KAMEZAWA Hiroyuki
  0 siblings, 1 reply; 9+ messages in thread
From: KAMEZAWA Hiroyuki @ 2008-05-12  1:55 UTC (permalink / raw)
  To: Heiko Carstens
  Cc: Andrew Morton, Andy Whitcroft, Dave Hansen, Gerald Schaefer,
	linux-kernel, linux-mm

On Sat, 10 May 2008 14:45:01 +0200
Heiko Carstens <heiko.carstens@de.ibm.com> wrote:
> > Recently, I added a check "zone's start_pfn < pfn < zone's end"
> > to memmap_init_zone()'s usemap initialization for !SPARSEMEM case bug FIX.
> > (and I think the fix itself is sane.)
> 
> Oh, you broke memory hot-add on -stable ;)
>
Ah yes, my mistake. Very sorry, (but stable was also broken if !SPARSEMEM)
 
> > How about calling grow_pgdat_span()/grow_zone_span() from __add_zone() ?
> 
> Like this?
> 
> Note that this patch is on top of the other patch I already sent, but
> reverts it...
> 
> This works for me. A final version (if this is acceptable) should move
> the grow* functions also.
> 
> ---
>  mm/memory_hotplug.c |   28 +++++++++++++++++++---------
>  mm/page_alloc.c     |    3 +--
>  2 files changed, 20 insertions(+), 11 deletions(-)
> 
> Index: linux-2.6/mm/memory_hotplug.c
> ===================================================================
> --- linux-2.6.orig/mm/memory_hotplug.c
> +++ linux-2.6/mm/memory_hotplug.c
> @@ -159,17 +159,33 @@ void register_page_bootmem_info_node(str
>  }
>  #endif /* !CONFIG_SPARSEMEM_VMEMMAP */
>  
> +static void grow_zone_span(struct zone *zone, unsigned long start_pfn,
> +			   unsigned long end_pfn);
> +static void grow_pgdat_span(struct pglist_data *pgdat,
> +			    unsigned long start_pfn, unsigned long end_pfn);
> +
>  static int __add_zone(struct zone *zone, unsigned long phys_start_pfn)
>  {
>  	struct pglist_data *pgdat = zone->zone_pgdat;
>  	int nr_pages = PAGES_PER_SECTION;
>  	int nid = pgdat->node_id;
>  	int zone_type;
> +	unsigned long flags;
>  
>  	zone_type = zone - pgdat->node_zones;
> -	if (!zone->wait_table)
> -		return init_currently_empty_zone(zone, phys_start_pfn,
> -						 nr_pages, MEMMAP_HOTPLUG);
> +	if (!zone->wait_table) {
> +		int ret;
> +
> +		ret = init_currently_empty_zone(zone, phys_start_pfn,
> +						nr_pages, MEMMAP_HOTPLUG);
> +		if (ret)
> +			return ret;
> +	}
> +	pgdat_resize_lock(zone->zone_pgdat, &flags);
> +	grow_zone_span(zone, phys_start_pfn, phys_start_pfn + nr_pages);
> +	grow_pgdat_span(zone->zone_pgdat, phys_start_pfn,
> +			phys_start_pfn + nr_pages);
> +	pgdat_resize_unlock(zone->zone_pgdat, &flags);
>  	memmap_init_zone(nr_pages, nid, zone_type,
>  			 phys_start_pfn, MEMMAP_HOTPLUG);
>  	return 0;
seems good. I'll try this logic on my ia64 box, which allows
NUMA-node hotplug.

Thank you!
-Kame

> @@ -363,7 +379,6 @@ static int online_pages_range(unsigned l
>  
>  int online_pages(unsigned long pfn, unsigned long nr_pages)
>  {
> -	unsigned long flags;
>  	unsigned long onlined_pages = 0;
>  	struct zone *zone;
>  	int need_zonelists_rebuild = 0;
> @@ -391,11 +406,6 @@ int online_pages(unsigned long pfn, unsi
>  	 * memory_block->state_mutex.
>  	 */
>  	zone = page_zone(pfn_to_page(pfn));
> -	pgdat_resize_lock(zone->zone_pgdat, &flags);
> -	grow_zone_span(zone, pfn, pfn + nr_pages);
> -	grow_pgdat_span(zone->zone_pgdat, pfn, pfn + nr_pages);
> -	pgdat_resize_unlock(zone->zone_pgdat, &flags);
> -
>  	/*
>  	 * If this zone is not populated, then it is not in zonelist.
>  	 * This means the page allocator ignores this zone.
> Index: linux-2.6/mm/page_alloc.c
> ===================================================================
> --- linux-2.6.orig/mm/page_alloc.c
> +++ linux-2.6/mm/page_alloc.c
> @@ -2862,8 +2862,6 @@ __meminit int init_currently_empty_zone(
>  
>  	zone->zone_start_pfn = zone_start_pfn;
>  
> -	memmap_init(size, pgdat->node_id, zone_idx(zone), zone_start_pfn);
> -
>  	zone_init_free_lists(zone);
>  
>  	return 0;
> @@ -3433,6 +3431,7 @@ static void __paginginit free_area_init_
>  		ret = init_currently_empty_zone(zone, zone_start_pfn,
>  						size, MEMMAP_EARLY);
>  		BUG_ON(ret);
> +		memmap_init(size, nid, j, zone_start_pfn);
>  		zone_start_pfn += size;
>  	}
>  }
> 


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

* Re: [PATCH] memory_hotplug: always initialize pageblock bitmap.
  2008-05-12  1:55     ` KAMEZAWA Hiroyuki
@ 2008-05-12  9:19       ` KAMEZAWA Hiroyuki
  2008-05-13 11:58         ` Heiko Carstens
  0 siblings, 1 reply; 9+ messages in thread
From: KAMEZAWA Hiroyuki @ 2008-05-12  9:19 UTC (permalink / raw)
  To: KAMEZAWA Hiroyuki
  Cc: Heiko Carstens, Andrew Morton, Andy Whitcroft, Dave Hansen,
	Gerald Schaefer, linux-kernel, linux-mm

On Mon, 12 May 2008 10:55:00 +0900
KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote:
> seems good. I'll try this logic on my ia64 box, which allows
> NUMA-node hotplug.
> 
I'm sorry but I found memory-hotplug on my box doesn't work now
maybe because of recent? changes in linux's ACPI. It seems no notifies
reach to acpi container/memory driver. (maybe another regression...)
I'll dig...but it may take some amount of time.

I have no objection to your patch as a result of review.

Thanks,
-Kame


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

* memory_hotplug: always initialize pageblock bitmap.
  2008-05-12  9:19       ` KAMEZAWA Hiroyuki
@ 2008-05-13 11:58         ` Heiko Carstens
  2008-05-13 18:52           ` Heiko Carstens
  0 siblings, 1 reply; 9+ messages in thread
From: Heiko Carstens @ 2008-05-13 11:58 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Andy Whitcroft, Dave Hansen, Gerald Schaefer, linux-kernel,
	linux-mm, KAMEZAWA Hiroyuki

From: Heiko Carstens <heiko.carstens@de.ibm.com>

Trying to online a new memory section that was added via memory hotplug
sometimes results in crashes when the new pages are added via __free_page.
Reason for that is that the pageblock bitmap isn't initialized and hence
contains random stuff.  That means that get_pageblock_migratetype() returns
also random stuff and therefore

	list_add(&page->lru,
		&zone->free_area[order].free_list[migratetype]);

in __free_one_page() tries to do a list_add to something that isn't even
necessarily a list.

This happens since 86051ca5eaf5e560113ec7673462804c54284456
("mm: fix usemap initialization") which makes sure that the pageblock
bitmap gets only initialized for pages present in a zone.
Unfortunately for hot-added memory the zones "grow" after the memmap
and the pageblock memmap have been initialized. Which means that the
new pages have an unitialized bitmap.
To solve this the calls to grow_zone_span() and grow_pgdat_span() are
moved to __add_zone() just before the initialization happens.

The patch also moves the two functions since __add_zone() is the only
caller and I didn't want to add a forward declaration.

Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
Cc: Andy Whitcroft <apw@shadowen.org>
Cc: Dave Hansen <haveblue@us.ibm.com>
Cc: Gerald Schaefer <gerald.schaefer@de.ibm.com>
Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Cc: Yasunori Goto <y-goto@jp.fujitsu.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 mm/memory_hotplug.c |   83 +++++++++++++++++++++++++++-------------------------
 mm/page_alloc.c     |    3 -
 2 files changed, 45 insertions(+), 41 deletions(-)

Index: linux-2.6/mm/memory_hotplug.c
===================================================================
--- linux-2.6.orig/mm/memory_hotplug.c
+++ linux-2.6/mm/memory_hotplug.c
@@ -159,17 +159,58 @@ void register_page_bootmem_info_node(str
 }
 #endif /* !CONFIG_SPARSEMEM_VMEMMAP */
 
+static void grow_zone_span(struct zone *zone, unsigned long start_pfn,
+			   unsigned long end_pfn)
+{
+	unsigned long old_zone_end_pfn;
+
+	zone_span_writelock(zone);
+
+	old_zone_end_pfn = zone->zone_start_pfn + zone->spanned_pages;
+	if (start_pfn < zone->zone_start_pfn)
+		zone->zone_start_pfn = start_pfn;
+
+	zone->spanned_pages = max(old_zone_end_pfn, end_pfn) -
+				zone->zone_start_pfn;
+
+	zone_span_writeunlock(zone);
+}
+
+static void grow_pgdat_span(struct pglist_data *pgdat, unsigned long start_pfn,
+			    unsigned long end_pfn)
+{
+	unsigned long old_pgdat_end_pfn =
+		pgdat->node_start_pfn + pgdat->node_spanned_pages;
+
+	if (start_pfn < pgdat->node_start_pfn)
+		pgdat->node_start_pfn = start_pfn;
+
+	pgdat->node_spanned_pages = max(old_pgdat_end_pfn, end_pfn) -
+					pgdat->node_start_pfn;
+}
+
 static int __add_zone(struct zone *zone, unsigned long phys_start_pfn)
 {
 	struct pglist_data *pgdat = zone->zone_pgdat;
 	int nr_pages = PAGES_PER_SECTION;
 	int nid = pgdat->node_id;
 	int zone_type;
+	unsigned long flags;
 
 	zone_type = zone - pgdat->node_zones;
-	if (!zone->wait_table)
-		return init_currently_empty_zone(zone, phys_start_pfn,
-						 nr_pages, MEMMAP_HOTPLUG);
+	if (!zone->wait_table) {
+		int ret;
+
+		ret = init_currently_empty_zone(zone, phys_start_pfn,
+						nr_pages, MEMMAP_HOTPLUG);
+		if (ret)
+			return ret;
+	}
+	pgdat_resize_lock(zone->zone_pgdat, &flags);
+	grow_zone_span(zone, phys_start_pfn, phys_start_pfn + nr_pages);
+	grow_pgdat_span(zone->zone_pgdat, phys_start_pfn,
+			phys_start_pfn + nr_pages);
+	pgdat_resize_unlock(zone->zone_pgdat, &flags);
 	memmap_init_zone(nr_pages, nid, zone_type,
 			 phys_start_pfn, MEMMAP_HOTPLUG);
 	return 0;
@@ -295,36 +336,6 @@ int __remove_pages(struct zone *zone, un
 }
 EXPORT_SYMBOL_GPL(__remove_pages);
 
-static void grow_zone_span(struct zone *zone,
-		unsigned long start_pfn, unsigned long end_pfn)
-{
-	unsigned long old_zone_end_pfn;
-
-	zone_span_writelock(zone);
-
-	old_zone_end_pfn = zone->zone_start_pfn + zone->spanned_pages;
-	if (start_pfn < zone->zone_start_pfn)
-		zone->zone_start_pfn = start_pfn;
-
-	zone->spanned_pages = max(old_zone_end_pfn, end_pfn) -
-				zone->zone_start_pfn;
-
-	zone_span_writeunlock(zone);
-}
-
-static void grow_pgdat_span(struct pglist_data *pgdat,
-		unsigned long start_pfn, unsigned long end_pfn)
-{
-	unsigned long old_pgdat_end_pfn =
-		pgdat->node_start_pfn + pgdat->node_spanned_pages;
-
-	if (start_pfn < pgdat->node_start_pfn)
-		pgdat->node_start_pfn = start_pfn;
-
-	pgdat->node_spanned_pages = max(old_pgdat_end_pfn, end_pfn) -
-					pgdat->node_start_pfn;
-}
-
 void online_page(struct page *page)
 {
 	totalram_pages++;
@@ -363,7 +374,6 @@ static int online_pages_range(unsigned l
 
 int online_pages(unsigned long pfn, unsigned long nr_pages)
 {
-	unsigned long flags;
 	unsigned long onlined_pages = 0;
 	struct zone *zone;
 	int need_zonelists_rebuild = 0;
@@ -391,11 +401,6 @@ int online_pages(unsigned long pfn, unsi
 	 * memory_block->state_mutex.
 	 */
 	zone = page_zone(pfn_to_page(pfn));
-	pgdat_resize_lock(zone->zone_pgdat, &flags);
-	grow_zone_span(zone, pfn, pfn + nr_pages);
-	grow_pgdat_span(zone->zone_pgdat, pfn, pfn + nr_pages);
-	pgdat_resize_unlock(zone->zone_pgdat, &flags);
-
 	/*
 	 * If this zone is not populated, then it is not in zonelist.
 	 * This means the page allocator ignores this zone.
Index: linux-2.6/mm/page_alloc.c
===================================================================
--- linux-2.6.orig/mm/page_alloc.c
+++ linux-2.6/mm/page_alloc.c
@@ -2862,8 +2862,6 @@ __meminit int init_currently_empty_zone(
 
 	zone->zone_start_pfn = zone_start_pfn;
 
-	memmap_init(size, pgdat->node_id, zone_idx(zone), zone_start_pfn);
-
 	zone_init_free_lists(zone);
 
 	return 0;
@@ -3433,6 +3431,7 @@ static void __paginginit free_area_init_
 		ret = init_currently_empty_zone(zone, zone_start_pfn,
 						size, MEMMAP_EARLY);
 		BUG_ON(ret);
+		memmap_init(size, nid, j, zone_start_pfn);
 		zone_start_pfn += size;
 	}
 }

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

* Re: memory_hotplug: always initialize pageblock bitmap.
  2008-05-13 11:58         ` Heiko Carstens
@ 2008-05-13 18:52           ` Heiko Carstens
  2008-05-14  0:52             ` KAMEZAWA Hiroyuki
  0 siblings, 1 reply; 9+ messages in thread
From: Heiko Carstens @ 2008-05-13 18:52 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Andy Whitcroft, Dave Hansen, Gerald Schaefer, linux-kernel,
	linux-mm, KAMEZAWA Hiroyuki

On Tue, May 13, 2008 at 01:58:25PM +0200, Heiko Carstens wrote:
> From: Heiko Carstens <heiko.carstens@de.ibm.com>
> 
> Trying to online a new memory section that was added via memory hotplug
> sometimes results in crashes when the new pages are added via __free_page.
> Reason for that is that the pageblock bitmap isn't initialized and hence
> contains random stuff.  That means that get_pageblock_migratetype() returns
> also random stuff and therefore
> 
> 	list_add(&page->lru,
> 		&zone->free_area[order].free_list[migratetype]);
> 
> in __free_one_page() tries to do a list_add to something that isn't even
> necessarily a list.
> 
> This happens since 86051ca5eaf5e560113ec7673462804c54284456
> ("mm: fix usemap initialization") which makes sure that the pageblock
> bitmap gets only initialized for pages present in a zone.
> Unfortunately for hot-added memory the zones "grow" after the memmap
> and the pageblock memmap have been initialized. Which means that the
> new pages have an unitialized bitmap.
> To solve this the calls to grow_zone_span() and grow_pgdat_span() are
> moved to __add_zone() just before the initialization happens.
> 
> The patch also moves the two functions since __add_zone() is the only
> caller and I didn't want to add a forward declaration.
> 
> Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
> Cc: Andy Whitcroft <apw@shadowen.org>
> Cc: Dave Hansen <haveblue@us.ibm.com>
> Cc: Gerald Schaefer <gerald.schaefer@de.ibm.com>
> Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> Cc: Yasunori Goto <y-goto@jp.fujitsu.com>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> ---

Oops... the patch is tested and works for me. However it was not Signed-off
by Andrew. And in addition I forgot to add [PATCH] to the subject.
Sorry about that!

If all agree that this patch is ok it should probably also go into
-stable, since it fixes the above mentioned regression.

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

* Re: memory_hotplug: always initialize pageblock bitmap.
  2008-05-13 18:52           ` Heiko Carstens
@ 2008-05-14  0:52             ` KAMEZAWA Hiroyuki
  0 siblings, 0 replies; 9+ messages in thread
From: KAMEZAWA Hiroyuki @ 2008-05-14  0:52 UTC (permalink / raw)
  To: Heiko Carstens
  Cc: Andrew Morton, Andy Whitcroft, Dave Hansen, Gerald Schaefer,
	linux-kernel, linux-mm

On Tue, 13 May 2008 20:52:42 +0200
Heiko Carstens <heiko.carstens@de.ibm.com> wrote:
> Oops... the patch is tested and works for me. However it was not Signed-off
> by Andrew. And in addition I forgot to add [PATCH] to the subject.
> Sorry about that!
> 
> If all agree that this patch is ok it should probably also go into
> -stable, since it fixes the above mentioned regression.
> 
Thank you very much! The patch seems ok to me.
(But I cannot test this until my box is available...)

If other memory-hotplug guys say ok, I have no objection to -stable.

Thanks,
-Kame


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

end of thread, other threads:[~2008-05-14  0:55 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-05-09  6:06 [PATCH] memory_hotplug: always initialize pageblock bitmap Heiko Carstens
2008-05-09  6:39 ` KAMEZAWA Hiroyuki
2008-05-09  6:45   ` Heiko Carstens
2008-05-10 12:45   ` Heiko Carstens
2008-05-12  1:55     ` KAMEZAWA Hiroyuki
2008-05-12  9:19       ` KAMEZAWA Hiroyuki
2008-05-13 11:58         ` Heiko Carstens
2008-05-13 18:52           ` Heiko Carstens
2008-05-14  0:52             ` KAMEZAWA Hiroyuki

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