* [PATCH 0/2] sparc32: switch to NO_BOOTMEM @ 2018-08-02 11:53 ` Mike Rapoport 0 siblings, 0 replies; 10+ messages in thread From: Mike Rapoport @ 2018-08-02 11:53 UTC (permalink / raw) To: David S. Miller Cc: Sam Ravnborg, Michal Hocko, sparclinux, linux-mm, linux-kernel, Mike Rapoport Hi, These patches convert sparc32 to use memblock + nobootmem. I've made the conversion as simple as possible, just enough to allow moving HAVE_MEMBLOCK and NO_BOOTMEM to the common SPARC configuration. Mike Rapoport (2): sparc32: switch to NO_BOOTMEM sparc32: tidy up ramdisk memory reservation arch/sparc/Kconfig | 4 +-- arch/sparc/mm/init_32.c | 90 +++++++++++++++---------------------------------- 2 files changed, 29 insertions(+), 65 deletions(-) -- 2.7.4 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 0/2] sparc32: switch to NO_BOOTMEM @ 2018-08-02 11:53 ` Mike Rapoport 0 siblings, 0 replies; 10+ messages in thread From: Mike Rapoport @ 2018-08-02 11:53 UTC (permalink / raw) To: David S. Miller Cc: Sam Ravnborg, Michal Hocko, sparclinux, linux-mm, linux-kernel, Mike Rapoport Hi, These patches convert sparc32 to use memblock + nobootmem. I've made the conversion as simple as possible, just enough to allow moving HAVE_MEMBLOCK and NO_BOOTMEM to the common SPARC configuration. Mike Rapoport (2): sparc32: switch to NO_BOOTMEM sparc32: tidy up ramdisk memory reservation arch/sparc/Kconfig | 4 +-- arch/sparc/mm/init_32.c | 90 +++++++++++++++---------------------------------- 2 files changed, 29 insertions(+), 65 deletions(-) -- 2.7.4 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/2] sparc32: switch to NO_BOOTMEM 2018-08-02 11:53 ` Mike Rapoport @ 2018-08-02 11:53 ` Mike Rapoport -1 siblings, 0 replies; 10+ messages in thread From: Mike Rapoport @ 2018-08-02 11:53 UTC (permalink / raw) To: David S. Miller Cc: Sam Ravnborg, Michal Hocko, sparclinux, linux-mm, linux-kernel, Mike Rapoport Each populated sparc_phys_bank is added to memblock.memory. The reserve_bootmem() calls are replaced with memblock_reserve(), and the bootmem bitmap initialization is droppped. Signed-off-by: Mike Rapoport <rppt@linux.vnet.ibm.com> --- arch/sparc/Kconfig | 4 +-- arch/sparc/mm/init_32.c | 74 ++++++++++++++----------------------------------- 2 files changed, 23 insertions(+), 55 deletions(-) diff --git a/arch/sparc/Kconfig b/arch/sparc/Kconfig index 0f535de..0a874c8 100644 --- a/arch/sparc/Kconfig +++ b/arch/sparc/Kconfig @@ -45,6 +45,8 @@ config SPARC select LOCKDEP_SMALL if LOCKDEP select NEED_DMA_MAP_STATE select NEED_SG_DMA_LENGTH + select HAVE_MEMBLOCK + select NO_BOOTMEM config SPARC32 def_bool !64BIT @@ -60,7 +62,6 @@ config SPARC64 select HAVE_KRETPROBES select HAVE_KPROBES select HAVE_RCU_TABLE_FREE if SMP - select HAVE_MEMBLOCK select HAVE_MEMBLOCK_NODE_MAP select HAVE_ARCH_TRANSPARENT_HUGEPAGE select HAVE_DYNAMIC_FTRACE @@ -79,7 +80,6 @@ config SPARC64 select IRQ_PREFLOW_FASTEOI select ARCH_HAVE_NMI_SAFE_CMPXCHG select HAVE_C_RECORDMCOUNT - select NO_BOOTMEM select HAVE_ARCH_AUDITSYSCALL select ARCH_SUPPORTS_ATOMIC_RMW select HAVE_NMI diff --git a/arch/sparc/mm/init_32.c b/arch/sparc/mm/init_32.c index 95fe4f0..5117a5e 100644 --- a/arch/sparc/mm/init_32.c +++ b/arch/sparc/mm/init_32.c @@ -23,6 +23,7 @@ #include <linux/init.h> #include <linux/highmem.h> #include <linux/bootmem.h> +#include <linux/memblock.h> #include <linux/pagemap.h> #include <linux/poison.h> #include <linux/gfp.h> @@ -103,11 +104,15 @@ static unsigned long calc_max_low_pfn(void) unsigned long __init bootmem_init(unsigned long *pages_avail) { - unsigned long bootmap_size, start_pfn; + unsigned long start_pfn; unsigned long end_of_phys_memory = 0UL; - unsigned long bootmap_pfn, bytes_avail, size; + unsigned long bytes_avail, size; + unsigned long high_pages = 0UL; int i; + memblock_set_bottom_up(true); + memblock_allow_resize(); + bytes_avail = 0UL; for (i = 0; sp_banks[i].num_bytes != 0; i++) { end_of_phys_memory = sp_banks[i].base_addr + @@ -124,12 +129,15 @@ unsigned long __init bootmem_init(unsigned long *pages_avail) if (sp_banks[i].num_bytes == 0) { sp_banks[i].base_addr = 0xdeadbeef; } else { + memblock_add(sp_banks[i].base_addr, + sp_banks[i].num_bytes); sp_banks[i+1].num_bytes = 0; sp_banks[i+1].base_addr = 0xdeadbeef; } break; } } + memblock_add(sp_banks[i].base_addr, sp_banks[i].num_bytes); } /* Start with page aligned address of last symbol in kernel @@ -140,8 +148,6 @@ unsigned long __init bootmem_init(unsigned long *pages_avail) /* Now shift down to get the real physical page frame number. */ start_pfn >>= PAGE_SHIFT; - bootmap_pfn = start_pfn; - max_pfn = end_of_phys_memory >> PAGE_SHIFT; max_low_pfn = max_pfn; @@ -150,12 +156,16 @@ unsigned long __init bootmem_init(unsigned long *pages_avail) if (max_low_pfn > pfn_base + (SRMMU_MAXMEM >> PAGE_SHIFT)) { highstart_pfn = pfn_base + (SRMMU_MAXMEM >> PAGE_SHIFT); max_low_pfn = calc_max_low_pfn(); + high_pages = calc_highpages(); printk(KERN_NOTICE "%ldMB HIGHMEM available.\n", - calc_highpages() >> (20 - PAGE_SHIFT)); + high_pages >> (20 - PAGE_SHIFT)); } #ifdef CONFIG_BLK_DEV_INITRD - /* Now have to check initial ramdisk, so that bootmap does not overwrite it */ + /* + * Now have to check initial ramdisk, so that it won't pass + * the end of memory + */ if (sparc_ramdisk_image) { if (sparc_ramdisk_image >= (unsigned long)&_end - 2 * PAGE_SIZE) sparc_ramdisk_image -= KERNBASE; @@ -167,67 +177,25 @@ unsigned long __init bootmem_init(unsigned long *pages_avail) initrd_end, end_of_phys_memory); initrd_start = 0; } - if (initrd_start) { - if (initrd_start >= (start_pfn << PAGE_SHIFT) && - initrd_start < (start_pfn << PAGE_SHIFT) + 2 * PAGE_SIZE) - bootmap_pfn = PAGE_ALIGN (initrd_end) >> PAGE_SHIFT; - } } -#endif - /* Initialize the boot-time allocator. */ - bootmap_size = init_bootmem_node(NODE_DATA(0), bootmap_pfn, pfn_base, - max_low_pfn); - - /* Now register the available physical memory with the - * allocator. - */ - *pages_avail = 0; - for (i = 0; sp_banks[i].num_bytes != 0; i++) { - unsigned long curr_pfn, last_pfn; - - curr_pfn = sp_banks[i].base_addr >> PAGE_SHIFT; - if (curr_pfn >= max_low_pfn) - break; - - last_pfn = (sp_banks[i].base_addr + sp_banks[i].num_bytes) >> PAGE_SHIFT; - if (last_pfn > max_low_pfn) - last_pfn = max_low_pfn; - - /* - * .. finally, did all the rounding and playing - * around just make the area go away? - */ - if (last_pfn <= curr_pfn) - continue; +#endif - size = (last_pfn - curr_pfn) << PAGE_SHIFT; - *pages_avail += last_pfn - curr_pfn; - - free_bootmem(sp_banks[i].base_addr, size); - } + *pages_avail = (memblock_phys_mem_size() >> PAGE_SHIFT) - high_pages; #ifdef CONFIG_BLK_DEV_INITRD if (initrd_start) { /* Reserve the initrd image area. */ size = initrd_end - initrd_start; - reserve_bootmem(initrd_start, size, BOOTMEM_DEFAULT); + memblock_reserve(initrd_start, size); *pages_avail -= PAGE_ALIGN(size) >> PAGE_SHIFT; initrd_start = (initrd_start - phys_base) + PAGE_OFFSET; - initrd_end = (initrd_end - phys_base) + PAGE_OFFSET; + initrd_end = (initrd_end - phys_base) + PAGE_OFFSET; } #endif /* Reserve the kernel text/data/bss. */ size = (start_pfn << PAGE_SHIFT) - phys_base; - reserve_bootmem(phys_base, size, BOOTMEM_DEFAULT); - *pages_avail -= PAGE_ALIGN(size) >> PAGE_SHIFT; - - /* Reserve the bootmem map. We do not account for it - * in pages_avail because we will release that memory - * in free_all_bootmem. - */ - size = bootmap_size; - reserve_bootmem((bootmap_pfn << PAGE_SHIFT), size, BOOTMEM_DEFAULT); + memblock_reserve(phys_base, size); *pages_avail -= PAGE_ALIGN(size) >> PAGE_SHIFT; return max_pfn; -- 2.7.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 1/2] sparc32: switch to NO_BOOTMEM @ 2018-08-02 11:53 ` Mike Rapoport 0 siblings, 0 replies; 10+ messages in thread From: Mike Rapoport @ 2018-08-02 11:53 UTC (permalink / raw) To: David S. Miller Cc: Sam Ravnborg, Michal Hocko, sparclinux, linux-mm, linux-kernel, Mike Rapoport Each populated sparc_phys_bank is added to memblock.memory. The reserve_bootmem() calls are replaced with memblock_reserve(), and the bootmem bitmap initialization is droppped. Signed-off-by: Mike Rapoport <rppt@linux.vnet.ibm.com> --- arch/sparc/Kconfig | 4 +-- arch/sparc/mm/init_32.c | 74 ++++++++++++++----------------------------------- 2 files changed, 23 insertions(+), 55 deletions(-) diff --git a/arch/sparc/Kconfig b/arch/sparc/Kconfig index 0f535de..0a874c8 100644 --- a/arch/sparc/Kconfig +++ b/arch/sparc/Kconfig @@ -45,6 +45,8 @@ config SPARC select LOCKDEP_SMALL if LOCKDEP select NEED_DMA_MAP_STATE select NEED_SG_DMA_LENGTH + select HAVE_MEMBLOCK + select NO_BOOTMEM config SPARC32 def_bool !64BIT @@ -60,7 +62,6 @@ config SPARC64 select HAVE_KRETPROBES select HAVE_KPROBES select HAVE_RCU_TABLE_FREE if SMP - select HAVE_MEMBLOCK select HAVE_MEMBLOCK_NODE_MAP select HAVE_ARCH_TRANSPARENT_HUGEPAGE select HAVE_DYNAMIC_FTRACE @@ -79,7 +80,6 @@ config SPARC64 select IRQ_PREFLOW_FASTEOI select ARCH_HAVE_NMI_SAFE_CMPXCHG select HAVE_C_RECORDMCOUNT - select NO_BOOTMEM select HAVE_ARCH_AUDITSYSCALL select ARCH_SUPPORTS_ATOMIC_RMW select HAVE_NMI diff --git a/arch/sparc/mm/init_32.c b/arch/sparc/mm/init_32.c index 95fe4f0..5117a5e 100644 --- a/arch/sparc/mm/init_32.c +++ b/arch/sparc/mm/init_32.c @@ -23,6 +23,7 @@ #include <linux/init.h> #include <linux/highmem.h> #include <linux/bootmem.h> +#include <linux/memblock.h> #include <linux/pagemap.h> #include <linux/poison.h> #include <linux/gfp.h> @@ -103,11 +104,15 @@ static unsigned long calc_max_low_pfn(void) unsigned long __init bootmem_init(unsigned long *pages_avail) { - unsigned long bootmap_size, start_pfn; + unsigned long start_pfn; unsigned long end_of_phys_memory = 0UL; - unsigned long bootmap_pfn, bytes_avail, size; + unsigned long bytes_avail, size; + unsigned long high_pages = 0UL; int i; + memblock_set_bottom_up(true); + memblock_allow_resize(); + bytes_avail = 0UL; for (i = 0; sp_banks[i].num_bytes != 0; i++) { end_of_phys_memory = sp_banks[i].base_addr + @@ -124,12 +129,15 @@ unsigned long __init bootmem_init(unsigned long *pages_avail) if (sp_banks[i].num_bytes = 0) { sp_banks[i].base_addr = 0xdeadbeef; } else { + memblock_add(sp_banks[i].base_addr, + sp_banks[i].num_bytes); sp_banks[i+1].num_bytes = 0; sp_banks[i+1].base_addr = 0xdeadbeef; } break; } } + memblock_add(sp_banks[i].base_addr, sp_banks[i].num_bytes); } /* Start with page aligned address of last symbol in kernel @@ -140,8 +148,6 @@ unsigned long __init bootmem_init(unsigned long *pages_avail) /* Now shift down to get the real physical page frame number. */ start_pfn >>= PAGE_SHIFT; - bootmap_pfn = start_pfn; - max_pfn = end_of_phys_memory >> PAGE_SHIFT; max_low_pfn = max_pfn; @@ -150,12 +156,16 @@ unsigned long __init bootmem_init(unsigned long *pages_avail) if (max_low_pfn > pfn_base + (SRMMU_MAXMEM >> PAGE_SHIFT)) { highstart_pfn = pfn_base + (SRMMU_MAXMEM >> PAGE_SHIFT); max_low_pfn = calc_max_low_pfn(); + high_pages = calc_highpages(); printk(KERN_NOTICE "%ldMB HIGHMEM available.\n", - calc_highpages() >> (20 - PAGE_SHIFT)); + high_pages >> (20 - PAGE_SHIFT)); } #ifdef CONFIG_BLK_DEV_INITRD - /* Now have to check initial ramdisk, so that bootmap does not overwrite it */ + /* + * Now have to check initial ramdisk, so that it won't pass + * the end of memory + */ if (sparc_ramdisk_image) { if (sparc_ramdisk_image >= (unsigned long)&_end - 2 * PAGE_SIZE) sparc_ramdisk_image -= KERNBASE; @@ -167,67 +177,25 @@ unsigned long __init bootmem_init(unsigned long *pages_avail) initrd_end, end_of_phys_memory); initrd_start = 0; } - if (initrd_start) { - if (initrd_start >= (start_pfn << PAGE_SHIFT) && - initrd_start < (start_pfn << PAGE_SHIFT) + 2 * PAGE_SIZE) - bootmap_pfn = PAGE_ALIGN (initrd_end) >> PAGE_SHIFT; - } } -#endif - /* Initialize the boot-time allocator. */ - bootmap_size = init_bootmem_node(NODE_DATA(0), bootmap_pfn, pfn_base, - max_low_pfn); - - /* Now register the available physical memory with the - * allocator. - */ - *pages_avail = 0; - for (i = 0; sp_banks[i].num_bytes != 0; i++) { - unsigned long curr_pfn, last_pfn; - - curr_pfn = sp_banks[i].base_addr >> PAGE_SHIFT; - if (curr_pfn >= max_low_pfn) - break; - - last_pfn = (sp_banks[i].base_addr + sp_banks[i].num_bytes) >> PAGE_SHIFT; - if (last_pfn > max_low_pfn) - last_pfn = max_low_pfn; - - /* - * .. finally, did all the rounding and playing - * around just make the area go away? - */ - if (last_pfn <= curr_pfn) - continue; +#endif - size = (last_pfn - curr_pfn) << PAGE_SHIFT; - *pages_avail += last_pfn - curr_pfn; - - free_bootmem(sp_banks[i].base_addr, size); - } + *pages_avail = (memblock_phys_mem_size() >> PAGE_SHIFT) - high_pages; #ifdef CONFIG_BLK_DEV_INITRD if (initrd_start) { /* Reserve the initrd image area. */ size = initrd_end - initrd_start; - reserve_bootmem(initrd_start, size, BOOTMEM_DEFAULT); + memblock_reserve(initrd_start, size); *pages_avail -= PAGE_ALIGN(size) >> PAGE_SHIFT; initrd_start = (initrd_start - phys_base) + PAGE_OFFSET; - initrd_end = (initrd_end - phys_base) + PAGE_OFFSET; + initrd_end = (initrd_end - phys_base) + PAGE_OFFSET; } #endif /* Reserve the kernel text/data/bss. */ size = (start_pfn << PAGE_SHIFT) - phys_base; - reserve_bootmem(phys_base, size, BOOTMEM_DEFAULT); - *pages_avail -= PAGE_ALIGN(size) >> PAGE_SHIFT; - - /* Reserve the bootmem map. We do not account for it - * in pages_avail because we will release that memory - * in free_all_bootmem. - */ - size = bootmap_size; - reserve_bootmem((bootmap_pfn << PAGE_SHIFT), size, BOOTMEM_DEFAULT); + memblock_reserve(phys_base, size); *pages_avail -= PAGE_ALIGN(size) >> PAGE_SHIFT; return max_pfn; -- 2.7.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] sparc32: switch to NO_BOOTMEM 2018-08-02 11:53 ` Mike Rapoport @ 2018-08-03 20:07 ` Sam Ravnborg -1 siblings, 0 replies; 10+ messages in thread From: Sam Ravnborg @ 2018-08-03 20:07 UTC (permalink / raw) To: Mike Rapoport Cc: David S. Miller, Michal Hocko, sparclinux, linux-mm, linux-kernel Hi Mike. A nice simplification of the arch code - with a potential to do much more. Some review comments, see the following. Sam On Thu, Aug 02, 2018 at 02:53:52PM +0300, Mike Rapoport wrote: > Each populated sparc_phys_bank is added to memblock.memory. The > reserve_bootmem() calls are replaced with memblock_reserve(), and the > bootmem bitmap initialization is droppped. > > Signed-off-by: Mike Rapoport <rppt@linux.vnet.ibm.com> > --- > @@ -103,11 +104,15 @@ static unsigned long calc_max_low_pfn(void) > > unsigned long __init bootmem_init(unsigned long *pages_avail) > { > - unsigned long bootmap_size, start_pfn; > + unsigned long start_pfn; > unsigned long end_of_phys_memory = 0UL; > - unsigned long bootmap_pfn, bytes_avail, size; > + unsigned long bytes_avail, size; > + unsigned long high_pages = 0UL; > int i; Variable definitions here, but assignments after definitions. And sort the variable definitions as inverse christmas tree. (Longest line first, sorter alpabetically if same length) No reason to use "0UL" for simple zero assignments. > > + memblock_set_bottom_up(true); > + memblock_allow_resize(); > + > bytes_avail = 0UL; > for (i = 0; sp_banks[i].num_bytes != 0; i++) { > end_of_phys_memory = sp_banks[i].base_addr + > @@ -124,12 +129,15 @@ unsigned long __init bootmem_init(unsigned long *pages_avail) > if (sp_banks[i].num_bytes = 0) { > sp_banks[i].base_addr = 0xdeadbeef; > } else { > + memblock_add(sp_banks[i].base_addr, > + sp_banks[i].num_bytes); > sp_banks[i+1].num_bytes = 0; > sp_banks[i+1].base_addr = 0xdeadbeef; > } > break; > } > } > + memblock_add(sp_banks[i].base_addr, sp_banks[i].num_bytes); This parts looks OK. You add the memory blocks to memblock that are relevant, and ignore the block above a possible limit from the command line (cmdline_memory_size). > } > > /* Start with page aligned address of last symbol in kernel > @@ -140,8 +148,6 @@ unsigned long __init bootmem_init(unsigned long *pages_avail) > /* Now shift down to get the real physical page frame number. */ > start_pfn >>= PAGE_SHIFT; > > - bootmap_pfn = start_pfn; > - OK, this is a bootmem artifact. > max_pfn = end_of_phys_memory >> PAGE_SHIFT; > > max_low_pfn = max_pfn; > @@ -150,12 +156,16 @@ unsigned long __init bootmem_init(unsigned long *pages_avail) > if (max_low_pfn > pfn_base + (SRMMU_MAXMEM >> PAGE_SHIFT)) { > highstart_pfn = pfn_base + (SRMMU_MAXMEM >> PAGE_SHIFT); > max_low_pfn = calc_max_low_pfn(); > + high_pages = calc_highpages(); > printk(KERN_NOTICE "%ldMB HIGHMEM available.\n", > - calc_highpages() >> (20 - PAGE_SHIFT)); > + high_pages >> (20 - PAGE_SHIFT)); This change looked un-related, but you need high_pages later. OK > } > > #ifdef CONFIG_BLK_DEV_INITRD > - /* Now have to check initial ramdisk, so that bootmap does not overwrite it */ > + /* > + * Now have to check initial ramdisk, so that it won't pass > + * the end of memory > + */ This is a reformatting of a comment but you remove the "bootmap" reference. Please reformat comment to saprc style. No empty "/*" so it should look like this: /* Now have to check initial ramdisk, so that it won't pass * the end of memory */ > if (sparc_ramdisk_image) { > if (sparc_ramdisk_image >= (unsigned long)&_end - 2 * PAGE_SIZE) > sparc_ramdisk_image -= KERNBASE; > @@ -167,67 +177,25 @@ unsigned long __init bootmem_init(unsigned long *pages_avail) > initrd_end, end_of_phys_memory); > initrd_start = 0; > } > - if (initrd_start) { > - if (initrd_start >= (start_pfn << PAGE_SHIFT) && > - initrd_start < (start_pfn << PAGE_SHIFT) + 2 * PAGE_SIZE) > - bootmap_pfn = PAGE_ALIGN (initrd_end) >> PAGE_SHIFT; > - } > } > -#endif > - /* Initialize the boot-time allocator. */ > - bootmap_size = init_bootmem_node(NODE_DATA(0), bootmap_pfn, pfn_base, > - max_low_pfn); > - > - /* Now register the available physical memory with the > - * allocator. > - */ > - *pages_avail = 0; > - for (i = 0; sp_banks[i].num_bytes != 0; i++) { > - unsigned long curr_pfn, last_pfn; > - > - curr_pfn = sp_banks[i].base_addr >> PAGE_SHIFT; > - if (curr_pfn >= max_low_pfn) > - break; > - > - last_pfn = (sp_banks[i].base_addr + sp_banks[i].num_bytes) >> PAGE_SHIFT; > - if (last_pfn > max_low_pfn) > - last_pfn = max_low_pfn; > - > - /* > - * .. finally, did all the rounding and playing > - * around just make the area go away? > - */ > - if (last_pfn <= curr_pfn) > - continue; > +#endif > > - size = (last_pfn - curr_pfn) << PAGE_SHIFT; > - *pages_avail += last_pfn - curr_pfn; > - > - free_bootmem(sp_banks[i].base_addr, size); > - } Good to see all this code gone. > + *pages_avail = (memblock_phys_mem_size() >> PAGE_SHIFT) - high_pages; Can we do this simpler? memblock knows the amount of memory not reserved. So we can ask memblock for the amount of non-reserved memory at the end of this function and then we have the result. Then we do not have to maintain pages_avail in the following code. > > #ifdef CONFIG_BLK_DEV_INITRD > if (initrd_start) { > /* Reserve the initrd image area. */ > size = initrd_end - initrd_start; > - reserve_bootmem(initrd_start, size, BOOTMEM_DEFAULT); > + memblock_reserve(initrd_start, size); > *pages_avail -= PAGE_ALIGN(size) >> PAGE_SHIFT; > > initrd_start = (initrd_start - phys_base) + PAGE_OFFSET; > - initrd_end = (initrd_end - phys_base) + PAGE_OFFSET; > + initrd_end = (initrd_end - phys_base) + PAGE_OFFSET; This is a pure white space change, but leave it in. > } > #endif > /* Reserve the kernel text/data/bss. */ > size = (start_pfn << PAGE_SHIFT) - phys_base; > - reserve_bootmem(phys_base, size, BOOTMEM_DEFAULT); > - *pages_avail -= PAGE_ALIGN(size) >> PAGE_SHIFT; > - > - /* Reserve the bootmem map. We do not account for it > - * in pages_avail because we will release that memory > - * in free_all_bootmem. > - */ > - size = bootmap_size; > - reserve_bootmem((bootmap_pfn << PAGE_SHIFT), size, BOOTMEM_DEFAULT); > + memblock_reserve(phys_base, size); > *pages_avail -= PAGE_ALIGN(size) >> PAGE_SHIFT; > > return max_pfn; > -- > 2.7.4 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] sparc32: switch to NO_BOOTMEM @ 2018-08-03 20:07 ` Sam Ravnborg 0 siblings, 0 replies; 10+ messages in thread From: Sam Ravnborg @ 2018-08-03 20:07 UTC (permalink / raw) To: Mike Rapoport Cc: David S. Miller, Michal Hocko, sparclinux, linux-mm, linux-kernel Hi Mike. A nice simplification of the arch code - with a potential to do much more. Some review comments, see the following. Sam On Thu, Aug 02, 2018 at 02:53:52PM +0300, Mike Rapoport wrote: > Each populated sparc_phys_bank is added to memblock.memory. The > reserve_bootmem() calls are replaced with memblock_reserve(), and the > bootmem bitmap initialization is droppped. > > Signed-off-by: Mike Rapoport <rppt@linux.vnet.ibm.com> > --- > @@ -103,11 +104,15 @@ static unsigned long calc_max_low_pfn(void) > > unsigned long __init bootmem_init(unsigned long *pages_avail) > { > - unsigned long bootmap_size, start_pfn; > + unsigned long start_pfn; > unsigned long end_of_phys_memory = 0UL; > - unsigned long bootmap_pfn, bytes_avail, size; > + unsigned long bytes_avail, size; > + unsigned long high_pages = 0UL; > int i; Variable definitions here, but assignments after definitions. And sort the variable definitions as inverse christmas tree. (Longest line first, sorter alpabetically if same length) No reason to use "0UL" for simple zero assignments. > > + memblock_set_bottom_up(true); > + memblock_allow_resize(); > + > bytes_avail = 0UL; > for (i = 0; sp_banks[i].num_bytes != 0; i++) { > end_of_phys_memory = sp_banks[i].base_addr + > @@ -124,12 +129,15 @@ unsigned long __init bootmem_init(unsigned long *pages_avail) > if (sp_banks[i].num_bytes == 0) { > sp_banks[i].base_addr = 0xdeadbeef; > } else { > + memblock_add(sp_banks[i].base_addr, > + sp_banks[i].num_bytes); > sp_banks[i+1].num_bytes = 0; > sp_banks[i+1].base_addr = 0xdeadbeef; > } > break; > } > } > + memblock_add(sp_banks[i].base_addr, sp_banks[i].num_bytes); This parts looks OK. You add the memory blocks to memblock that are relevant, and ignore the block above a possible limit from the command line (cmdline_memory_size). > } > > /* Start with page aligned address of last symbol in kernel > @@ -140,8 +148,6 @@ unsigned long __init bootmem_init(unsigned long *pages_avail) > /* Now shift down to get the real physical page frame number. */ > start_pfn >>= PAGE_SHIFT; > > - bootmap_pfn = start_pfn; > - OK, this is a bootmem artifact. > max_pfn = end_of_phys_memory >> PAGE_SHIFT; > > max_low_pfn = max_pfn; > @@ -150,12 +156,16 @@ unsigned long __init bootmem_init(unsigned long *pages_avail) > if (max_low_pfn > pfn_base + (SRMMU_MAXMEM >> PAGE_SHIFT)) { > highstart_pfn = pfn_base + (SRMMU_MAXMEM >> PAGE_SHIFT); > max_low_pfn = calc_max_low_pfn(); > + high_pages = calc_highpages(); > printk(KERN_NOTICE "%ldMB HIGHMEM available.\n", > - calc_highpages() >> (20 - PAGE_SHIFT)); > + high_pages >> (20 - PAGE_SHIFT)); This change looked un-related, but you need high_pages later. OK > } > > #ifdef CONFIG_BLK_DEV_INITRD > - /* Now have to check initial ramdisk, so that bootmap does not overwrite it */ > + /* > + * Now have to check initial ramdisk, so that it won't pass > + * the end of memory > + */ This is a reformatting of a comment but you remove the "bootmap" reference. Please reformat comment to saprc style. No empty "/*" so it should look like this: /* Now have to check initial ramdisk, so that it won't pass * the end of memory */ > if (sparc_ramdisk_image) { > if (sparc_ramdisk_image >= (unsigned long)&_end - 2 * PAGE_SIZE) > sparc_ramdisk_image -= KERNBASE; > @@ -167,67 +177,25 @@ unsigned long __init bootmem_init(unsigned long *pages_avail) > initrd_end, end_of_phys_memory); > initrd_start = 0; > } > - if (initrd_start) { > - if (initrd_start >= (start_pfn << PAGE_SHIFT) && > - initrd_start < (start_pfn << PAGE_SHIFT) + 2 * PAGE_SIZE) > - bootmap_pfn = PAGE_ALIGN (initrd_end) >> PAGE_SHIFT; > - } > } > -#endif > - /* Initialize the boot-time allocator. */ > - bootmap_size = init_bootmem_node(NODE_DATA(0), bootmap_pfn, pfn_base, > - max_low_pfn); > - > - /* Now register the available physical memory with the > - * allocator. > - */ > - *pages_avail = 0; > - for (i = 0; sp_banks[i].num_bytes != 0; i++) { > - unsigned long curr_pfn, last_pfn; > - > - curr_pfn = sp_banks[i].base_addr >> PAGE_SHIFT; > - if (curr_pfn >= max_low_pfn) > - break; > - > - last_pfn = (sp_banks[i].base_addr + sp_banks[i].num_bytes) >> PAGE_SHIFT; > - if (last_pfn > max_low_pfn) > - last_pfn = max_low_pfn; > - > - /* > - * .. finally, did all the rounding and playing > - * around just make the area go away? > - */ > - if (last_pfn <= curr_pfn) > - continue; > +#endif > > - size = (last_pfn - curr_pfn) << PAGE_SHIFT; > - *pages_avail += last_pfn - curr_pfn; > - > - free_bootmem(sp_banks[i].base_addr, size); > - } Good to see all this code gone. > + *pages_avail = (memblock_phys_mem_size() >> PAGE_SHIFT) - high_pages; Can we do this simpler? memblock knows the amount of memory not reserved. So we can ask memblock for the amount of non-reserved memory at the end of this function and then we have the result. Then we do not have to maintain pages_avail in the following code. > > #ifdef CONFIG_BLK_DEV_INITRD > if (initrd_start) { > /* Reserve the initrd image area. */ > size = initrd_end - initrd_start; > - reserve_bootmem(initrd_start, size, BOOTMEM_DEFAULT); > + memblock_reserve(initrd_start, size); > *pages_avail -= PAGE_ALIGN(size) >> PAGE_SHIFT; > > initrd_start = (initrd_start - phys_base) + PAGE_OFFSET; > - initrd_end = (initrd_end - phys_base) + PAGE_OFFSET; > + initrd_end = (initrd_end - phys_base) + PAGE_OFFSET; This is a pure white space change, but leave it in. > } > #endif > /* Reserve the kernel text/data/bss. */ > size = (start_pfn << PAGE_SHIFT) - phys_base; > - reserve_bootmem(phys_base, size, BOOTMEM_DEFAULT); > - *pages_avail -= PAGE_ALIGN(size) >> PAGE_SHIFT; > - > - /* Reserve the bootmem map. We do not account for it > - * in pages_avail because we will release that memory > - * in free_all_bootmem. > - */ > - size = bootmap_size; > - reserve_bootmem((bootmap_pfn << PAGE_SHIFT), size, BOOTMEM_DEFAULT); > + memblock_reserve(phys_base, size); > *pages_avail -= PAGE_ALIGN(size) >> PAGE_SHIFT; > > return max_pfn; > -- > 2.7.4 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/2] sparc32: tidy up ramdisk memory reservation 2018-08-02 11:53 ` Mike Rapoport @ 2018-08-02 11:53 ` Mike Rapoport -1 siblings, 0 replies; 10+ messages in thread From: Mike Rapoport @ 2018-08-02 11:53 UTC (permalink / raw) To: David S. Miller Cc: Sam Ravnborg, Michal Hocko, sparclinux, linux-mm, linux-kernel, Mike Rapoport The detection and reservation of ramdisk memory were separated to allow bootmem bitmap initialization after the ramdisk boundaries are detected. Since the bootmem initialization is removed, the reservation of ramdisk memory can be done immediately after its boundaries are found. Signed-off-by: Mike Rapoport <rppt@linux.vnet.ibm.com> --- arch/sparc/mm/init_32.c | 24 ++++++++++-------------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/arch/sparc/mm/init_32.c b/arch/sparc/mm/init_32.c index 5117a5e..b5d8f90 100644 --- a/arch/sparc/mm/init_32.c +++ b/arch/sparc/mm/init_32.c @@ -161,6 +161,8 @@ unsigned long __init bootmem_init(unsigned long *pages_avail) high_pages >> (20 - PAGE_SHIFT)); } + *pages_avail = (memblock_phys_mem_size() >> PAGE_SHIFT) - high_pages; + #ifdef CONFIG_BLK_DEV_INITRD /* * Now have to check initial ramdisk, so that it won't pass @@ -176,23 +178,17 @@ unsigned long __init bootmem_init(unsigned long *pages_avail) "(0x%016lx > 0x%016lx)\ndisabling initrd\n", initrd_end, end_of_phys_memory); initrd_start = 0; + } else { + /* Reserve the initrd image area. */ + size = initrd_end - initrd_start; + memblock_reserve(initrd_start, size); + *pages_avail -= PAGE_ALIGN(size) >> PAGE_SHIFT; + + initrd_start = (initrd_start - phys_base) + PAGE_OFFSET; + initrd_end = (initrd_end - phys_base) + PAGE_OFFSET; } } #endif - - *pages_avail = (memblock_phys_mem_size() >> PAGE_SHIFT) - high_pages; - -#ifdef CONFIG_BLK_DEV_INITRD - if (initrd_start) { - /* Reserve the initrd image area. */ - size = initrd_end - initrd_start; - memblock_reserve(initrd_start, size); - *pages_avail -= PAGE_ALIGN(size) >> PAGE_SHIFT; - - initrd_start = (initrd_start - phys_base) + PAGE_OFFSET; - initrd_end = (initrd_end - phys_base) + PAGE_OFFSET; - } -#endif /* Reserve the kernel text/data/bss. */ size = (start_pfn << PAGE_SHIFT) - phys_base; memblock_reserve(phys_base, size); -- 2.7.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/2] sparc32: tidy up ramdisk memory reservation @ 2018-08-02 11:53 ` Mike Rapoport 0 siblings, 0 replies; 10+ messages in thread From: Mike Rapoport @ 2018-08-02 11:53 UTC (permalink / raw) To: David S. Miller Cc: Sam Ravnborg, Michal Hocko, sparclinux, linux-mm, linux-kernel, Mike Rapoport The detection and reservation of ramdisk memory were separated to allow bootmem bitmap initialization after the ramdisk boundaries are detected. Since the bootmem initialization is removed, the reservation of ramdisk memory can be done immediately after its boundaries are found. Signed-off-by: Mike Rapoport <rppt@linux.vnet.ibm.com> --- arch/sparc/mm/init_32.c | 24 ++++++++++-------------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/arch/sparc/mm/init_32.c b/arch/sparc/mm/init_32.c index 5117a5e..b5d8f90 100644 --- a/arch/sparc/mm/init_32.c +++ b/arch/sparc/mm/init_32.c @@ -161,6 +161,8 @@ unsigned long __init bootmem_init(unsigned long *pages_avail) high_pages >> (20 - PAGE_SHIFT)); } + *pages_avail = (memblock_phys_mem_size() >> PAGE_SHIFT) - high_pages; + #ifdef CONFIG_BLK_DEV_INITRD /* * Now have to check initial ramdisk, so that it won't pass @@ -176,23 +178,17 @@ unsigned long __init bootmem_init(unsigned long *pages_avail) "(0x%016lx > 0x%016lx)\ndisabling initrd\n", initrd_end, end_of_phys_memory); initrd_start = 0; + } else { + /* Reserve the initrd image area. */ + size = initrd_end - initrd_start; + memblock_reserve(initrd_start, size); + *pages_avail -= PAGE_ALIGN(size) >> PAGE_SHIFT; + + initrd_start = (initrd_start - phys_base) + PAGE_OFFSET; + initrd_end = (initrd_end - phys_base) + PAGE_OFFSET; } } #endif - - *pages_avail = (memblock_phys_mem_size() >> PAGE_SHIFT) - high_pages; - -#ifdef CONFIG_BLK_DEV_INITRD - if (initrd_start) { - /* Reserve the initrd image area. */ - size = initrd_end - initrd_start; - memblock_reserve(initrd_start, size); - *pages_avail -= PAGE_ALIGN(size) >> PAGE_SHIFT; - - initrd_start = (initrd_start - phys_base) + PAGE_OFFSET; - initrd_end = (initrd_end - phys_base) + PAGE_OFFSET; - } -#endif /* Reserve the kernel text/data/bss. */ size = (start_pfn << PAGE_SHIFT) - phys_base; memblock_reserve(phys_base, size); -- 2.7.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] sparc32: tidy up ramdisk memory reservation 2018-08-02 11:53 ` Mike Rapoport @ 2018-08-03 20:18 ` Sam Ravnborg -1 siblings, 0 replies; 10+ messages in thread From: Sam Ravnborg @ 2018-08-03 20:18 UTC (permalink / raw) To: Mike Rapoport Cc: David S. Miller, Michal Hocko, sparclinux, linux-mm, linux-kernel Hi Mike. On Thu, Aug 02, 2018 at 02:53:53PM +0300, Mike Rapoport wrote: > The detection and reservation of ramdisk memory were separated to allow > bootmem bitmap initialization after the ramdisk boundaries are detected. > Since the bootmem initialization is removed, the reservation of ramdisk > memory can be done immediately after its boundaries are found. When touching this area could you look at introducing a find_ramdisk() function like we do for sparc64? It is always nice when the codebases look alike. Then you could combine your simplification with some refactoring that further increases readability. See: https://patchwork.ozlabs.org/patch/151194/ for my attempt from long time ago. Sam ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] sparc32: tidy up ramdisk memory reservation @ 2018-08-03 20:18 ` Sam Ravnborg 0 siblings, 0 replies; 10+ messages in thread From: Sam Ravnborg @ 2018-08-03 20:18 UTC (permalink / raw) To: Mike Rapoport Cc: David S. Miller, Michal Hocko, sparclinux, linux-mm, linux-kernel Hi Mike. On Thu, Aug 02, 2018 at 02:53:53PM +0300, Mike Rapoport wrote: > The detection and reservation of ramdisk memory were separated to allow > bootmem bitmap initialization after the ramdisk boundaries are detected. > Since the bootmem initialization is removed, the reservation of ramdisk > memory can be done immediately after its boundaries are found. When touching this area could you look at introducing a find_ramdisk() function like we do for sparc64? It is always nice when the codebases look alike. Then you could combine your simplification with some refactoring that further increases readability. See: https://patchwork.ozlabs.org/patch/151194/ for my attempt from long time ago. Sam ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2018-08-03 20:18 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-08-02 11:53 [PATCH 0/2] sparc32: switch to NO_BOOTMEM Mike Rapoport 2018-08-02 11:53 ` Mike Rapoport 2018-08-02 11:53 ` [PATCH 1/2] " Mike Rapoport 2018-08-02 11:53 ` Mike Rapoport 2018-08-03 20:07 ` Sam Ravnborg 2018-08-03 20:07 ` Sam Ravnborg 2018-08-02 11:53 ` [PATCH 2/2] sparc32: tidy up ramdisk memory reservation Mike Rapoport 2018-08-02 11:53 ` Mike Rapoport 2018-08-03 20:18 ` Sam Ravnborg 2018-08-03 20:18 ` Sam Ravnborg
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.