Re: [PATCH v5 1/9] MIPS: Avoid future duplicate elf core header reservation

On Mon, Aug 23, 2021 at 12:17:50PM +0200, Geert Uytterhoeven wrote:

Hi Mike,

On Mon, Aug 16, 2021 at 7:52 AM Mike Rapoport [off-list ref] wrote:

quoted

On Wed, Aug 11, 2021 at 10:50:59AM +0200, Geert Uytterhoeven wrote:

quoted

Prepare for early_init_fdt_scan_reserved_mem() reserving the memory
occupied by an elf core header described in the device tree.
As arch_mem_init() calls early_init_fdt_scan_reserved_mem() before
mips_reserve_vmcore(), the latter needs to check if the memory has
already been reserved before.

Doing memblock_reserve() for the same region is usually fine, did you
encounter any issues without this patch?

Does it also work if the same region is part of an earlier larger
reservation?  I am no memblock expert, so I don't know.
I didn't run into any issues, as my MIPS platform is non-DT, but I
assume arch/arm64/mm/init.c:reserve_elfcorehdr() had the check for
a reason.

The memory will be reserved regardless of the earlier reservation, the
issue may appear when the reservations are made for different purpose. E.g.
if there was crash kernel allocation before the reservation of elfcorehdr.

The check in such case will prevent the second reservation, but, at least
in arch/arm64/mm/init.c:reserve_elfcorehdr() it does not seem to prevent
different users of the overlapping regions to step on each others toes.

Moreover, arm64::reserve_elfcorehdr() seems buggy to me, because of there
is only a partial overlap of the elfcorehdr with the previous reservation,
the non-overlapping part of elfcorehdr won't get reserved at all.

Thanks!

quoted

quoted

Note that mips_reserve_vmcore() cannot just be removed, as not all MIPS
systems use DT.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v5:
  - New.
---
 arch/mips/kernel/setup.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/mips/kernel/setup.c b/arch/mips/kernel/setup.c
index 23a140327a0bac1b..4693add05743d78b 100644
--- a/arch/mips/kernel/setup.c
+++ b/arch/mips/kernel/setup.c

@@ -429,7 +429,8 @@ static void __init mips_reserve_vmcore(void)
      pr_info("Reserving %ldKB of memory at %ldKB for kdump\n",
              (unsigned long)elfcorehdr_size >> 10, (unsigned long)elfcorehdr_addr >> 10);

-     memblock_reserve(elfcorehdr_addr, elfcorehdr_size);
+     if (!memblock_is_region_reserved(elfcorehdr_addr, elfcorehdr_size)
+             memblock_reserve(elfcorehdr_addr, elfcorehdr_size);
 #endif
 }

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

-- 
Sincerely yours,
Mike.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel