| Message ID | 20120426.235507.1967848292117984225.davem@davemloft.net |
|---|---|
| State | Accepted |
| Delegated to: | David Miller |
| Headers | show |
> diff --git a/arch/sparc/mm/init_64.c b/arch/sparc/mm/init_64.c > index 5b84559..def9223 100644 > --- a/arch/sparc/mm/init_64.c > +++ b/arch/sparc/mm/init_64.c > @@ -1748,22 +1748,27 @@ void __init paging_init(void) > #endif > } > > + /* Setup bootmem... */ > + last_valid_pfn = end_pfn = bootmem_init(phys_base); > + > +#ifndef CONFIG_NEED_MULTIPLE_NODES > + max_mapnr = last_valid_pfn; > +#endif This assignment looks redundant. I cannot see any uses of max_mapnr in the kernel (which is relevant for sparc64). I am perfectly aware that this was not introduced by your patch, but I looked at it anyway. > /* Once the OF device tree and MDESC have been setup, we know > * the list of possible cpus. Therefore we can allocate the > * IRQ stacks. > */ > for_each_possible_cpu(i) { > - /* XXX Use node local allocations... XXX */ > - softirq_stack[i] = __va(memblock_alloc(THREAD_SIZE, THREAD_SIZE)); > - hardirq_stack[i] = __va(memblock_alloc(THREAD_SIZE, THREAD_SIZE)); > - } > + int node = cpu_to_node(i); > > - /* Setup bootmem... */ > - last_valid_pfn = end_pfn = bootmem_init(phys_base); > + softirq_stack[i] = __alloc_bootmem_node(NODE_DATA(node), > + THREAD_SIZE, > + THREAD_SIZE, 0); Knowing that sparc64 just got converted to NO_BOOTMEM it hurts my eyes to see memblock_alloc() be replaced by __alloc_bootmem_node(). It looks like memblock_alloc_try_nid() could be used. It will try nid and revert back to all nodes if it fails to allocate memory on nid. But you would then have to memset the memory manually and convert to __va() too. Sam -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Sam Ravnborg <sam@ravnborg.org> Date: Fri, 27 Apr 2012 19:27:50 +0200 > This assignment looks redundant. I cannot see any uses of max_mapnr > in the kernel (which is relevant for sparc64). pfn_valid() via asm-generic/page.h > Knowing that sparc64 just got converted to NO_BOOTMEM it > hurts my eyes to see memblock_alloc() be replaced > by __alloc_bootmem_node(). Since the stack allocations happen now (for the sake of setting up the NUMA mappings) after bootmem init, it's prudent to use the alloc_bootmem interfaces. Not only visually, but syntactically as it provides: 1) fallbacks to non-NUMA allocations when NUMA node specific allocations fail. 2) a virtual pointer to zero'd memory so we don't need that __va() and memset crap -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: David Miller <davem@davemloft.net> Date: Fri, 27 Apr 2012 13:34:07 -0400 (EDT) > From: Sam Ravnborg <sam@ravnborg.org> > Date: Fri, 27 Apr 2012 19:27:50 +0200 > >> This assignment looks redundant. I cannot see any uses of max_mapnr >> in the kernel (which is relevant for sparc64). > > pfn_valid() via asm-generic/page.h Actually this isn't right. We don't use asm-generic/page.h on sparc64. We get the pfn_valid() definition instead from linux/mmzone.h which doesn't make use of max_mapnr. So yes we can remove this assignment and I will do so in the sparc-next tree, thanks! -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Apr 27, 2012 at 01:34:07PM -0400, David Miller wrote: > From: Sam Ravnborg <sam@ravnborg.org> > Date: Fri, 27 Apr 2012 19:27:50 +0200 > > > This assignment looks redundant. I cannot see any uses of max_mapnr > > in the kernel (which is relevant for sparc64). > > pfn_valid() via asm-generic/page.h asm-generic/page.h is for non-MMU architectures. It includes: #ifdef CONFIG_MMU #error need to prove a real asm/page.h #endif > > > Knowing that sparc64 just got converted to NO_BOOTMEM it > > hurts my eyes to see memblock_alloc() be replaced > > by __alloc_bootmem_node(). > > Since the stack allocations happen now (for the sake of setting > up the NUMA mappings) after bootmem init, it's prudent to use > the alloc_bootmem interfaces. It was my understanding that everything provided by mm/nobootmem.c is wrappers provided for backward compatibility with the old bootmem based early allocator. And that we later would make this file optional when. But I seems to be wrong here. Sam -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Sam Ravnborg <sam@ravnborg.org> Date: Fri, 27 Apr 2012 19:58:16 +0200 > On Fri, Apr 27, 2012 at 01:34:07PM -0400, David Miller wrote: >> > Knowing that sparc64 just got converted to NO_BOOTMEM it >> > hurts my eyes to see memblock_alloc() be replaced >> > by __alloc_bootmem_node(). >> >> Since the stack allocations happen now (for the sake of setting >> up the NUMA mappings) after bootmem init, it's prudent to use >> the alloc_bootmem interfaces. > > It was my understanding that everything provided by mm/nobootmem.c > is wrappers provided for backward compatibility with the old > bootmem based early allocator. > And that we later would make this file optional when. > > But I seems to be wrong here. Once the memblock functions provide NUMA node fallbacks and __va()/memset wrappers, we can certainly use memblock directly :-) -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/sparc/mm/init_64.c b/arch/sparc/mm/init_64.c index 5b84559..def9223 100644 --- a/arch/sparc/mm/init_64.c +++ b/arch/sparc/mm/init_64.c @@ -1748,22 +1748,27 @@ void __init paging_init(void) #endif } + /* Setup bootmem... */ + last_valid_pfn = end_pfn = bootmem_init(phys_base); + +#ifndef CONFIG_NEED_MULTIPLE_NODES + max_mapnr = last_valid_pfn; +#endif /* Once the OF device tree and MDESC have been setup, we know * the list of possible cpus. Therefore we can allocate the * IRQ stacks. */ for_each_possible_cpu(i) { - /* XXX Use node local allocations... XXX */ - softirq_stack[i] = __va(memblock_alloc(THREAD_SIZE, THREAD_SIZE)); - hardirq_stack[i] = __va(memblock_alloc(THREAD_SIZE, THREAD_SIZE)); - } + int node = cpu_to_node(i); - /* Setup bootmem... */ - last_valid_pfn = end_pfn = bootmem_init(phys_base); + softirq_stack[i] = __alloc_bootmem_node(NODE_DATA(node), + THREAD_SIZE, + THREAD_SIZE, 0); + hardirq_stack[i] = __alloc_bootmem_node(NODE_DATA(node), + THREAD_SIZE, + THREAD_SIZE, 0); + } -#ifndef CONFIG_NEED_MULTIPLE_NODES - max_mapnr = last_valid_pfn; -#endif kernel_physical_mapping_init(); {
Signed-off-by: David S. Miller <davem@davemloft.net> --- Committed to sparc-next arch/sparc/mm/init_64.c | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-)