[PATCH] powerpc: mem_init crash for sparsemem

[PATCH] powerpc: mem_init crash for sparsemem

[Arnd Bergmann]

I have a Cell blade with some broken memory in the middle of the
physical address space and this is correctly detected by the
firmware, but not relocated. When I enable CONFIG_SPARSEMEM,
the memsections for the nonexistant address space do not
get struct page entries allocated, as expected.

However, mem_init for the non-NUMA configuration tries to
access these pages without first looking if they are there.
I'm currently using the hack below to work around that, but
I have the feeling that there should be a cleaner solution
for this.

Please comment.

Signed-off-by: Arnd Bergmann <arndb@de.ibm.com>

--- linux-2.6.15-rc.orig/arch/powerpc/mm/mem.c
+++ linux-2.6.15-rc/arch/powerpc/mm/mem.c
@@ -348,6 +348,9 @@ void __init mem_init(void)
 #endif
 	for_each_pgdat(pgdat) {
 		for (i = 0; i < pgdat->node_spanned_pages; i++) {
+			if (!section_has_mem_map(__pfn_to_section
+					(pgdat->node_start_pfn + i)))
+				continue;
 			page = pgdat_page_nr(pgdat, i);
 			if (PageReserved(page))
 				reservedpages++;

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

Re: [PATCH] powerpc: mem_init crash for sparsemem

[Andy Whitcroft]

Arnd Bergmann wrote:
> I have a Cell blade with some broken memory in the middle of the
> physical address space and this is correctly detected by the
> firmware, but not relocated. When I enable CONFIG_SPARSEMEM,
> the memsections for the nonexistant address space do not
> get struct page entries allocated, as expected.
> 
> However, mem_init for the non-NUMA configuration tries to
> access these pages without first looking if they are there.
> I'm currently using the hack below to work around that, but
> I have the feeling that there should be a cleaner solution
> for this.
> 
> Please comment.
> 
> Signed-off-by: Arnd Bergmann <arndb@de.ibm.com>
> 
> --- linux-2.6.15-rc.orig/arch/powerpc/mm/mem.c
> +++ linux-2.6.15-rc/arch/powerpc/mm/mem.c
> @@ -348,6 +348,9 @@ void __init mem_init(void)
>  #endif
>  	for_each_pgdat(pgdat) {
>  		for (i = 0; i < pgdat->node_spanned_pages; i++) {
> +			if (!section_has_mem_map(__pfn_to_section
> +					(pgdat->node_start_pfn + i)))
> +				continue;
>  			page = pgdat_page_nr(pgdat, i);
>  			if (PageReserved(page))
>  				reservedpages++;

Would it not make sense to use pfn_valid(), as that is not sparsemem
specific?  Not looked at the code in question specifically, but if you
can use section_has_mem_map() it should be equivalent:

	if (!pfn_valid(pgdat->node_start_pfn + i))
		continue;

Want to spin us a patch and I'll give it some general testing.

-apw

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

Re: [PATCH] powerpc: mem_init crash for sparsemem

[Mike Kravetz]

On Fri, Nov 04, 2005 at 08:18:19PM +0000, Andy Whitcroft wrote:
> Arnd Bergmann wrote:
> > I have a Cell blade with some broken memory in the middle of the
> > physical address space and this is correctly detected by the
> > firmware, but not relocated. When I enable CONFIG_SPARSEMEM,
> > the memsections for the nonexistant address space do not
> > get struct page entries allocated, as expected.
> > 
> > However, mem_init for the non-NUMA configuration tries to
> > access these pages without first looking if they are there.

This earlier statement in mem_init (or at least the comment),

num_physpages = max_pfn;        /* RAM is assumed contiguous */

may be a cause for concern.  I'm pretty sure max_pfn has previously
been set based on the value of lmb_end_of_DRAM().  My guess is that we
are going to report the system as having more memory that it actually
does (will not account for the hole(s)).

That being said, the pfn_valid() check is still needed here.  But,
it looks like that code was originally written under the assumption
that there were no holes.

Can someone 'more in the know' of ppc architecture comment on the
ram is contiguous assumption?  Is this no longer the case?
-- 
Mike

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

Re: [PATCH] powerpc: mem_init crash for sparsemem

[Arnd Bergmann]

On Freedag 04 November 2005 21:18, Andy Whitcroft wrote:
> Would it not make sense to use pfn_valid(), as that is not sparsemem
> specific?  Not looked at the code in question specifically, but if you
> can use section_has_mem_map() it should be equivalent:
> 
>         if (!pfn_valid(pgdat->node_start_pfn + i))
>                 continue;
> 
> Want to spin us a patch and I'll give it some general testing.

Yes, I guess pfn_valid() is the function I was looking for, thanks
for pointing that out.

Unfortunately, I don't have access to the machine over the weekend,
so I won't be able to test that until Monday.

	Arnd <><

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

Re: [PATCH] powerpc: mem_init crash for sparsemem

[Arnd Bergmann]

On Freedag 04 November 2005 21:57, Mike Kravetz wrote:

> This earlier statement in mem_init (or at least the comment),
> 
> num_physpages = max_pfn;        /* RAM is assumed contiguous */
> 
> may be a cause for concern.  I'm pretty sure max_pfn has previously
> been set based on the value of lmb_end_of_DRAM().  My guess is that we
> are going to report the system as having more memory that it actually
> does (will not account for the hole(s)).

Yes, that's likely to cause trouble later. Unfortunately, there are still
multiple places that determine the memory size by different means
and save the result in a global variable, so it's hard to get them
all right.

I'll probably move Cell to use NUMA mode for setups with multiple CPUs
(each of which is already SMT), which means we can use the code that
we already know handles this correctly, in addtition to the option
of using NUMA aware memory allocation and scheduling for the SPUs.

> That being said, the pfn_valid() check is still needed here.  But,
> it looks like that code was originally written under the assumption
> that there were no holes.
> 
> Can someone 'more in the know' of ppc architecture comment on the
> ram is contiguous assumption?  Is this no longer the case?

For all I know, the firmware interface can legally declare noncontiguous
memory, but that is not done on product level hardware except NUMA.
The configuration for SPARSEMEM without NUMA is normally not possible
on ppc64, I had to hack Kconfig to allow this in the first place.
Without SPARSEMEM, the noncontiguous memory seems to be handled well
except for the size detection.

	Arnd <><

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