diff mbox

sh: switch to NO_BOOTMEM

Message ID 20180511134559.13464-1-robh@kernel.org (mailing list archive)
State New, archived
Headers show

Commit Message

Rob Herring May 11, 2018, 1:45 p.m. UTC
Commit 0fa1c579349f ("of/fdt: use memblock_virt_alloc for early alloc")
inadvertently switched the DT unflattening allocations from memblock to
bootmem which doesn't work because the unflattening happens before
bootmem is initialized. Swapping the order of bootmem init and
unflattening could also fix this, but removing bootmem is desired. So
enable NO_BOOTMEM on SH like other architectures have done.

Fixes: 0fa1c579349f ("of/fdt: use memblock_virt_alloc for early alloc")
Reported-by: Rich Felker <dalias@libc.org>
Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
Signed-off-by: Rob Herring <robh@kernel.org>
---
This is compile tested only, but similar to microblaze and h8300 
conversions. The additional complexity for SH is NUMA support (which to 
me looks like an abuse of NUMA support to map a small amount of 
on-chip? RAM to NUMA nodes). 

 arch/sh/Kconfig        |  1 +
 arch/sh/kernel/setup.c |  1 -
 arch/sh/mm/init.c      | 68 ++++--------------------------------------
 arch/sh/mm/numa.c      | 19 ------------
 4 files changed, 7 insertions(+), 82 deletions(-)

Comments

Rich Felker May 11, 2018, 3:02 p.m. UTC | #1
On Fri, May 11, 2018 at 08:45:59AM -0500, Rob Herring wrote:
> Commit 0fa1c579349f ("of/fdt: use memblock_virt_alloc for early alloc")
> inadvertently switched the DT unflattening allocations from memblock to
> bootmem which doesn't work because the unflattening happens before
> bootmem is initialized. Swapping the order of bootmem init and
> unflattening could also fix this, but removing bootmem is desired. So
> enable NO_BOOTMEM on SH like other architectures have done.
> 
> Fixes: 0fa1c579349f ("of/fdt: use memblock_virt_alloc for early alloc")
> Reported-by: Rich Felker <dalias@libc.org>
> Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
> This is compile tested only, but similar to microblaze and h8300 
> conversions. The additional complexity for SH is NUMA support (which to 
> me looks like an abuse of NUMA support to map a small amount of 
> on-chip? RAM to NUMA nodes). 

Thanks! I was just reading the corresponding microblaze commit, and
think this approach makes sense. I'll test it now with both DT and
non-DT sh systems and let you know if it works. If it's good would you
like me to send it upstream via arch/sh? I already have 2
regression-fix patches to submit in a pull request asap.

Rich


>  arch/sh/Kconfig        |  1 +
>  arch/sh/kernel/setup.c |  1 -
>  arch/sh/mm/init.c      | 68 ++++--------------------------------------
>  arch/sh/mm/numa.c      | 19 ------------
>  4 files changed, 7 insertions(+), 82 deletions(-)
> 
> diff --git a/arch/sh/Kconfig b/arch/sh/Kconfig
> index 97fe29316476..1851eaeee131 100644
> --- a/arch/sh/Kconfig
> +++ b/arch/sh/Kconfig
> @@ -9,6 +9,7 @@ config SUPERH
>  	select HAVE_IDE if HAS_IOPORT_MAP
>  	select HAVE_MEMBLOCK
>  	select HAVE_MEMBLOCK_NODE_MAP
> +	select NO_BOOTMEM
>  	select ARCH_DISCARD_MEMBLOCK
>  	select HAVE_OPROFILE
>  	select HAVE_GENERIC_DMA_COHERENT
> diff --git a/arch/sh/kernel/setup.c b/arch/sh/kernel/setup.c
> index d34e998b809f..c286cf5da6e7 100644
> --- a/arch/sh/kernel/setup.c
> +++ b/arch/sh/kernel/setup.c
> @@ -11,7 +11,6 @@
>  #include <linux/ioport.h>
>  #include <linux/init.h>
>  #include <linux/initrd.h>
> -#include <linux/bootmem.h>
>  #include <linux/console.h>
>  #include <linux/root_dev.h>
>  #include <linux/utsname.h>
> diff --git a/arch/sh/mm/init.c b/arch/sh/mm/init.c
> index ce0bbaa7e404..4034035fbede 100644
> --- a/arch/sh/mm/init.c
> +++ b/arch/sh/mm/init.c
> @@ -211,59 +211,15 @@ void __init allocate_pgdat(unsigned int nid)
>  
>  	NODE_DATA(nid) = __va(phys);
>  	memset(NODE_DATA(nid), 0, sizeof(struct pglist_data));
> -
> -	NODE_DATA(nid)->bdata = &bootmem_node_data[nid];
>  #endif
>  
>  	NODE_DATA(nid)->node_start_pfn = start_pfn;
>  	NODE_DATA(nid)->node_spanned_pages = end_pfn - start_pfn;
>  }
>  
> -static void __init bootmem_init_one_node(unsigned int nid)
> -{
> -	unsigned long total_pages, paddr;
> -	unsigned long end_pfn;
> -	struct pglist_data *p;
> -
> -	p = NODE_DATA(nid);
> -
> -	/* Nothing to do.. */
> -	if (!p->node_spanned_pages)
> -		return;
> -
> -	end_pfn = pgdat_end_pfn(p);
> -
> -	total_pages = bootmem_bootmap_pages(p->node_spanned_pages);
> -
> -	paddr = memblock_alloc(total_pages << PAGE_SHIFT, PAGE_SIZE);
> -	if (!paddr)
> -		panic("Can't allocate bootmap for nid[%d]\n", nid);
> -
> -	init_bootmem_node(p, paddr >> PAGE_SHIFT, p->node_start_pfn, end_pfn);
> -
> -	free_bootmem_with_active_regions(nid, end_pfn);
> -
> -	/*
> -	 * XXX Handle initial reservations for the system memory node
> -	 * only for the moment, we'll refactor this later for handling
> -	 * reservations in other nodes.
> -	 */
> -	if (nid == 0) {
> -		struct memblock_region *reg;
> -
> -		/* Reserve the sections we're already using. */
> -		for_each_memblock(reserved, reg) {
> -			reserve_bootmem(reg->base, reg->size, BOOTMEM_DEFAULT);
> -		}
> -	}
> -
> -	sparse_memory_present_with_active_regions(nid);
> -}
> -
>  static void __init do_init_bootmem(void)
>  {
>  	struct memblock_region *reg;
> -	int i;
>  
>  	/* Add active regions with valid PFNs. */
>  	for_each_memblock(memory, reg) {
> @@ -279,9 +235,12 @@ static void __init do_init_bootmem(void)
>  
>  	plat_mem_setup();
>  
> -	for_each_online_node(i)
> -		bootmem_init_one_node(i);
> +	for_each_memblock(memory, reg) {
> +		int nid = memblock_get_region_node(reg);
>  
> +		memory_present(nid, memblock_region_memory_base_pfn(reg),
> +			memblock_region_memory_end_pfn(reg));
> +	}
>  	sparse_init();
>  }
>  
> @@ -322,7 +281,6 @@ void __init paging_init(void)
>  {
>  	unsigned long max_zone_pfns[MAX_NR_ZONES];
>  	unsigned long vaddr, end;
> -	int nid;
>  
>  	sh_mv.mv_mem_init();
>  
> @@ -377,21 +335,7 @@ void __init paging_init(void)
>  	kmap_coherent_init();
>  
>  	memset(max_zone_pfns, 0, sizeof(max_zone_pfns));
> -
> -	for_each_online_node(nid) {
> -		pg_data_t *pgdat = NODE_DATA(nid);
> -		unsigned long low, start_pfn;
> -
> -		start_pfn = pgdat->bdata->node_min_pfn;
> -		low = pgdat->bdata->node_low_pfn;
> -
> -		if (max_zone_pfns[ZONE_NORMAL] < low)
> -			max_zone_pfns[ZONE_NORMAL] = low;
> -
> -		printk("Node %u: start_pfn = 0x%lx, low = 0x%lx\n",
> -		       nid, start_pfn, low);
> -	}
> -
> +	max_zone_pfns[ZONE_NORMAL] = max_low_pfn;
>  	free_area_init_nodes(max_zone_pfns);
>  }
>  
> diff --git a/arch/sh/mm/numa.c b/arch/sh/mm/numa.c
> index 05713d190247..830e8b3684e4 100644
> --- a/arch/sh/mm/numa.c
> +++ b/arch/sh/mm/numa.c
> @@ -8,7 +8,6 @@
>   * for more details.
>   */
>  #include <linux/module.h>
> -#include <linux/bootmem.h>
>  #include <linux/memblock.h>
>  #include <linux/mm.h>
>  #include <linux/numa.h>
> @@ -26,9 +25,7 @@ EXPORT_SYMBOL_GPL(node_data);
>   */
>  void __init setup_bootmem_node(int nid, unsigned long start, unsigned long end)
>  {
> -	unsigned long bootmap_pages;
>  	unsigned long start_pfn, end_pfn;
> -	unsigned long bootmem_paddr;
>  
>  	/* Don't allow bogus node assignment */
>  	BUG_ON(nid >= MAX_NUMNODES || nid <= 0);
> @@ -48,25 +45,9 @@ void __init setup_bootmem_node(int nid, unsigned long start, unsigned long end)
>  					     SMP_CACHE_BYTES, end));
>  	memset(NODE_DATA(nid), 0, sizeof(struct pglist_data));
>  
> -	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;
>  
> -	/* Node-local bootmap */
> -	bootmap_pages = bootmem_bootmap_pages(end_pfn - start_pfn);
> -	bootmem_paddr = memblock_alloc_base(bootmap_pages << PAGE_SHIFT,
> -				       PAGE_SIZE, end);
> -	init_bootmem_node(NODE_DATA(nid), bootmem_paddr >> PAGE_SHIFT,
> -			  start_pfn, end_pfn);
> -
> -	free_bootmem_with_active_regions(nid, end_pfn);
> -
> -	/* Reserve the pgdat and bootmap space with the bootmem allocator */
> -	reserve_bootmem_node(NODE_DATA(nid), start_pfn << PAGE_SHIFT,
> -			     sizeof(struct pglist_data), BOOTMEM_DEFAULT);
> -	reserve_bootmem_node(NODE_DATA(nid), bootmem_paddr,
> -			     bootmap_pages << PAGE_SHIFT, BOOTMEM_DEFAULT);
> -
>  	/* It's up */
>  	node_set_online(nid);
>  
> -- 
> 2.17.0
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rob Herring May 11, 2018, 3:37 p.m. UTC | #2
On Fri, May 11, 2018 at 10:02 AM, Rich Felker <dalias@libc.org> wrote:
> On Fri, May 11, 2018 at 08:45:59AM -0500, Rob Herring wrote:
>> Commit 0fa1c579349f ("of/fdt: use memblock_virt_alloc for early alloc")
>> inadvertently switched the DT unflattening allocations from memblock to
>> bootmem which doesn't work because the unflattening happens before
>> bootmem is initialized. Swapping the order of bootmem init and
>> unflattening could also fix this, but removing bootmem is desired. So
>> enable NO_BOOTMEM on SH like other architectures have done.
>>
>> Fixes: 0fa1c579349f ("of/fdt: use memblock_virt_alloc for early alloc")
>> Reported-by: Rich Felker <dalias@libc.org>
>> Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
>> Signed-off-by: Rob Herring <robh@kernel.org>
>> ---
>> This is compile tested only, but similar to microblaze and h8300
>> conversions. The additional complexity for SH is NUMA support (which to
>> me looks like an abuse of NUMA support to map a small amount of
>> on-chip? RAM to NUMA nodes).
>
> Thanks! I was just reading the corresponding microblaze commit, and
> think this approach makes sense. I'll test it now with both DT and
> non-DT sh systems and let you know if it works. If it's good would you
> like me to send it upstream via arch/sh? I already have 2
> regression-fix patches to submit in a pull request asap.

Yes, please do.

Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rich Felker May 11, 2018, 3:55 p.m. UTC | #3
On Fri, May 11, 2018 at 10:37:42AM -0500, Rob Herring wrote:
> On Fri, May 11, 2018 at 10:02 AM, Rich Felker <dalias@libc.org> wrote:
> > On Fri, May 11, 2018 at 08:45:59AM -0500, Rob Herring wrote:
> >> Commit 0fa1c579349f ("of/fdt: use memblock_virt_alloc for early alloc")
> >> inadvertently switched the DT unflattening allocations from memblock to
> >> bootmem which doesn't work because the unflattening happens before
> >> bootmem is initialized. Swapping the order of bootmem init and
> >> unflattening could also fix this, but removing bootmem is desired. So
> >> enable NO_BOOTMEM on SH like other architectures have done.
> >>
> >> Fixes: 0fa1c579349f ("of/fdt: use memblock_virt_alloc for early alloc")
> >> Reported-by: Rich Felker <dalias@libc.org>
> >> Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
> >> Signed-off-by: Rob Herring <robh@kernel.org>
> >> ---
> >> This is compile tested only, but similar to microblaze and h8300
> >> conversions. The additional complexity for SH is NUMA support (which to
> >> me looks like an abuse of NUMA support to map a small amount of
> >> on-chip? RAM to NUMA nodes).
> >
> > Thanks! I was just reading the corresponding microblaze commit, and
> > think this approach makes sense. I'll test it now with both DT and
> > non-DT sh systems and let you know if it works. If it's good would you
> > like me to send it upstream via arch/sh? I already have 2
> > regression-fix patches to submit in a pull request asap.
> 
> Yes, please do.

OK. So far looks good. J2 works again (DT-based) and I'm now doing a
new build to test under qemu (r2d board) for non-DT sh.

Rich
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Christoph Hellwig May 14, 2018, 12:03 p.m. UTC | #4
> unflattening could also fix this, but removing bootmem is desired. So
> enable NO_BOOTMEM on SH like other architectures have done.

Can you send out similarly shoot in the wild conversion for the
remaining architectures while you are at it?  We still seem to be
missing: alpha, c6x, hexagon, ia64, m68k, mips, nios2 and unicore32.
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rob Herring May 14, 2018, 8:01 p.m. UTC | #5
On Mon, May 14, 2018 at 7:03 AM, Christoph Hellwig <hch@infradead.org> wrote:
>> unflattening could also fix this, but removing bootmem is desired. So
>> enable NO_BOOTMEM on SH like other architectures have done.
>
> Can you send out similarly shoot in the wild conversion for the
> remaining architectures while you are at it?  We still seem to be
> missing: alpha, c6x, hexagon, ia64, m68k, mips, nios2 and unicore32.

Sparc (32-bit only) too.

I might be talked into the remaining arches that support DT which
would be c6x, hexagon, mips, and nios2. Though, looking at the MIPS
arch code always makes me cry (just try "git grep 'bootmem'
arch/mips/").

Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/arch/sh/Kconfig b/arch/sh/Kconfig
index 97fe29316476..1851eaeee131 100644
--- a/arch/sh/Kconfig
+++ b/arch/sh/Kconfig
@@ -9,6 +9,7 @@  config SUPERH
 	select HAVE_IDE if HAS_IOPORT_MAP
 	select HAVE_MEMBLOCK
 	select HAVE_MEMBLOCK_NODE_MAP
+	select NO_BOOTMEM
 	select ARCH_DISCARD_MEMBLOCK
 	select HAVE_OPROFILE
 	select HAVE_GENERIC_DMA_COHERENT
diff --git a/arch/sh/kernel/setup.c b/arch/sh/kernel/setup.c
index d34e998b809f..c286cf5da6e7 100644
--- a/arch/sh/kernel/setup.c
+++ b/arch/sh/kernel/setup.c
@@ -11,7 +11,6 @@ 
 #include <linux/ioport.h>
 #include <linux/init.h>
 #include <linux/initrd.h>
-#include <linux/bootmem.h>
 #include <linux/console.h>
 #include <linux/root_dev.h>
 #include <linux/utsname.h>
diff --git a/arch/sh/mm/init.c b/arch/sh/mm/init.c
index ce0bbaa7e404..4034035fbede 100644
--- a/arch/sh/mm/init.c
+++ b/arch/sh/mm/init.c
@@ -211,59 +211,15 @@  void __init allocate_pgdat(unsigned int nid)
 
 	NODE_DATA(nid) = __va(phys);
 	memset(NODE_DATA(nid), 0, sizeof(struct pglist_data));
-
-	NODE_DATA(nid)->bdata = &bootmem_node_data[nid];
 #endif
 
 	NODE_DATA(nid)->node_start_pfn = start_pfn;
 	NODE_DATA(nid)->node_spanned_pages = end_pfn - start_pfn;
 }
 
