Re: [PATCH 6/8] cleanup do_init_bootmem()

2008-12-15 Thread Paul Mackerras
Dave Hansen writes:

 I'm debating whether this is worth it. It makes this a bit more clean
 looking, but doesn't seriously enhance readability.  But, I do think
 it helps a bit.

I get this when compiling a pseries config (with the patches up to
this point applied but not 7/8 or 8/8):

  CC  arch/powerpc/mm/numa.o
/home/paulus/kernel/powerpc/arch/powerpc/mm/numa.c: In function 
'do_init_bootmem_node':
/home/paulus/kernel/powerpc/arch/powerpc/mm/numa.c:949: error: 'nid' undeclared 
(first use in this function)
/home/paulus/kernel/powerpc/arch/powerpc/mm/numa.c:949: error: (Each undeclared 
identifier is reported only once
/home/paulus/kernel/powerpc/arch/powerpc/mm/numa.c:949: error: for each 
function it appears in.)
make[2]: *** [arch/powerpc/mm/numa.o] Error 1

Paul.
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


[PATCH 6/8] cleanup do_init_bootmem()

2008-12-09 Thread Dave Hansen

I'm debating whether this is worth it. It makes this a bit more clean
looking, but doesn't seriously enhance readability.  But, I do think
it helps a bit.

Thoughts?

Signed-off-by: Dave Hansen [EMAIL PROTECTED]
---

 linux-2.6.git-dave/arch/powerpc/mm/numa.c |  104 +++---
 1 file changed, 55 insertions(+), 49 deletions(-)

diff -puN arch/powerpc/mm/numa.c~cleanup-careful_allocation3 
arch/powerpc/mm/numa.c
--- linux-2.6.git/arch/powerpc/mm/numa.c~cleanup-careful_allocation3
2008-12-09 10:16:07.0 -0800
+++ linux-2.6.git-dave/arch/powerpc/mm/numa.c   2008-12-09 10:16:07.0 
-0800
@@ -938,6 +938,59 @@ static void mark_reserved_regions_for_ni
}
 }
 
+void do_init_bootmem_node(int node)
+{
+   unsigned long start_pfn, end_pfn;
+   void *bootmem_vaddr;
+   unsigned long bootmap_pages;
+
+   dbg(node %d is online\n, nid);
+   get_pfn_range_for_nid(nid, start_pfn, end_pfn);
+
+   /*
+* Allocate the node structure node local if possible
+*
+* Be careful moving this around, as it relies on all
+* previous nodes' bootmem to be initialized and have
+* all reserved areas marked.
+*/
+   NODE_DATA(nid) = careful_zallocation(nid,
+   sizeof(struct pglist_data),
+   SMP_CACHE_BYTES, end_pfn);
+
+   dbg(node %d\n, nid);
+   dbg(NODE_DATA() = %p\n, NODE_DATA(nid));
+
+   NODE_DATA(nid)-bdata = bootmem_node_data[nid];
+   NODE_DATA(nid)-node_start_pfn = start_pfn;
+   NODE_DATA(nid)-node_spanned_pages = end_pfn - start_pfn;
+
+   if (NODE_DATA(nid)-node_spanned_pages == 0)
+   return;
+
+   dbg(start_paddr = %lx\n, start_pfn  PAGE_SHIFT);
+   dbg(end_paddr = %lx\n, end_pfn  PAGE_SHIFT);
+
+   bootmap_pages = bootmem_bootmap_pages(end_pfn - start_pfn);
+   bootmem_vaddr = careful_zallocation(nid,
+   bootmap_pages  PAGE_SHIFT,
+   PAGE_SIZE, end_pfn);
+
+   dbg(bootmap_vaddr = %p\n, bootmem_vaddr);
+
+   init_bootmem_node(NODE_DATA(nid),
+ __pa(bootmem_vaddr)  PAGE_SHIFT,
+ start_pfn, end_pfn);
+
+   free_bootmem_with_active_regions(nid, end_pfn);
+   /*
+* Be very careful about moving this around.  Future
+* calls to careful_zallocation() depend on this getting
+* done correctly.
+*/
+   mark_reserved_regions_for_nid(nid);
+   sparse_memory_present_with_active_regions(nid);
+}
 
 void __init do_init_bootmem(void)
 {
@@ -958,55 +1011,8 @@ void __init do_init_bootmem(void)
  (void *)(unsigned long)boot_cpuid);
 
for_each_online_node(nid) {
-   unsigned long start_pfn, end_pfn;
-   void *bootmem_vaddr;
-   unsigned long bootmap_pages;
-
-   get_pfn_range_for_nid(nid, start_pfn, end_pfn);
-
-   /*
-* Allocate the node structure node local if possible
-*
-* Be careful moving this around, as it relies on all
-* previous nodes' bootmem to be initialized and have
-* all reserved areas marked.
-*/
-   NODE_DATA(nid) = careful_zallocation(nid,
-   sizeof(struct pglist_data),
-   SMP_CACHE_BYTES, end_pfn);
-
-   dbg(node %d\n, nid);
-   dbg(NODE_DATA() = %p\n, NODE_DATA(nid));
-
-   NODE_DATA(nid)-bdata = bootmem_node_data[nid];
-   NODE_DATA(nid)-node_start_pfn = start_pfn;
-   NODE_DATA(nid)-node_spanned_pages = end_pfn - start_pfn;
-
-   if (NODE_DATA(nid)-node_spanned_pages == 0)
-   continue;
-
-   dbg(start_paddr = %lx\n, start_pfn  PAGE_SHIFT);
-   dbg(end_paddr = %lx\n, end_pfn  PAGE_SHIFT);
-
-   bootmap_pages = bootmem_bootmap_pages(end_pfn - start_pfn);
-   bootmem_vaddr = careful_zallocation(nid,
-   bootmap_pages  PAGE_SHIFT,
-   PAGE_SIZE, end_pfn);
-
-   dbg(bootmap_vaddr = %p\n, bootmem_vaddr);
-
-   init_bootmem_node(NODE_DATA(nid),
- __pa(bootmem_vaddr)  PAGE_SHIFT,
- start_pfn, end_pfn);
-
-   free_bootmem_with_active_regions(nid, end_pfn);
-   /*
-* Be very careful about moving this around.  Future
-* calls to careful_zallocation() depend on this getting
-* done correctly.
-*/
-   mark_reserved_regions_for_nid(nid);
-   sparse_memory_present_with_active_regions(nid);
+   dbg(node %d: marked online, initializing bootmem\n, nid);
+   

Re: [PATCH 6/8] cleanup do_init_bootmem()

2008-12-09 Thread Serge E. Hallyn
Quoting Dave Hansen ([EMAIL PROTECTED]):
 
 I'm debating whether this is worth it. It makes this a bit more clean
 looking, but doesn't seriously enhance readability.  But, I do think
 it helps a bit.
 
 Thoughts?

Absolutely.  do_init_bootmem_node() is *still* a bit largish,
but far better broken out.
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev