All of lore.kernel.org
 help / color / mirror / Atom feed
From: Arnd Bergmann <arnd@arndb.de>
To: linuxppc-dev@ozlabs.org
Cc: Andrew Morton <akpm@osdl.org>,
	mkravetz@us.ibm.com, linux-kernel@vger.kernel.org,
	hch@infradead.org, linux-mm@kvack.org, paulus@samba.org,
	kmannth@us.ibm.com, gone@us.ibm.com, cbe-oss-dev@ozlabs.org,
	KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Subject: Re: [PATCH] Fix sparsemem on Cell
Date: Mon, 18 Dec 2006 23:54:45 +0100	[thread overview]
Message-ID: <200612182354.47685.arnd@arndb.de> (raw)
In-Reply-To: <20061216170353.2dfa27b1.kamezawa.hiroyu@jp.fujitsu.com>

On Saturday 16 December 2006 09:03, KAMEZAWA Hiroyuki wrote:
> @@ -273,10 +284,13 @@
> =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0if (ret)
> =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0g=
oto error;
> =A0=A0=A0=A0=A0=A0=A0=A0}
> +=A0=A0=A0=A0=A0=A0=A0atomic_inc(&memory_hotadd_count);
> =A0
> =A0=A0=A0=A0=A0=A0=A0=A0/* call arch's memory hotadd */
> =A0=A0=A0=A0=A0=A0=A0=A0ret =3D arch_add_memory(nid, start, size);
> =A0
> +=A0=A0=A0=A0=A0=A0=A0atomic_dec(&memory_hotadd_count);
> +
> =A0=A0=A0=A0=A0=A0=A0=A0if (ret < 0)
> =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0goto error;
> =A0

This also doesn't fix the problem on cell, since the time when the bug
happens, we're not calling through this function, or arch_add_memory,
at all, but rather invoke __add_pages directly. As BenH already mentioned,
we shouldn't do really call __add_pages at all.

Let me attempt another fix that might address all cases. This is completely
untested as of now, but also addresses Dave's latest comment.

	Arnd <><

diff --git a/arch/ia64/mm/init.c b/arch/ia64/mm/init.c
index 1a3d8a2..723d220 100644
=2D-- a/arch/ia64/mm/init.c
+++ b/arch/ia64/mm/init.c
@@ -543,7 +543,7 @@ virtual_memmap_init (u64 start, u64 end, void *arg)
=20
 	if (map_start < map_end)
 		memmap_init_zone((unsigned long)(map_end - map_start),
=2D				 args->nid, args->zone, page_to_pfn(map_start));
+				 args->nid, args->zone, page_to_pfn(map_start), 1);
 	return 0;
 }
=20
diff --git a/arch/s390/mm/vmem.c b/arch/s390/mm/vmem.c
index 7f2944d..1e52cd1 100644
=2D-- a/arch/s390/mm/vmem.c
+++ b/arch/s390/mm/vmem.c
@@ -61,7 +61,7 @@ void memmap_init(unsigned long size, int nid, unsigned lo=
ng zone,
=20
 		if (map_start < map_end)
 			memmap_init_zone((unsigned long)(map_end - map_start),
=2D					 nid, zone, page_to_pfn(map_start));
+					 nid, zone, page_to_pfn(map_start), 1);
 	}
 }
=20
diff --git a/include/linux/mm.h b/include/linux/mm.h
index a17b147..6d85068 100644
=2D-- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -978,7 +978,7 @@ extern int early_pfn_to_nid(unsigned long pfn);
 #endif /* CONFIG_HAVE_ARCH_EARLY_PFN_TO_NID */
 #endif /* CONFIG_ARCH_POPULATES_NODE_MAP */
 extern void set_dma_reserve(unsigned long new_dma_reserve);
=2Dextern void memmap_init_zone(unsigned long, int, unsigned long, unsigned=
 long);
+extern void memmap_init_zone(unsigned long, int, unsigned long, unsigned l=
ong, int);
 extern void setup_per_zone_pages_min(void);
 extern void mem_init(void);
 extern void show_mem(void);
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index 0c055a0..16c9930 100644
=2D-- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -71,7 +71,7 @@ static int __add_zone(struct zone *zone, unsigned long ph=
ys_start_pfn)
 		if (ret < 0)
 			return ret;
 	}
=2D	memmap_init_zone(nr_pages, nid, zone_type, phys_start_pfn);
+	memmap_init_zone(nr_pages, nid, zone_type, phys_start_pfn, 0);
 	return 0;
 }
=20
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 8c1a116..60d1ac8 100644
=2D-- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -1953,17 +1953,19 @@ static inline unsigned long wait_table_bits(unsigne=
d long size)
  * done. Non-atomic initialization, single-pass.
  */
 void __meminit memmap_init_zone(unsigned long size, int nid, unsigned long=
 zone,
=2D		unsigned long start_pfn)
+		unsigned long start_pfn, int early)
 {
 	struct page *page;
 	unsigned long end_pfn =3D start_pfn + size;
 	unsigned long pfn;
=20
 	for (pfn =3D start_pfn; pfn < end_pfn; pfn++) {
=2D		if (!early_pfn_valid(pfn))
=2D			continue;
=2D		if (!early_pfn_in_nid(pfn, nid))
=2D			continue;
+		if (early) {
+			if (!early_pfn_valid(pfn))
+				continue;
+			if (!early_pfn_in_nid(pfn, nid))
+				continue;
+		}
 		page =3D pfn_to_page(pfn);
 		set_page_links(page, zone, nid, pfn);
 		init_page_count(page);
@@ -1990,7 +1992,7 @@ void zone_init_free_lists(struct pglist_data *pgdat, =
struct zone *zone,
=20
 #ifndef __HAVE_ARCH_MEMMAP_INIT
 #define memmap_init(size, nid, zone, start_pfn) \
=2D	memmap_init_zone((size), (nid), (zone), (start_pfn))
+	memmap_init_zone((size), (nid), (zone), (start_pfn), 1)
 #endif
=20
 static int __cpuinit zone_batchsize(struct zone *zone)

WARNING: multiple messages have this Message-ID (diff)
From: Arnd Bergmann <arnd@arndb.de>
To: linuxppc-dev@ozlabs.org
Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>,
	Andrew Morton <akpm@osdl.org>,
	kmannth@us.ibm.com, linux-kernel@vger.kernel.org,
	hch@infradead.org, linux-mm@kvack.org, paulus@samba.org,
	mkravetz@us.ibm.com, gone@us.ibm.com, cbe-oss-dev@ozlabs.org,
	Dave Hansen <haveblue@us.ibm.com>
Subject: Re: [PATCH] Fix sparsemem on Cell
Date: Mon, 18 Dec 2006 23:54:45 +0100	[thread overview]
Message-ID: <200612182354.47685.arnd@arndb.de> (raw)
In-Reply-To: <20061216170353.2dfa27b1.kamezawa.hiroyu@jp.fujitsu.com>

On Saturday 16 December 2006 09:03, KAMEZAWA Hiroyuki wrote:
> @@ -273,10 +284,13 @@
>                 if (ret)
>                         goto error;
>         }
> +       atomic_inc(&memory_hotadd_count);
>  
>         /* call arch's memory hotadd */
>         ret = arch_add_memory(nid, start, size);
>  
> +       atomic_dec(&memory_hotadd_count);
> +
>         if (ret < 0)
>                 goto error;
>  

This also doesn't fix the problem on cell, since the time when the bug
happens, we're not calling through this function, or arch_add_memory,
at all, but rather invoke __add_pages directly. As BenH already mentioned,
we shouldn't do really call __add_pages at all.

Let me attempt another fix that might address all cases. This is completely
untested as of now, but also addresses Dave's latest comment.

	Arnd <><

diff --git a/arch/ia64/mm/init.c b/arch/ia64/mm/init.c
index 1a3d8a2..723d220 100644
--- a/arch/ia64/mm/init.c
+++ b/arch/ia64/mm/init.c
@@ -543,7 +543,7 @@ virtual_memmap_init (u64 start, u64 end, void *arg)
 
 	if (map_start < map_end)
 		memmap_init_zone((unsigned long)(map_end - map_start),
-				 args->nid, args->zone, page_to_pfn(map_start));
+				 args->nid, args->zone, page_to_pfn(map_start), 1);
 	return 0;
 }
 
diff --git a/arch/s390/mm/vmem.c b/arch/s390/mm/vmem.c
index 7f2944d..1e52cd1 100644
--- a/arch/s390/mm/vmem.c
+++ b/arch/s390/mm/vmem.c
@@ -61,7 +61,7 @@ void memmap_init(unsigned long size, int nid, unsigned long zone,
 
 		if (map_start < map_end)
 			memmap_init_zone((unsigned long)(map_end - map_start),
-					 nid, zone, page_to_pfn(map_start));
+					 nid, zone, page_to_pfn(map_start), 1);
 	}
 }
 
diff --git a/include/linux/mm.h b/include/linux/mm.h
index a17b147..6d85068 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -978,7 +978,7 @@ extern int early_pfn_to_nid(unsigned long pfn);
 #endif /* CONFIG_HAVE_ARCH_EARLY_PFN_TO_NID */
 #endif /* CONFIG_ARCH_POPULATES_NODE_MAP */
 extern void set_dma_reserve(unsigned long new_dma_reserve);
-extern void memmap_init_zone(unsigned long, int, unsigned long, unsigned long);
+extern void memmap_init_zone(unsigned long, int, unsigned long, unsigned long, int);
 extern void setup_per_zone_pages_min(void);
 extern void mem_init(void);
 extern void show_mem(void);
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index 0c055a0..16c9930 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -71,7 +71,7 @@ static int __add_zone(struct zone *zone, unsigned long phys_start_pfn)
 		if (ret < 0)
 			return ret;
 	}
-	memmap_init_zone(nr_pages, nid, zone_type, phys_start_pfn);
+	memmap_init_zone(nr_pages, nid, zone_type, phys_start_pfn, 0);
 	return 0;
 }
 
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 8c1a116..60d1ac8 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -1953,17 +1953,19 @@ static inline unsigned long wait_table_bits(unsigned long size)
  * done. Non-atomic initialization, single-pass.
  */
 void __meminit memmap_init_zone(unsigned long size, int nid, unsigned long zone,
-		unsigned long start_pfn)
+		unsigned long start_pfn, int early)
 {
 	struct page *page;
 	unsigned long end_pfn = start_pfn + size;
 	unsigned long pfn;
 
 	for (pfn = start_pfn; pfn < end_pfn; pfn++) {
-		if (!early_pfn_valid(pfn))
-			continue;
-		if (!early_pfn_in_nid(pfn, nid))
-			continue;
+		if (early) {
+			if (!early_pfn_valid(pfn))
+				continue;
+			if (!early_pfn_in_nid(pfn, nid))
+				continue;
+		}
 		page = pfn_to_page(pfn);
 		set_page_links(page, zone, nid, pfn);
 		init_page_count(page);
@@ -1990,7 +1992,7 @@ void zone_init_free_lists(struct pglist_data *pgdat, struct zone *zone,
 
 #ifndef __HAVE_ARCH_MEMMAP_INIT
 #define memmap_init(size, nid, zone, start_pfn) \
-	memmap_init_zone((size), (nid), (zone), (start_pfn))
+	memmap_init_zone((size), (nid), (zone), (start_pfn), 1)
 #endif
 
 static int __cpuinit zone_batchsize(struct zone *zone)

WARNING: multiple messages have this Message-ID (diff)
From: Arnd Bergmann <arnd@arndb.de>
To: linuxppc-dev@ozlabs.org
Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>,
	Andrew Morton <akpm@osdl.org>,
	kmannth@us.ibm.com, linux-kernel@vger.kernel.org,
	hch@infradead.org, linux-mm@kvack.org, paulus@samba.org,
	mkravetz@us.ibm.com, gone@us.ibm.com, cbe-oss-dev@ozlabs.org,
	Dave Hansen <haveblue@us.ibm.com>
Subject: Re: [PATCH] Fix sparsemem on Cell
Date: Mon, 18 Dec 2006 23:54:45 +0100	[thread overview]
Message-ID: <200612182354.47685.arnd@arndb.de> (raw)
In-Reply-To: <20061216170353.2dfa27b1.kamezawa.hiroyu@jp.fujitsu.com>

On Saturday 16 December 2006 09:03, KAMEZAWA Hiroyuki wrote:
> @@ -273,10 +284,13 @@
>                 if (ret)
>                         goto error;
>         }
> +       atomic_inc(&memory_hotadd_count);
>  
>         /* call arch's memory hotadd */
>         ret = arch_add_memory(nid, start, size);
>  
> +       atomic_dec(&memory_hotadd_count);
> +
>         if (ret < 0)
>                 goto error;
>  

This also doesn't fix the problem on cell, since the time when the bug
happens, we're not calling through this function, or arch_add_memory,
at all, but rather invoke __add_pages directly. As BenH already mentioned,
we shouldn't do really call __add_pages at all.

Let me attempt another fix that might address all cases. This is completely
untested as of now, but also addresses Dave's latest comment.

	Arnd <><

diff --git a/arch/ia64/mm/init.c b/arch/ia64/mm/init.c
index 1a3d8a2..723d220 100644
--- a/arch/ia64/mm/init.c
+++ b/arch/ia64/mm/init.c
@@ -543,7 +543,7 @@ virtual_memmap_init (u64 start, u64 end, void *arg)
 
 	if (map_start < map_end)
 		memmap_init_zone((unsigned long)(map_end - map_start),
-				 args->nid, args->zone, page_to_pfn(map_start));
+				 args->nid, args->zone, page_to_pfn(map_start), 1);
 	return 0;
 }
 
diff --git a/arch/s390/mm/vmem.c b/arch/s390/mm/vmem.c
index 7f2944d..1e52cd1 100644
--- a/arch/s390/mm/vmem.c
+++ b/arch/s390/mm/vmem.c
@@ -61,7 +61,7 @@ void memmap_init(unsigned long size, int nid, unsigned long zone,
 
 		if (map_start < map_end)
 			memmap_init_zone((unsigned long)(map_end - map_start),
-					 nid, zone, page_to_pfn(map_start));
+					 nid, zone, page_to_pfn(map_start), 1);
 	}
 }
 
diff --git a/include/linux/mm.h b/include/linux/mm.h
index a17b147..6d85068 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -978,7 +978,7 @@ extern int early_pfn_to_nid(unsigned long pfn);
 #endif /* CONFIG_HAVE_ARCH_EARLY_PFN_TO_NID */
 #endif /* CONFIG_ARCH_POPULATES_NODE_MAP */
 extern void set_dma_reserve(unsigned long new_dma_reserve);
-extern void memmap_init_zone(unsigned long, int, unsigned long, unsigned long);
+extern void memmap_init_zone(unsigned long, int, unsigned long, unsigned long, int);
 extern void setup_per_zone_pages_min(void);
 extern void mem_init(void);
 extern void show_mem(void);
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index 0c055a0..16c9930 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -71,7 +71,7 @@ static int __add_zone(struct zone *zone, unsigned long phys_start_pfn)
 		if (ret < 0)
 			return ret;
 	}
-	memmap_init_zone(nr_pages, nid, zone_type, phys_start_pfn);
+	memmap_init_zone(nr_pages, nid, zone_type, phys_start_pfn, 0);
 	return 0;
 }
 
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 8c1a116..60d1ac8 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -1953,17 +1953,19 @@ static inline unsigned long wait_table_bits(unsigned long size)
  * done. Non-atomic initialization, single-pass.
  */
 void __meminit memmap_init_zone(unsigned long size, int nid, unsigned long zone,
-		unsigned long start_pfn)
+		unsigned long start_pfn, int early)
 {
 	struct page *page;
 	unsigned long end_pfn = start_pfn + size;
 	unsigned long pfn;
 
 	for (pfn = start_pfn; pfn < end_pfn; pfn++) {
-		if (!early_pfn_valid(pfn))
-			continue;
-		if (!early_pfn_in_nid(pfn, nid))
-			continue;
+		if (early) {
+			if (!early_pfn_valid(pfn))
+				continue;
+			if (!early_pfn_in_nid(pfn, nid))
+				continue;
+		}
 		page = pfn_to_page(pfn);
 		set_page_links(page, zone, nid, pfn);
 		init_page_count(page);
@@ -1990,7 +1992,7 @@ void zone_init_free_lists(struct pglist_data *pgdat, struct zone *zone,
 
 #ifndef __HAVE_ARCH_MEMMAP_INIT
 #define memmap_init(size, nid, zone, start_pfn) \
-	memmap_init_zone((size), (nid), (zone), (start_pfn))
+	memmap_init_zone((size), (nid), (zone), (start_pfn), 1)
 #endif
 
 static int __cpuinit zone_batchsize(struct zone *zone)

--
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: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  parent reply	other threads:[~2006-12-18 22:54 UTC|newest]

Thread overview: 68+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-12-15 16:53 [PATCH] Fix sparsemem on Cell Dave Hansen
2006-12-15 16:53 ` Dave Hansen
2006-12-15 16:53 ` Dave Hansen
2006-12-15 17:11 ` Andy Whitcroft
2006-12-15 17:11   ` Andy Whitcroft
2006-12-15 17:11   ` Andy Whitcroft
2006-12-15 17:24   ` Dave Hansen
2006-12-15 17:24     ` Dave Hansen
2006-12-15 17:24     ` Dave Hansen
2006-12-15 19:45     ` Andrew Morton
2006-12-15 19:45       ` Andrew Morton
2006-12-15 19:45       ` Andrew Morton
2006-12-16  8:03       ` KAMEZAWA Hiroyuki
2006-12-16  8:03         ` KAMEZAWA Hiroyuki
2006-12-16  8:03         ` KAMEZAWA Hiroyuki
2006-12-18 21:13         ` Dave Hansen
2006-12-18 21:13           ` Dave Hansen
2006-12-18 21:13           ` Dave Hansen
2006-12-18 22:15           ` Christoph Hellwig
2006-12-18 22:15             ` Christoph Hellwig
2006-12-18 22:15             ` Christoph Hellwig
2006-12-18 22:54         ` Arnd Bergmann [this message]
2006-12-18 22:54           ` Arnd Bergmann
2006-12-18 22:54           ` Arnd Bergmann
2006-12-18 23:16           ` Dave Hansen
2006-12-18 23:16             ` Dave Hansen
2006-12-18 23:16             ` Dave Hansen
2006-12-19  0:16             ` KAMEZAWA Hiroyuki
2006-12-19  0:16               ` KAMEZAWA Hiroyuki
2006-12-19  0:16               ` KAMEZAWA Hiroyuki
2006-12-19  8:59             ` Arnd Bergmann
2006-12-19  8:59               ` Arnd Bergmann
2006-12-19  8:59               ` Arnd Bergmann
2006-12-19 19:34               ` Dave Hansen
2006-12-19 19:34                 ` Dave Hansen
2006-12-19 19:34                 ` Dave Hansen
2007-01-06  1:10               ` [PATCH] Fix sparsemem on Cell (take 3) Dave Hansen
2007-01-06  1:10                 ` Dave Hansen
2007-01-06  1:10                 ` Dave Hansen
2007-01-06  4:52                 ` John Rose
2007-01-06  4:52                   ` John Rose
2007-01-06  4:52                   ` John Rose
2007-01-07  8:58                   ` Dave Hansen
2007-01-07  8:58                     ` Dave Hansen
2007-01-07  8:58                     ` Dave Hansen
2007-01-07 12:07                     ` Arnd Bergmann
2007-01-07 12:07                       ` Arnd Bergmann
2007-01-07 12:07                       ` Arnd Bergmann
2007-01-08  6:31                     ` Tim Pepper
2007-01-08  6:31                       ` Tim Pepper
2007-01-08  6:47                     ` Tim Pepper
2007-01-08  6:47                       ` Tim Pepper
2007-01-08  6:47                       ` Tim Pepper
2006-12-15 17:22 ` [PATCH] Fix sparsemem on Cell Michael Buesch
2006-12-15 17:22   ` Michael Buesch
2006-12-15 17:22   ` Michael Buesch
2006-12-15 17:57   ` Dave Hansen
2006-12-15 17:57     ` Dave Hansen
2006-12-15 17:57     ` Dave Hansen
2006-12-15 20:21 ` Benjamin Herrenschmidt
2006-12-15 20:21   ` Benjamin Herrenschmidt
2006-12-15 20:21   ` Benjamin Herrenschmidt
  -- strict thread matches above, loose matches on Subject: below --
2006-12-15 17:14 Dave Hansen
2006-12-15 17:14 ` Dave Hansen
2006-12-15 17:14 ` Dave Hansen
2006-12-17 23:02 ` Arnd Bergmann
2006-12-17 23:02   ` Arnd Bergmann
2006-12-17 23:02   ` Arnd Bergmann

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=200612182354.47685.arnd@arndb.de \
    --to=arnd@arndb.de \
    --cc=akpm@osdl.org \
    --cc=cbe-oss-dev@ozlabs.org \
    --cc=gone@us.ibm.com \
    --cc=hch@infradead.org \
    --cc=kamezawa.hiroyu@jp.fujitsu.com \
    --cc=kmannth@us.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linuxppc-dev@ozlabs.org \
    --cc=mkravetz@us.ibm.com \
    --cc=paulus@samba.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.