-static void __init bootmem_init_one_node(unsigned int nid)
-{
-	unsigned long total_pages, paddr;
-	unsigned long end_pfn;
-	struct pglist_data *p;
-
-	p = NODE_DATA(nid);
-
-	/* Nothing to do.. */
-	if (!p->node_spanned_pages)
-		return;
-
-	end_pfn = pgdat_end_pfn(p);
-
-	total_pages = bootmem_bootmap_pages(p->node_spanned_pages);
-
-	paddr = memblock_alloc(total_pages << PAGE_SHIFT, PAGE_SIZE);
-	if (!paddr)
-		panic("Can't allocate bootmap for nid[%d]\n", nid);
-
-	init_bootmem_node(p, paddr >> PAGE_SHIFT, p->node_start_pfn, end_pfn);
-
-	free_bootmem_with_active_regions(nid, end_pfn);
-
-	/*
-	 * XXX Handle initial reservations for the system memory node
-	 * only for the moment, we'll refactor this later for handling
-	 * reservations in other nodes.
-	 */
-	if (nid == 0) {
-		struct memblock_region *reg;
-
-		/* Reserve the sections we're already using. */
-		for_each_memblock(reserved, reg) {
-			reserve_bootmem(reg->base, reg->size, BOOTMEM_DEFAULT);
-		}
-	}
-
-	sparse_memory_present_with_active_regions(nid);
-}
-
 static void __init do_init_bootmem(void)
 {
 	struct memblock_region *reg;
-	int i;
 
 	/* Add active regions with valid PFNs. */
 	for_each_memblock(memory, reg) {
@@ -279,9 +235,12 @@  static void __init do_init_bootmem(void)
 
 	plat_mem_setup();
 
-	for_each_online_node(i)
-		bootmem_init_one_node(i);
+	for_each_memblock(memory, reg) {
+		int nid = memblock_get_region_node(reg);
 
+		memory_present(nid, memblock_region_memory_base_pfn(reg),
+			memblock_region_memory_end_pfn(reg));
+	}
 	sparse_init();
 }
 
@@ -322,7 +281,6 @@  void __init paging_init(void)
 {
 	unsigned long max_zone_pfns[MAX_NR_ZONES];
 	unsigned long vaddr, end;
-	int nid;
 
 	sh_mv.mv_mem_init();
 
@@ -377,21 +335,7 @@  void __init paging_init(void)
 	kmap_coherent_init();
 
 	memset(max_zone_pfns, 0, sizeof(max_zone_pfns));
-
-	for_each_online_node(nid) {
-		pg_data_t *pgdat = NODE_DATA(nid);
-		unsigned long low, start_pfn;
-
-		start_pfn = pgdat->bdata->node_min_pfn;
-		low = pgdat->bdata->node_low_pfn;
-
-		if (max_zone_pfns[ZONE_NORMAL] < low)
-			max_zone_pfns[ZONE_NORMAL] = low;
-
-		printk("Node %u: start_pfn = 0x%lx, low = 0x%lx\n",
-		       nid, start_pfn, low);
-	}
-
+	max_zone_pfns[ZONE_NORMAL] = max_low_pfn;
 	free_area_init_nodes(max_zone_pfns);
 }
 
diff --git a/arch/sh/mm/numa.c b/arch/sh/mm/numa.c
index 05713d190247..830e8b3684e4 100644
--- a/arch/sh/mm/numa.c
+++ b/arch/sh/mm/numa.c
@@ -8,7 +8,6 @@ 
  * for more details.
  */
 #include <linux/module.h>
-#include <linux/bootmem.h>
 #include <linux/memblock.h>
 #include <linux/mm.h>
 #include <linux/numa.h>
@@ -26,9 +25,7 @@  EXPORT_SYMBOL_GPL(node_data);
  */
 void __init setup_bootmem_node(int nid, unsigned long start, unsigned long end)
 {
-	unsigned long bootmap_pages;
 	unsigned long start_pfn, end_pfn;
-	unsigned long bootmem_paddr;
 
 	/* Don't allow bogus node assignment */
 	BUG_ON(nid >= MAX_NUMNODES || nid <= 0);
@@ -48,25 +45,9 @@  void __init setup_bootmem_node(int nid, unsigned long start, unsigned long end)
 					     SMP_CACHE_BYTES, end));
 	memset(NODE_DATA(nid), 0, sizeof(struct pglist_data));
 
-	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;
 
-	/* Node-local bootmap */
-	bootmap_pages = bootmem_bootmap_pages(end_pfn - start_pfn);
-	bootmem_paddr = memblock_alloc_base(bootmap_pages << PAGE_SHIFT,
-				       PAGE_SIZE, end);
-	init_bootmem_node(NODE_DATA(nid), bootmem_paddr >> PAGE_SHIFT,
-			  start_pfn, end_pfn);
-
-	free_bootmem_with_active_regions(nid, end_pfn);
-
-	/* Reserve the pgdat and bootmap space with the bootmem allocator */
-	reserve_bootmem_node(NODE_DATA(nid), start_pfn << PAGE_SHIFT,
-			     sizeof(struct pglist_data), BOOTMEM_DEFAULT);
-	reserve_bootmem_node(NODE_DATA(nid), bootmem_paddr,
-			     bootmap_pages << PAGE_SHIFT, BOOTMEM_DEFAULT);
-
 	/* It's up */
 	node_set_online(nid);