* [PATCH 0/2] Fix a bug and cleanup NUMA boot-time code
@ 2008-12-11 18:36 Dave Hansen
2008-12-11 18:36 ` [PATCH 1/2] remove unused i var Dave Hansen
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Dave Hansen @ 2008-12-11 18:36 UTC (permalink / raw)
To: paulus
Cc: Jon Tollefson, Mel Gorman, Dave Hansen, linuxppc-dev,
Serge E. Hallyn
The first patch in this series is a genuine bug fix. The rest
are really just an RFC.
Jon introduced a bug a while ago. I introduced another when
trying to fix Jon's bug. I refuse to accept personal blame for
this and, instead, blame the code. :)
The reset of the series are "cleanups" that I think will help
clarify the code in numa.c and work to ensure that the next
bonehead like me is not as able to easily muck up the code. :)
The cleanups increase in impact and intrusiveness as the series
goes along, so please consider them an RFC. But, what I really
want to figure out is a safer way to initialize NODE_DATA() and
start using it as we bring up bootmem on all the nodes.
^ permalink raw reply [flat|nested] 4+ messages in thread* [PATCH 1/2] remove unused i var 2008-12-11 18:36 [PATCH 0/2] Fix a bug and cleanup NUMA boot-time code Dave Hansen @ 2008-12-11 18:36 ` Dave Hansen 2008-12-11 18:36 ` [PATCH 2/2] fix bootmem reservation on uninitialized node Dave Hansen 2008-12-11 18:40 ` [PATCH 0/2] Fix a bug and cleanup NUMA boot-time code Dave Hansen 2 siblings, 0 replies; 4+ messages in thread From: Dave Hansen @ 2008-12-11 18:36 UTC (permalink / raw) To: paulus Cc: Jon Tollefson, Mel Gorman, Dave Hansen, linuxppc-dev, Serge E. Hallyn --- linux-2.6.git-dave/arch/powerpc/mm/numa.c | 1 - 1 file changed, 1 deletion(-) diff -puN arch/powerpc/mm/numa.c~remove-unused-i-var arch/powerpc/mm/numa.c --- linux-2.6.git/arch/powerpc/mm/numa.c~remove-unused-i-var 2008-12-11 09:12:57.000000000 -0800 +++ linux-2.6.git-dave/arch/powerpc/mm/numa.c 2008-12-11 09:12:57.000000000 -0800 @@ -929,7 +929,6 @@ static void mark_reserved_regions_for_ni void __init do_init_bootmem(void) { int nid; - unsigned int i; min_low_pfn = 0; max_low_pfn = lmb_end_of_DRAM() >> PAGE_SHIFT; _ ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 2/2] fix bootmem reservation on uninitialized node 2008-12-11 18:36 [PATCH 0/2] Fix a bug and cleanup NUMA boot-time code Dave Hansen 2008-12-11 18:36 ` [PATCH 1/2] remove unused i var Dave Hansen @ 2008-12-11 18:36 ` Dave Hansen 2008-12-11 18:40 ` [PATCH 0/2] Fix a bug and cleanup NUMA boot-time code Dave Hansen 2 siblings, 0 replies; 4+ messages in thread From: Dave Hansen @ 2008-12-11 18:36 UTC (permalink / raw) To: paulus Cc: Jon Tollefson, Mel Gorman, Dave Hansen, linuxppc-dev, Serge E. Hallyn careful_allocation() was calling into the bootemem allocator for nodes which had not been fully initialized and caused a previous bug. http://patchwork.ozlabs.org/patch/10528/ So, I merged a few broken out loops in do_init_bootmem() to fix it. That changed the code ordering. I think this bug is triggered by having reserved areas for a node which are spanned by another node's contents. In the mark_reserved_regions_for_nid() code, we attempt to reserve the area for a node before we have allocated the NODE_DATA() for that nid. We do this since I reordered that loop. I suck. This may only present on some systems that have 16GB pages reserved. But, it can probably happen on any system that is trying to reserve large swaths of memory that happen to span other nodes' contents. This patch ensures that we do not touch bootmem for any node which has not been initialized. Signed-off-by: Dave Hansen <dave@linux.vnet.ibm.com> --- linux-2.6.git-dave/arch/powerpc/mm/numa.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff -puN arch/powerpc/mm/numa.c~fix-bad-node-reserve arch/powerpc/mm/numa.c --- linux-2.6.git/arch/powerpc/mm/numa.c~fix-bad-node-reserve 2008-12-10 14:54:18.000000000 -0800 +++ linux-2.6.git-dave/arch/powerpc/mm/numa.c 2008-12-10 14:55:33.000000000 -0800 @@ -901,10 +901,17 @@ static void mark_reserved_regions_for_ni if (end_pfn > node_ar.end_pfn) reserve_size = (node_ar.end_pfn << PAGE_SHIFT) - (start_pfn << PAGE_SHIFT); - dbg("reserve_bootmem %lx %lx nid=%d\n", physbase, - reserve_size, node_ar.nid); - reserve_bootmem_node(NODE_DATA(node_ar.nid), physbase, - reserve_size, BOOTMEM_DEFAULT); + /* + * Only worry about *this* node, others may not + * yet have valid NODE_DATA(). + */ + if (node_ar.nid == nid) { + dbg("reserve_bootmem %lx %lx nid=%d\n", + physbase, reserve_size, node_ar.nid); + reserve_bootmem_node(NODE_DATA(node_ar.nid), + physbase, reserve_size, + BOOTMEM_DEFAULT); + } /* * if reserved region is contained in the active region * then done. _ ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 0/2] Fix a bug and cleanup NUMA boot-time code 2008-12-11 18:36 [PATCH 0/2] Fix a bug and cleanup NUMA boot-time code Dave Hansen 2008-12-11 18:36 ` [PATCH 1/2] remove unused i var Dave Hansen 2008-12-11 18:36 ` [PATCH 2/2] fix bootmem reservation on uninitialized node Dave Hansen @ 2008-12-11 18:40 ` Dave Hansen 2 siblings, 0 replies; 4+ messages in thread From: Dave Hansen @ 2008-12-11 18:40 UTC (permalink / raw) To: paulus; +Cc: Jon Tollefson, Mel Gorman, linuxppc-dev, Serge E. Hallyn Gah, sorry. I resent my description for the whole series from yesterday instead of one just for these bug fixes. The first patch removes a warning in numa.c that I just caused. The second one is a rework of the bug fix that was at the head of the series yesterday, addressing some comments that Paul made. -- Dave ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2008-12-11 18:40 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2008-12-11 18:36 [PATCH 0/2] Fix a bug and cleanup NUMA boot-time code Dave Hansen 2008-12-11 18:36 ` [PATCH 1/2] remove unused i var Dave Hansen 2008-12-11 18:36 ` [PATCH 2/2] fix bootmem reservation on uninitialized node Dave Hansen 2008-12-11 18:40 ` [PATCH 0/2] Fix a bug and cleanup NUMA boot-time code Dave Hansen
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).