From: H. Peter Anvin on 31 Mar 2010 19:40 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? 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. -hpa -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: H. Peter Anvin on 31 Mar 2010 19:40 On 03/31/2010 03:58 PM, James Morris wrote: > On Wed, 31 Mar 2010, Ingo Molnar wrote: > >>> >>> Yes, it was happening with -rc3. >> >> Could you please send the bootlog that Yinghai asked for, plus also one that >> you get with NO_BOOTMEM turned off (for comparison)? > > I don't have the old boot logs, and have since upgraded the system > further. > Upgraded how? The problem no longer happens? > IIRC, the boot was failing after not being able to find the root fs > (ext3/lvm/raid0). I thought it was a dracut issue, but it seemed to be > fixed by enabling bootmem. This would rather match the problem that was addressed by the patch in -rc3. Any help in reproducing it would be great. -hpa -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: James Morris on 31 Mar 2010 19:50 On Wed, 31 Mar 2010, H. Peter Anvin wrote: > On 03/31/2010 03:58 PM, James Morris wrote: > > On Wed, 31 Mar 2010, Ingo Molnar wrote: > > > >>> > >>> Yes, it was happening with -rc3. > >> > >> Could you please send the bootlog that Yinghai asked for, plus also one that > >> you get with NO_BOOTMEM turned off (for comparison)? > > > > I don't have the old boot logs, and have since upgraded the system > > further. > > > > Upgraded how? The problem no longer happens? Upgraded to the latest rawhide userland -- I have not since tested with bootmem off. I'll try and do so again when I get a chance. > > > IIRC, the boot was failing after not being able to find the root fs > > (ext3/lvm/raid0). I thought it was a dracut issue, but it seemed to be > > fixed by enabling bootmem. > > This would rather match the problem that was addressed by the patch in > -rc3. Any help in reproducing it would be great. > > -hpa > -- James Morris <jmorris(a)namei.org> -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: H. Peter Anvin on 31 Mar 2010 20:00 On 03/31/2010 04:43 PM, James Morris wrote: >> >> Upgraded how? The problem no longer happens? > > Upgraded to the latest rawhide userland -- I have not since tested with > bootmem off. I'll try and do so again when I get a chance. > That would be great. The sooner the better, obviously. -hpa -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Yinghai Lu on 31 Mar 2010 20:00
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(a)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(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |