2018-08-27 08:01:15

by Mike Rapoport

[permalink] [raw]
Subject: [PATCH RESEND] mips: switch to NO_BOOTMEM

MIPS already has memblock support and all the memory is already registered
with it.

This patch replaces bootmem memory reservations with memblock ones and
removes the bootmem initialization.

Signed-off-by: Mike Rapoport <[email protected]>
---

arch/mips/Kconfig | 1 +
arch/mips/kernel/setup.c | 89 +++++-----------------------------
arch/mips/loongson64/loongson-3/numa.c | 34 ++++++-------
arch/mips/sgi-ip27/ip27-memory.c | 11 ++---
4 files changed, 33 insertions(+), 102 deletions(-)

diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index 3551199..b8c5fea 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -78,6 +78,7 @@ config MIPS
select RTC_LIB if !MACH_LOONGSON64
select SYSCTL_EXCEPTION_TRACE
select VIRT_TO_BUS
+ select NO_BOOTMEM

menu "Machine selection"

diff --git a/arch/mips/kernel/setup.c b/arch/mips/kernel/setup.c
index c71d1eb..4114d3c 100644
--- a/arch/mips/kernel/setup.c
+++ b/arch/mips/kernel/setup.c
@@ -333,7 +333,7 @@ static void __init finalize_initrd(void)

maybe_bswap_initrd();

- reserve_bootmem(__pa(initrd_start), size, BOOTMEM_DEFAULT);
+ memblock_reserve(__pa(initrd_start), size);
initrd_below_start_ok = 1;

pr_info("Initial ramdisk at: 0x%lx (%lu bytes)\n",
@@ -370,20 +370,10 @@ static void __init bootmem_init(void)

#else /* !CONFIG_SGI_IP27 */

-static unsigned long __init bootmap_bytes(unsigned long pages)
-{
- unsigned long bytes = DIV_ROUND_UP(pages, 8);
-
- return ALIGN(bytes, sizeof(long));
-}
-
static void __init bootmem_init(void)
{
unsigned long reserved_end;
- unsigned long mapstart = ~0UL;
- unsigned long bootmap_size;
phys_addr_t ramstart = PHYS_ADDR_MAX;
- bool bootmap_valid = false;
int i;

/*
@@ -395,6 +385,8 @@ static void __init bootmem_init(void)
init_initrd();
reserved_end = (unsigned long) PFN_UP(__pa_symbol(&_end));

+ memblock_reserve(PHYS_OFFSET, reserved_end << PAGE_SHIFT);
+
/*
* max_low_pfn is not a number of pages. The number of pages
* of the system is given by 'max_low_pfn - min_low_pfn'.
@@ -442,9 +434,6 @@ static void __init bootmem_init(void)
if (initrd_end && end <= (unsigned long)PFN_UP(__pa(initrd_end)))
continue;
#endif
- if (start >= mapstart)
- continue;
- mapstart = max(reserved_end, start);
}

if (min_low_pfn >= max_low_pfn)
@@ -456,9 +445,11 @@ static void __init bootmem_init(void)
/*
* Reserve any memory between the start of RAM and PHYS_OFFSET
*/
- if (ramstart > PHYS_OFFSET)
+ if (ramstart > PHYS_OFFSET) {
add_memory_region(PHYS_OFFSET, ramstart - PHYS_OFFSET,
BOOT_MEM_RESERVED);
+ memblock_reserve(PHYS_OFFSET, ramstart - PHYS_OFFSET);
+ }

if (min_low_pfn > ARCH_PFN_OFFSET) {
pr_info("Wasting %lu bytes for tracking %lu unused pages\n",
@@ -483,52 +474,6 @@ static void __init bootmem_init(void)
max_low_pfn = PFN_DOWN(HIGHMEM_START);
}

-#ifdef CONFIG_BLK_DEV_INITRD
- /*
- * mapstart should be after initrd_end
- */
- if (initrd_end)
- mapstart = max(mapstart, (unsigned long)PFN_UP(__pa(initrd_end)));
-#endif
-
- /*
- * check that mapstart doesn't overlap with any of
- * memory regions that have been reserved through eg. DTB
- */
- bootmap_size = bootmap_bytes(max_low_pfn - min_low_pfn);
-
- bootmap_valid = memory_region_available(PFN_PHYS(mapstart),
- bootmap_size);
- for (i = 0; i < boot_mem_map.nr_map && !bootmap_valid; i++) {
- unsigned long mapstart_addr;
-
- switch (boot_mem_map.map[i].type) {
- case BOOT_MEM_RESERVED:
- mapstart_addr = PFN_ALIGN(boot_mem_map.map[i].addr +
- boot_mem_map.map[i].size);
- if (PHYS_PFN(mapstart_addr) < mapstart)
- break;
-
- bootmap_valid = memory_region_available(mapstart_addr,
- bootmap_size);
- if (bootmap_valid)
- mapstart = PHYS_PFN(mapstart_addr);
- break;
- default:
- break;
- }
- }
-
- if (!bootmap_valid)
- panic("No memory area to place a bootmap bitmap");
-
- /*
- * Initialize the boot-time allocator with low memory only.
- */
- if (bootmap_size != init_bootmem_node(NODE_DATA(0), mapstart,
- min_low_pfn, max_low_pfn))
- panic("Unexpected memory size required for bootmap");
-
for (i = 0; i < boot_mem_map.nr_map; i++) {
unsigned long start, end;

@@ -577,9 +522,9 @@ static void __init bootmem_init(void)
default:
/* Not usable memory */
if (start > min_low_pfn && end < max_low_pfn)
- reserve_bootmem(boot_mem_map.map[i].addr,
- boot_mem_map.map[i].size,
- BOOTMEM_DEFAULT);
+ memblock_reserve(boot_mem_map.map[i].addr,
+ boot_mem_map.map[i].size);
+
continue;
}

@@ -602,15 +547,9 @@ static void __init bootmem_init(void)
size = end - start;

/* Register lowmem ranges */
- free_bootmem(PFN_PHYS(start), size << PAGE_SHIFT);
memory_present(0, start, end);
}

- /*
- * Reserve the bootmap memory.
- */
- reserve_bootmem(PFN_PHYS(mapstart), bootmap_size, BOOTMEM_DEFAULT);
-
#ifdef CONFIG_RELOCATABLE
/*
* The kernel reserves all memory below its _end symbol as bootmem,
@@ -912,17 +851,15 @@ static void __init arch_mem_init(char **cmdline_p)
if (setup_elfcorehdr && setup_elfcorehdr_size) {
printk(KERN_INFO "kdump reserved memory at %lx-%lx\n",
setup_elfcorehdr, setup_elfcorehdr_size);
- reserve_bootmem(setup_elfcorehdr, setup_elfcorehdr_size,
- BOOTMEM_DEFAULT);
+ memblock_reserve(setup_elfcorehdr, setup_elfcorehdr_size);
}
#endif

mips_parse_crashkernel();
#ifdef CONFIG_KEXEC
if (crashk_res.start != crashk_res.end)
- reserve_bootmem(crashk_res.start,
- crashk_res.end - crashk_res.start + 1,
- BOOTMEM_DEFAULT);
+ memblock_reserve(crashk_res.start,
+ crashk_res.end - crashk_res.start + 1);
#endif
device_tree_init();
sparse_init();
@@ -932,7 +869,7 @@ static void __init arch_mem_init(char **cmdline_p)
/* Tell bootmem about cma reserved memblock section */
for_each_memblock(reserved, reg)
if (reg->size != 0)
- reserve_bootmem(reg->base, reg->size, BOOTMEM_DEFAULT);
+ memblock_reserve(reg->base, reg->size);

reserve_bootmem_region(__pa_symbol(&__nosave_begin),
__pa_symbol(&__nosave_end)); /* Reserve for hibernation */
diff --git a/arch/mips/loongson64/loongson-3/numa.c b/arch/mips/loongson64/loongson-3/numa.c
index 9717106..c1e6ec5 100644
--- a/arch/mips/loongson64/loongson-3/numa.c
+++ b/arch/mips/loongson64/loongson-3/numa.c
@@ -180,43 +180,39 @@ static void __init szmem(unsigned int node)

static void __init node_mem_init(unsigned int node)
{
- unsigned long bootmap_size;
unsigned long node_addrspace_offset;
- unsigned long start_pfn, end_pfn, freepfn;
+ unsigned long start_pfn, end_pfn;

node_addrspace_offset = nid_to_addroffset(node);
pr_info("Node%d's addrspace_offset is 0x%lx\n",
node, node_addrspace_offset);

get_pfn_range_for_nid(node, &start_pfn, &end_pfn);
- freepfn = start_pfn;
- if (node == 0)
- freepfn = PFN_UP(__pa_symbol(&_end)); /* kernel end address */
- pr_info("Node%d: start_pfn=0x%lx, end_pfn=0x%lx, freepfn=0x%lx\n",
- node, start_pfn, end_pfn, freepfn);
+ pr_info("Node%d: start_pfn=0x%lx, end_pfn=0x%lx\n",
+ node, start_pfn, end_pfn);

__node_data[node] = prealloc__node_data + node;

- NODE_DATA(node)->bdata = &bootmem_node_data[node];
NODE_DATA(node)->node_start_pfn = start_pfn;
NODE_DATA(node)->node_spanned_pages = end_pfn - start_pfn;

- bootmap_size = init_bootmem_node(NODE_DATA(node), freepfn,
- start_pfn, end_pfn);
free_bootmem_with_active_regions(node, end_pfn);
- if (node == 0) /* used by finalize_initrd() */
+
+ if (node == 0) {
+ /* kernel end address */
+ unsigned long kernel_end_pfn = PFN_UP(__pa_symbol(&_end));
+
+ /* used by finalize_initrd() */
max_low_pfn = end_pfn;

- /* This is reserved for the kernel and bdata->node_bootmem_map */
- reserve_bootmem_node(NODE_DATA(node), start_pfn << PAGE_SHIFT,
- ((freepfn - start_pfn) << PAGE_SHIFT) + bootmap_size,
- BOOTMEM_DEFAULT);
+ /* Reserve the kernel text/data/bss */
+ memblock_reserve(start_pfn << PAGE_SHIFT,
+ ((kernel_end_pfn - start_pfn) << PAGE_SHIFT));

- if (node == 0 && node_end_pfn(0) >= (0xffffffff >> PAGE_SHIFT)) {
/* Reserve 0xfe000000~0xffffffff for RS780E integrated GPU */
- reserve_bootmem_node(NODE_DATA(node),
- (node_addrspace_offset | 0xfe000000),
- 32 << 20, BOOTMEM_DEFAULT);
+ if (node_end_pfn(0) >= (0xffffffff >> PAGE_SHIFT))
+ memblock_reserve((node_addrspace_offset | 0xfe000000),
+ 32 << 20);
}

sparse_memory_present_with_active_regions(node);
diff --git a/arch/mips/sgi-ip27/ip27-memory.c b/arch/mips/sgi-ip27/ip27-memory.c
index 59133d0a..6f7bef0 100644
--- a/arch/mips/sgi-ip27/ip27-memory.c
+++ b/arch/mips/sgi-ip27/ip27-memory.c
@@ -389,7 +389,6 @@ static void __init node_mem_init(cnodeid_t node)
{
unsigned long slot_firstpfn = slot_getbasepfn(node, 0);
unsigned long slot_freepfn = node_getfirstfree(node);
- unsigned long bootmap_size;
unsigned long start_pfn, end_pfn;

get_pfn_range_for_nid(node, &start_pfn, &end_pfn);
@@ -400,7 +399,6 @@ static void __init node_mem_init(cnodeid_t node)
__node_data[node] = __va(slot_freepfn << PAGE_SHIFT);
memset(__node_data[node], 0, PAGE_SIZE);

- NODE_DATA(node)->bdata = &bootmem_node_data[node];
NODE_DATA(node)->node_start_pfn = start_pfn;
NODE_DATA(node)->node_spanned_pages = end_pfn - start_pfn;

@@ -409,12 +407,11 @@ static void __init node_mem_init(cnodeid_t node)
slot_freepfn += PFN_UP(sizeof(struct pglist_data) +
sizeof(struct hub_data));

- bootmap_size = init_bootmem_node(NODE_DATA(node), slot_freepfn,
- start_pfn, end_pfn);
free_bootmem_with_active_regions(node, end_pfn);
- reserve_bootmem_node(NODE_DATA(node), slot_firstpfn << PAGE_SHIFT,
- ((slot_freepfn - slot_firstpfn) << PAGE_SHIFT) + bootmap_size,
- BOOTMEM_DEFAULT);
+
+ memblock_reserve(slot_firstpfn << PAGE_SHIFT,
+ ((slot_freepfn - slot_firstpfn) << PAGE_SHIFT));
+
sparse_memory_present_with_active_regions(node);
}

--
2.7.4



2018-08-30 21:50:45

by Paul Burton

[permalink] [raw]
Subject: Re: [PATCH RESEND] mips: switch to NO_BOOTMEM

Hi Mike,

On Mon, Aug 27, 2018 at 10:59:35AM +0300, Mike Rapoport wrote:
> MIPS already has memblock support and all the memory is already registered
> with it.
>
> This patch replaces bootmem memory reservations with memblock ones and
> removes the bootmem initialization.
>
> Signed-off-by: Mike Rapoport <[email protected]>
> ---
>
> arch/mips/Kconfig | 1 +
> arch/mips/kernel/setup.c | 89 +++++-----------------------------
> arch/mips/loongson64/loongson-3/numa.c | 34 ++++++-------
> arch/mips/sgi-ip27/ip27-memory.c | 11 ++---
> 4 files changed, 33 insertions(+), 102 deletions(-)

Thanks for working on this. Unfortunately it breaks boot for at least a
32r6el_defconfig kernel on QEMU:

$ qemu-system-mips64el \
-M boston \
-kernel arch/mips/boot/vmlinux.gz.itb \
-serial stdio \
-append "earlycon=uart8250,mmio32,0x17ffe000,115200 console=ttyS0,115200 debug memblock=debug mminit_loglevel=4"
[ 0.000000] Linux version 4.19.0-rc1-00008-g82d0f342eecd (pburton@pburton-laptop) (gcc version 8.1.0 (GCC)) #23 SMP Thu Aug 30 14:38:06 PDT 2018
[ 0.000000] CPU0 revision is: 0001a900 (MIPS I6400)
[ 0.000000] FPU revision is: 20f30300
[ 0.000000] MSA revision is: 00000300
[ 0.000000] MIPS: machine is img,boston
[ 0.000000] Determined physical RAM map:
[ 0.000000] memory: 10000000 @ 00000000 (usable)
[ 0.000000] memory: 30000000 @ 90000000 (usable)
[ 0.000000] earlycon: uart8250 at MMIO32 0x17ffe000 (options '115200')
[ 0.000000] bootconsole [uart8250] enabled
[ 0.000000] memblock_reserve: [0x00000000-0x009a8fff] setup_arch+0x224/0x718
[ 0.000000] memblock_reserve: [0x01360000-0x01361ca7] setup_arch+0x3d8/0x718
[ 0.000000] Initrd not found or empty - disabling initrd
[ 0.000000] memblock_virt_alloc_try_nid: 7336 bytes align=0x40 nid=-1 from=0x00000000 max_addr=0x00000000 early_init_dt_alloc_memory_arch+0x20/0x2c
[ 0.000000] memblock_reserve: [0xbfffe340-0xbfffffe7] memblock_virt_alloc_internal+0x120/0x1ec
<hang>

It looks like we took a TLB store exception after calling memset() with
a bogus address from memblock_virt_alloc_try_nid() or something inlined
into it.

This was with your patch applied atop the mips-next branch from [1],
which is currently at commit 35d017947401 ("MIPS: ralink: Add rt3352
SPI_CS1 pinmux").

Thanks,
Paul

[1] git://git.kernel.org/pub/scm/linux/kernel/git/mips/linux.git

2018-08-31 21:19:18

by Mike Rapoport

[permalink] [raw]
Subject: Re: [PATCH RESEND] mips: switch to NO_BOOTMEM

Hi Paul,

On Thu, Aug 30, 2018 at 02:48:57PM -0700, Paul Burton wrote:
> Hi Mike,
>
> On Mon, Aug 27, 2018 at 10:59:35AM +0300, Mike Rapoport wrote:
> > MIPS already has memblock support and all the memory is already registered
> > with it.
> >
> > This patch replaces bootmem memory reservations with memblock ones and
> > removes the bootmem initialization.
> >
> > Signed-off-by: Mike Rapoport <[email protected]>
> > ---
> >
> > arch/mips/Kconfig | 1 +
> > arch/mips/kernel/setup.c | 89 +++++-----------------------------
> > arch/mips/loongson64/loongson-3/numa.c | 34 ++++++-------
> > arch/mips/sgi-ip27/ip27-memory.c | 11 ++---
> > 4 files changed, 33 insertions(+), 102 deletions(-)
>
> Thanks for working on this. Unfortunately it breaks boot for at least a
> 32r6el_defconfig kernel on QEMU:
>
> $ qemu-system-mips64el \
> -M boston \
> -kernel arch/mips/boot/vmlinux.gz.itb \
> -serial stdio \
> -append "earlycon=uart8250,mmio32,0x17ffe000,115200 console=ttyS0,115200 debug memblock=debug mminit_loglevel=4"
> [ 0.000000] Linux version 4.19.0-rc1-00008-g82d0f342eecd (pburton@pburton-laptop) (gcc version 8.1.0 (GCC)) #23 SMP Thu Aug 30 14:38:06 PDT 2018
> [ 0.000000] CPU0 revision is: 0001a900 (MIPS I6400)
> [ 0.000000] FPU revision is: 20f30300
> [ 0.000000] MSA revision is: 00000300
> [ 0.000000] MIPS: machine is img,boston
> [ 0.000000] Determined physical RAM map:
> [ 0.000000] memory: 10000000 @ 00000000 (usable)
> [ 0.000000] memory: 30000000 @ 90000000 (usable)
> [ 0.000000] earlycon: uart8250 at MMIO32 0x17ffe000 (options '115200')
> [ 0.000000] bootconsole [uart8250] enabled
> [ 0.000000] memblock_reserve: [0x00000000-0x009a8fff] setup_arch+0x224/0x718
> [ 0.000000] memblock_reserve: [0x01360000-0x01361ca7] setup_arch+0x3d8/0x718
> [ 0.000000] Initrd not found or empty - disabling initrd
> [ 0.000000] memblock_virt_alloc_try_nid: 7336 bytes align=0x40 nid=-1 from=0x00000000 max_addr=0x00000000 early_init_dt_alloc_memory_arch+0x20/0x2c
> [ 0.000000] memblock_reserve: [0xbfffe340-0xbfffffe7] memblock_virt_alloc_internal+0x120/0x1ec
> <hang>
>
> It looks like we took a TLB store exception after calling memset() with
> a bogus address from memblock_virt_alloc_try_nid() or something inlined
> into it.

Memblock tries to allocate from the top and the resulting address ends up
in the high memory.

With the hunk below I was able to get to "VFS: Cannot open root device"

diff --git a/arch/mips/kernel/setup.c b/arch/mips/kernel/setup.c
index 4114d3c..4a9b0f7 100644
--- a/arch/mips/kernel/setup.c
+++ b/arch/mips/kernel/setup.c
@@ -577,6 +577,8 @@ static void __init bootmem_init(void)
* Reserve initrd memory if needed.
*/
finalize_initrd();
+
+ memblock_set_bottom_up(true);
}

#endif /* CONFIG_SGI_IP27 */

> This was with your patch applied atop the mips-next branch from [1],
> which is currently at commit 35d017947401 ("MIPS: ralink: Add rt3352
> SPI_CS1 pinmux").
>
> Thanks,
> Paul
>
> [1] git://git.kernel.org/pub/scm/linux/kernel/git/mips/linux.git
>

--
Sincerely yours,
Mike.


2018-09-05 17:48:48

by Paul Burton

[permalink] [raw]
Subject: Re: [PATCH RESEND] mips: switch to NO_BOOTMEM

Hi Mike,

On Sat, Sep 01, 2018 at 12:17:48AM +0300, Mike Rapoport wrote:
> On Thu, Aug 30, 2018 at 02:48:57PM -0700, Paul Burton wrote:
> > On Mon, Aug 27, 2018 at 10:59:35AM +0300, Mike Rapoport wrote:
> > > MIPS already has memblock support and all the memory is already registered
> > > with it.
> > >
> > > This patch replaces bootmem memory reservations with memblock ones and
> > > removes the bootmem initialization.
> > >
> > > Signed-off-by: Mike Rapoport <[email protected]>
> > > ---
> > >
> > > arch/mips/Kconfig | 1 +
> > > arch/mips/kernel/setup.c | 89 +++++-----------------------------
> > > arch/mips/loongson64/loongson-3/numa.c | 34 ++++++-------
> > > arch/mips/sgi-ip27/ip27-memory.c | 11 ++---
> > > 4 files changed, 33 insertions(+), 102 deletions(-)
> >
> > Thanks for working on this. Unfortunately it breaks boot for at least a
> > 32r6el_defconfig kernel on QEMU:
> >
> > $ qemu-system-mips64el \
> > -M boston \
> > -kernel arch/mips/boot/vmlinux.gz.itb \
> > -serial stdio \
> > -append "earlycon=uart8250,mmio32,0x17ffe000,115200 console=ttyS0,115200 debug memblock=debug mminit_loglevel=4"
> > [ 0.000000] Linux version 4.19.0-rc1-00008-g82d0f342eecd (pburton@pburton-laptop) (gcc version 8.1.0 (GCC)) #23 SMP Thu Aug 30 14:38:06 PDT 2018
> > [ 0.000000] CPU0 revision is: 0001a900 (MIPS I6400)
> > [ 0.000000] FPU revision is: 20f30300
> > [ 0.000000] MSA revision is: 00000300
> > [ 0.000000] MIPS: machine is img,boston
> > [ 0.000000] Determined physical RAM map:
> > [ 0.000000] memory: 10000000 @ 00000000 (usable)
> > [ 0.000000] memory: 30000000 @ 90000000 (usable)
> > [ 0.000000] earlycon: uart8250 at MMIO32 0x17ffe000 (options '115200')
> > [ 0.000000] bootconsole [uart8250] enabled
> > [ 0.000000] memblock_reserve: [0x00000000-0x009a8fff] setup_arch+0x224/0x718
> > [ 0.000000] memblock_reserve: [0x01360000-0x01361ca7] setup_arch+0x3d8/0x718
> > [ 0.000000] Initrd not found or empty - disabling initrd
> > [ 0.000000] memblock_virt_alloc_try_nid: 7336 bytes align=0x40 nid=-1 from=0x00000000 max_addr=0x00000000 early_init_dt_alloc_memory_arch+0x20/0x2c
> > [ 0.000000] memblock_reserve: [0xbfffe340-0xbfffffe7] memblock_virt_alloc_internal+0x120/0x1ec
> > <hang>
> >
> > It looks like we took a TLB store exception after calling memset() with
> > a bogus address from memblock_virt_alloc_try_nid() or something inlined
> > into it.
>
> Memblock tries to allocate from the top and the resulting address ends up
> in the high memory.
>
> With the hunk below I was able to get to "VFS: Cannot open root device"
>
> diff --git a/arch/mips/kernel/setup.c b/arch/mips/kernel/setup.c
> index 4114d3c..4a9b0f7 100644
> --- a/arch/mips/kernel/setup.c
> +++ b/arch/mips/kernel/setup.c
> @@ -577,6 +577,8 @@ static void __init bootmem_init(void)
> * Reserve initrd memory if needed.
> */
> finalize_initrd();
> +
> + memblock_set_bottom_up(true);
> }

That does seem to fix it, and some basic tests are looking good.

I notice you submitted this as part of your larger series to remove
bootmem - are you still happy for me to take this one through mips-next?

Thanks,
Paul

2018-09-05 18:40:22

by Mike Rapoport

[permalink] [raw]
Subject: Re: [PATCH RESEND] mips: switch to NO_BOOTMEM

On Wed, Sep 05, 2018 at 10:47:10AM -0700, Paul Burton wrote:
> Hi Mike,
>
> On Sat, Sep 01, 2018 at 12:17:48AM +0300, Mike Rapoport wrote:
> > On Thu, Aug 30, 2018 at 02:48:57PM -0700, Paul Burton wrote:
> > > On Mon, Aug 27, 2018 at 10:59:35AM +0300, Mike Rapoport wrote:
> > > > MIPS already has memblock support and all the memory is already registered
> > > > with it.
> > > >
> > > > This patch replaces bootmem memory reservations with memblock ones and
> > > > removes the bootmem initialization.
> > > >
> > > > Signed-off-by: Mike Rapoport <[email protected]>
> > > > ---
> > > >
> > > > arch/mips/Kconfig | 1 +
> > > > arch/mips/kernel/setup.c | 89 +++++-----------------------------
> > > > arch/mips/loongson64/loongson-3/numa.c | 34 ++++++-------
> > > > arch/mips/sgi-ip27/ip27-memory.c | 11 ++---
> > > > 4 files changed, 33 insertions(+), 102 deletions(-)
> > >
> > > Thanks for working on this. Unfortunately it breaks boot for at least a
> > > 32r6el_defconfig kernel on QEMU:
> > >
> > > $ qemu-system-mips64el \
> > > -M boston \
> > > -kernel arch/mips/boot/vmlinux.gz.itb \
> > > -serial stdio \
> > > -append "earlycon=uart8250,mmio32,0x17ffe000,115200 console=ttyS0,115200 debug memblock=debug mminit_loglevel=4"
> > > [ 0.000000] Linux version 4.19.0-rc1-00008-g82d0f342eecd (pburton@pburton-laptop) (gcc version 8.1.0 (GCC)) #23 SMP Thu Aug 30 14:38:06 PDT 2018
> > > [ 0.000000] CPU0 revision is: 0001a900 (MIPS I6400)
> > > [ 0.000000] FPU revision is: 20f30300
> > > [ 0.000000] MSA revision is: 00000300
> > > [ 0.000000] MIPS: machine is img,boston
> > > [ 0.000000] Determined physical RAM map:
> > > [ 0.000000] memory: 10000000 @ 00000000 (usable)
> > > [ 0.000000] memory: 30000000 @ 90000000 (usable)
> > > [ 0.000000] earlycon: uart8250 at MMIO32 0x17ffe000 (options '115200')
> > > [ 0.000000] bootconsole [uart8250] enabled
> > > [ 0.000000] memblock_reserve: [0x00000000-0x009a8fff] setup_arch+0x224/0x718
> > > [ 0.000000] memblock_reserve: [0x01360000-0x01361ca7] setup_arch+0x3d8/0x718
> > > [ 0.000000] Initrd not found or empty - disabling initrd
> > > [ 0.000000] memblock_virt_alloc_try_nid: 7336 bytes align=0x40 nid=-1 from=0x00000000 max_addr=0x00000000 early_init_dt_alloc_memory_arch+0x20/0x2c
> > > [ 0.000000] memblock_reserve: [0xbfffe340-0xbfffffe7] memblock_virt_alloc_internal+0x120/0x1ec
> > > <hang>
> > >
> > > It looks like we took a TLB store exception after calling memset() with
> > > a bogus address from memblock_virt_alloc_try_nid() or something inlined
> > > into it.
> >
> > Memblock tries to allocate from the top and the resulting address ends up
> > in the high memory.
> >
> > With the hunk below I was able to get to "VFS: Cannot open root device"
> >
> > diff --git a/arch/mips/kernel/setup.c b/arch/mips/kernel/setup.c
> > index 4114d3c..4a9b0f7 100644
> > --- a/arch/mips/kernel/setup.c
> > +++ b/arch/mips/kernel/setup.c
> > @@ -577,6 +577,8 @@ static void __init bootmem_init(void)
> > * Reserve initrd memory if needed.
> > */
> > finalize_initrd();
> > +
> > + memblock_set_bottom_up(true);
> > }
>
> That does seem to fix it, and some basic tests are looking good.

The bottom up mode has the downside of allocating memory below
MAX_DMA_ADDRESS.

I'd like to check if memblock_set_current_limit(max_low_pfn) will also fix
the issue, at least with the limited tests I can do with qemu.

> I notice you submitted this as part of your larger series to remove
> bootmem - are you still happy for me to take this one through mips-next?

Sure, I've just posted it as the part of the larger series for completeness.

I believe that in the next few days I'll be able to verify whether
memblock_set_current_limit() can be used instead of
memblock_set_bottom_up() and I'll resend the patch then.

> Thanks,
> Paul
>

--
Sincerely yours,
Mike.


2018-09-06 05:31:45

by Mike Rapoport

[permalink] [raw]
Subject: Re: [PATCH RESEND] mips: switch to NO_BOOTMEM

Hi Sergey,

On Wed, Sep 05, 2018 at 11:09:13PM +0300, Fancer's opinion wrote:
> Hello, Mike
> Could you CC me next time you send that larger patchset?

The larger patchset is here:

https://lore.kernel.org/lkml/[email protected]/

> -Sergey
>
>
> On Wed, Sep 5, 2018 at 9:38 PM Mike Rapoport <[email protected]> wrote:
>
> On Wed, Sep 05, 2018 at 10:47:10AM -0700, Paul Burton wrote:
> > Hi Mike,
> >
> > On Sat, Sep 01, 2018 at 12:17:48AM +0300, Mike Rapoport wrote:
> > > On Thu, Aug 30, 2018 at 02:48:57PM -0700, Paul Burton wrote:
> > > > On Mon, Aug 27, 2018 at 10:59:35AM +0300, Mike Rapoport wrote:
> > > > > MIPS already has memblock support and all the memory is already
> registered
> > > > > with it.
> > > > >
> > > > > This patch replaces bootmem memory reservations with memblock ones
> and
> > > > > removes the bootmem initialization.
> > > > >
> > > > > Signed-off-by: Mike Rapoport <[email protected]>
> > > > > ---
> > > > >
> > > > >? arch/mips/Kconfig? ? ? ? ? ? ? ? ? ? ? |? 1 +
> > > > >? arch/mips/kernel/setup.c? ? ? ? ? ? ? ?| 89
> +++++-----------------------------
> > > > >? arch/mips/loongson64/loongson-3/numa.c | 34 ++++++-------
> > > > >? arch/mips/sgi-ip27/ip27-memory.c? ? ? ?| 11 ++---
> > > > >? 4 files changed, 33 insertions(+), 102 deletions(-)
> > > >
> > > > Thanks for working on this. Unfortunately it breaks boot for at least
> a
> > > > 32r6el_defconfig kernel on QEMU:
> > > >
> > > >? ?$ qemu-system-mips64el \
> > > >? ? ?-M boston \
> > > >? ? ?-kernel arch/mips/boot/vmlinux.gz.itb \
> > > >? ? ?-serial stdio \
> > > >? ? ?-append "earlycon=uart8250,mmio32,0x17ffe000,115200 console=
> ttyS0,115200 debug memblock=debug mminit_loglevel=4"
> > > >? ?[? ? 0.000000] Linux version 4.19.0-rc1-00008-g82d0f342eecd
> (pburton@pburton-laptop) (gcc version 8.1.0 (GCC)) #23 SMP Thu Aug 30
> 14:38:06 PDT 2018
> > > >? ?[? ? 0.000000] CPU0 revision is: 0001a900 (MIPS I6400)
> > > >? ?[? ? 0.000000] FPU revision is: 20f30300
> > > >? ?[? ? 0.000000] MSA revision is: 00000300
> > > >? ?[? ? 0.000000] MIPS: machine is img,boston
> > > >? ?[? ? 0.000000] Determined physical RAM map:
> > > >? ?[? ? 0.000000]? memory: 10000000 @ 00000000 (usable)
> > > >? ?[? ? 0.000000]? memory: 30000000 @ 90000000 (usable)
> > > >? ?[? ? 0.000000] earlycon: uart8250 at MMIO32 0x17ffe000 (options
> '115200')
> > > >? ?[? ? 0.000000] bootconsole [uart8250] enabled
> > > >? ?[? ? 0.000000] memblock_reserve: [0x00000000-0x009a8fff]
> setup_arch+0x224/0x718
> > > >? ?[? ? 0.000000] memblock_reserve: [0x01360000-0x01361ca7]
> setup_arch+0x3d8/0x718
> > > >? ?[? ? 0.000000] Initrd not found or empty - disabling initrd
> > > >? ?[? ? 0.000000] memblock_virt_alloc_try_nid: 7336 bytes align=0x40
> nid=-1 from=0x00000000 max_addr=0x00000000
> early_init_dt_alloc_memory_arch+0x20/0x2c
> > > >? ?[? ? 0.000000] memblock_reserve: [0xbfffe340-0xbfffffe7]
> memblock_virt_alloc_internal+0x120/0x1ec
> > > >? ?<hang>
> > > >
> > > > It looks like we took a TLB store exception after calling memset()
> with
> > > > a bogus address from memblock_virt_alloc_try_nid() or something
> inlined
> > > > into it.
> > >
> > > Memblock tries to allocate from the top and the resulting address ends
> up
> > > in the high memory.
> > >
> > > With the hunk below I was able to get to "VFS: Cannot open root device"
> > >
> > > diff --git a/arch/mips/kernel/setup.c b/arch/mips/kernel/setup.c
> > > index 4114d3c..4a9b0f7 100644
> > > --- a/arch/mips/kernel/setup.c
> > > +++ b/arch/mips/kernel/setup.c
> > > @@ -577,6 +577,8 @@ static void __init bootmem_init(void)
> > >? ? ? ? ? * Reserve initrd memory if needed.
> > >? ? ? ? ? */
> > >? ? ? ? ?finalize_initrd();
> > > +
> > > +? ? ? ?memblock_set_bottom_up(true);
> > >? }
> >
> > That does seem to fix it, and some basic tests are looking good.
>
> The bottom up mode has the downside of allocating memory below
> MAX_DMA_ADDRESS.
>
> I'd like to check if memblock_set_current_limit(max_low_pfn) will also fix
> the issue, at least with the limited tests I can do with qemu.
>
> > I notice you submitted this as part of your larger series to remove
> > bootmem - are you still happy for me to take this one through mips-next?
>
> Sure, I've just posted it as the part of the larger series for
> completeness.
>
> I believe that in the next few days I'll be able to verify whether
> memblock_set_current_limit() can be used instead of
> memblock_set_bottom_up() and I'll resend the patch then.
>
> > Thanks,
> >? ? ?Paul
> >
>
> --
> Sincerely yours,
> Mike.
>
>

--
Sincerely yours,
Mike.