Patchwork sparc64: Use node local allocations for IRQ stacks.

login
register
mail settings
Submitter David Miller
Date April 27, 2012, 3:55 a.m.
Message ID <20120426.235507.1967848292117984225.davem@davemloft.net>
Download mbox | patch
Permalink /patch/155374/
State Accepted
Delegated to: David Miller
Headers show

Comments

David Miller - April 27, 2012, 3:55 a.m.
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(-)
Sam Ravnborg - April 27, 2012, 5:27 p.m.
> 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
David Miller - April 27, 2012, 5:34 p.m.
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
David Miller - April 27, 2012, 5:56 p.m.
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
Sam Ravnborg - April 27, 2012, 5:58 p.m.
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
David Miller - April 27, 2012, 6:08 p.m.
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

Patch

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();
 
 	{