[prev in list] [next in list] [prev in thread] [next in thread] 

List:       linux-kernel
Subject:    Re: Config NO_BOOTMEM breaks my amd64 box
From:       Yinghai Lu <yinghai () kernel ! org>
Date:       2010-03-31 23:54:50
Message-ID: 4BB3E0CA.5060405 () kernel ! org
[Download RAW message or body]

On 03/31/2010 04:34 PM, H. Peter Anvin wrote:
> On 03/31/2010 03:47 PM, Yinghai Lu wrote:
>>>
>>> Well and that whole #ifdeffery is disgusting as well - even if the goal was to 
>>> remove CONFIG_NO_BOOTMEM ASAP.
>>>
>>> Please learn to use proper intermediate helper functions and at minimum put 
>>> the conversion ugliness somewhere that doesnt intrude our daily flow in .c 
>>> files. The best rule is to _never ever_ put an #ifdef construct into a .c 
>>> file. It doesnt matter what the goal if the #ifdef is - such ugliness in code 
>>> is never justified.
>>
>> if you agree that i can have one nobootmem.c in mm/
>>
> 
> That would be better, or more commonly, use inlines.
> 
> I'm still totally puzzled about this patch as well as the comment:
> 
> +#if defined(CONFIG_NO_BOOTMEM) && defined(MAX_NUMNODES)
> +	/* In case some 32bit systems don't have RAM installed on node0 */
> +        totalram_pages += free_all_memory_core_early(MAX_NUMNODES);
> +#else
>  	totalram_pages += free_all_bootmem();
> +#endif
> 
> 
> Why is that "32 bits" specific?  Second, MAX_NUMNODES is defined
> whenever <linux/numa.h> is included, so what on Earth is this supposed
> to signify?  Are you trying to say MAX_NUMNODES > 1?  Or are you trying
> to say CONFIG_NUMA?

you are right, this one should be more clear.

Subject: [PATCH -v2] nobootmem, x86: Fix 32bit system without RAM on Node0

when 32bit numa is used, free_all_bootmem() will still only go over with
node id 0.

If node 0 doesn't have RAM installed, We need to go with node1
because early_node_map still use 1 for all ranges, and ram from node1
becom low ram.

Try to use MAX_NUMNODES like 64 numa does.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>

---
 mm/bootmem.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux-2.6/mm/bootmem.c
===================================================================
--- linux-2.6.orig/mm/bootmem.c
+++ linux-2.6/mm/bootmem.c
@@ -303,7 +303,7 @@ unsigned long __init free_all_bootmem_no
 unsigned long __init free_all_bootmem(void)
 {
 #ifdef CONFIG_NO_BOOTMEM
-	return free_all_memory_core_early(NODE_DATA(0)->node_id);
+	return free_all_memory_core_early(MAX_NUMNODES);
 #else
 	return free_all_bootmem_core(NODE_DATA(0)->bdata);
 #endif

> 
> Furthermore, I really don't see the connection between this and James
> Morris' reported problem, which he reports as "amd64", which presumably
> is an x86-64 kernel and not 32 bits...  James, is that correct?  Any
> more details you can give about the system?  I *really* don't want to go
> into cargo cult programming mode, that would suck eggs no matter what.

it happened one of my test setup, node0 ram disappear somehow.
and i found the 32bit numa doesn't work on that.

Thanks

Yinghai
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
[prev in list] [next in list] [prev in thread] [next in thread]