[PATCH] kdump: Defer the insertion of crashkernel resources

[PATCH] kdump: Defer the insertion of crashkernel resources

[Huacai Chen]

In /proc/iomem, sub-regions should be inserted after their parent,
otherwise the insertion of parent resource fails. But after generic
crashkernel reservation applied, in both RISC-V and ARM64 (LoongArch
will also use generic reservation later on), crashkernel resources are
inserted before their parent, which causes the parent disappear in
/proc/iomem. So we defer the insertion of crashkernel resources to an
early_initcall().

1, Without 'crashkernel' parameter:

 100d0100-100d01ff : LOON0001:00
   100d0100-100d01ff : LOON0001:00 LOON0001:00
 100e0000-100e0bff : LOON0002:00
   100e0000-100e0bff : LOON0002:00 LOON0002:00
 1fe001e0-1fe001e7 : serial
 90400000-fa17ffff : System RAM
   f6220000-f622ffff : Reserved
   f9ee0000-f9ee3fff : Reserved
   fa120000-fa17ffff : Reserved
 fa190000-fe0bffff : System RAM
   fa190000-fa1bffff : Reserved
 fe4e0000-47fffffff : System RAM
   43c000000-441ffffff : Reserved
   47ff98000-47ffa3fff : Reserved
   47ffa4000-47ffa7fff : Reserved
   47ffa8000-47ffabfff : Reserved
   47ffac000-47ffaffff : Reserved
   47ffb0000-47ffb3fff : Reserved

2, With 'crashkernel' parameter, before this patch:

 100d0100-100d01ff : LOON0001:00
   100d0100-100d01ff : LOON0001:00 LOON0001:00
 100e0000-100e0bff : LOON0002:00
   100e0000-100e0bff : LOON0002:00 LOON0002:00
 1fe001e0-1fe001e7 : serial
 e6200000-f61fffff : Crash kernel
 fa190000-fe0bffff : System RAM
   fa190000-fa1bffff : Reserved
 fe4e0000-47fffffff : System RAM
   43c000000-441ffffff : Reserved
   47ff98000-47ffa3fff : Reserved
   47ffa4000-47ffa7fff : Reserved
   47ffa8000-47ffabfff : Reserved
   47ffac000-47ffaffff : Reserved
   47ffb0000-47ffb3fff : Reserved

3, With 'crashkernel' parameter, after this patch:

 100d0100-100d01ff : LOON0001:00
   100d0100-100d01ff : LOON0001:00 LOON0001:00
 100e0000-100e0bff : LOON0002:00
   100e0000-100e0bff : LOON0002:00 LOON0002:00
 1fe001e0-1fe001e7 : serial
 90400000-fa17ffff : System RAM
   e6200000-f61fffff : Crash kernel
   f6220000-f622ffff : Reserved
   f9ee0000-f9ee3fff : Reserved
   fa120000-fa17ffff : Reserved
 fa190000-fe0bffff : System RAM
   fa190000-fa1bffff : Reserved
 fe4e0000-47fffffff : System RAM
   43c000000-441ffffff : Reserved
   47ff98000-47ffa3fff : Reserved
   47ffa4000-47ffa7fff : Reserved
   47ffa8000-47ffabfff : Reserved
   47ffac000-47ffaffff : Reserved
   47ffb0000-47ffb3fff : Reserved

Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
---
 kernel/crash_core.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/kernel/crash_core.c b/kernel/crash_core.c
index d4313b53837e..755d8d4ef5b0 100644
--- a/kernel/crash_core.c
+++ b/kernel/crash_core.c
@@ -377,7 +377,6 @@ static int __init reserve_crashkernel_low(unsigned long long low_size)
 
 	crashk_low_res.start = low_base;
 	crashk_low_res.end   = low_base + low_size - 1;
-	insert_resource(&iomem_resource, &crashk_low_res);
 #endif
 	return 0;
 }
@@ -459,8 +458,19 @@ void __init reserve_crashkernel_generic(char *cmdline,
 
 	crashk_res.start = crash_base;
 	crashk_res.end = crash_base + crash_size - 1;
-	insert_resource(&iomem_resource, &crashk_res);
 }
+
+static __init int insert_crashkernel_resources(void)
+{
+	if (crashk_res.start < crashk_res.end)
+		insert_resource(&iomem_resource, &crashk_res);
+
+	if (crashk_low_res.start < crashk_low_res.end)
+		insert_resource(&iomem_resource, &crashk_low_res);
+
+	return 0;
+}
+early_initcall(insert_crashkernel_resources);
 #endif
 
 int crash_prepare_elf64_headers(struct crash_mem *mem, int need_kernel_map,
-- 
2.39.3

Re: [PATCH] kdump: Defer the insertion of crashkernel resources

[Baoquan He]

Huacai,

On 12/29/23 at 04:02pm, Huacai Chen wrote:
> In /proc/iomem, sub-regions should be inserted after their parent,
> otherwise the insertion of parent resource fails. But after generic
> crashkernel reservation applied, in both RISC-V and ARM64 (LoongArch
> will also use generic reservation later on), crashkernel resources are
> inserted before their parent, which causes the parent disappear in
> /proc/iomem. So we defer the insertion of crashkernel resources to an
> early_initcall().
> 
> 1, Without 'crashkernel' parameter:
> 
>  100d0100-100d01ff : LOON0001:00
>    100d0100-100d01ff : LOON0001:00 LOON0001:00
>  100e0000-100e0bff : LOON0002:00
>    100e0000-100e0bff : LOON0002:00 LOON0002:00
>  1fe001e0-1fe001e7 : serial
>  90400000-fa17ffff : System RAM
>    f6220000-f622ffff : Reserved
>    f9ee0000-f9ee3fff : Reserved
>    fa120000-fa17ffff : Reserved
>  fa190000-fe0bffff : System RAM
>    fa190000-fa1bffff : Reserved
>  fe4e0000-47fffffff : System RAM
>    43c000000-441ffffff : Reserved
>    47ff98000-47ffa3fff : Reserved
>    47ffa4000-47ffa7fff : Reserved
>    47ffa8000-47ffabfff : Reserved
>    47ffac000-47ffaffff : Reserved
>    47ffb0000-47ffb3fff : Reserved
> 
> 2, With 'crashkernel' parameter, before this patch:
> 
>  100d0100-100d01ff : LOON0001:00
>    100d0100-100d01ff : LOON0001:00 LOON0001:00
>  100e0000-100e0bff : LOON0002:00
>    100e0000-100e0bff : LOON0002:00 LOON0002:00
>  1fe001e0-1fe001e7 : serial
>  e6200000-f61fffff : Crash kernel
>  fa190000-fe0bffff : System RAM
>    fa190000-fa1bffff : Reserved
>  fe4e0000-47fffffff : System RAM
>    43c000000-441ffffff : Reserved
>    47ff98000-47ffa3fff : Reserved
>    47ffa4000-47ffa7fff : Reserved
>    47ffa8000-47ffabfff : Reserved
>    47ffac000-47ffaffff : Reserved
>    47ffb0000-47ffb3fff : Reserved
> 
> 3, With 'crashkernel' parameter, after this patch:
> 
>  100d0100-100d01ff : LOON0001:00
>    100d0100-100d01ff : LOON0001:00 LOON0001:00
>  100e0000-100e0bff : LOON0002:00
>    100e0000-100e0bff : LOON0002:00 LOON0002:00
>  1fe001e0-1fe001e7 : serial
>  90400000-fa17ffff : System RAM
>    e6200000-f61fffff : Crash kernel
>    f6220000-f622ffff : Reserved
>    f9ee0000-f9ee3fff : Reserved
>    fa120000-fa17ffff : Reserved
>  fa190000-fe0bffff : System RAM
>    fa190000-fa1bffff : Reserved
>  fe4e0000-47fffffff : System RAM
>    43c000000-441ffffff : Reserved
>    47ff98000-47ffa3fff : Reserved
>    47ffa4000-47ffa7fff : Reserved
>    47ffa8000-47ffabfff : Reserved
>    47ffac000-47ffaffff : Reserved
>    47ffb0000-47ffb3fff : Reserved

This looks like a great catch. I am curious where arm64 and loongarch
insert the system RAM range into iomem before crashk_res and
crashk_low_res. On x86, it should be done by pci or acpi init which is
earlier than crashkernel parsing and inserting into iomem, just went\
through codes, haven't adding debugging code to print.

> 
> Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
> ---
>  kernel/crash_core.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/crash_core.c b/kernel/crash_core.c
> index d4313b53837e..755d8d4ef5b0 100644
> --- a/kernel/crash_core.c
> +++ b/kernel/crash_core.c
> @@ -377,7 +377,6 @@ static int __init reserve_crashkernel_low(unsigned long long low_size)
>  
>  	crashk_low_res.start = low_base;
>  	crashk_low_res.end   = low_base + low_size - 1;
> -	insert_resource(&iomem_resource, &crashk_low_res);
>  #endif
>  	return 0;
>  }
> @@ -459,8 +458,19 @@ void __init reserve_crashkernel_generic(char *cmdline,
>  
>  	crashk_res.start = crash_base;
>  	crashk_res.end = crash_base + crash_size - 1;
> -	insert_resource(&iomem_resource, &crashk_res);
>  }
> +
> +static __init int insert_crashkernel_resources(void)
> +{
> +	if (crashk_res.start < crashk_res.end)
> +		insert_resource(&iomem_resource, &crashk_res);
> +
> +	if (crashk_low_res.start < crashk_low_res.end)
> +		insert_resource(&iomem_resource, &crashk_low_res);
> +
> +	return 0;
> +}
> +early_initcall(insert_crashkernel_resources);
>  #endif
>  
>  int crash_prepare_elf64_headers(struct crash_mem *mem, int need_kernel_map,
> -- 
> 2.39.3
> 
> 
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec
> 

Re: [PATCH] kdump: Defer the insertion of crashkernel resources

[Andrew Morton]

On Fri, 29 Dec 2023 16:02:13 +0800 Huacai Chen <chenhuacai@loongson.cn> wrote:

> In /proc/iomem, sub-regions should be inserted after their parent,
> otherwise the insertion of parent resource fails. But after generic
> crashkernel reservation applied, in both RISC-V and ARM64 (LoongArch
> will also use generic reservation later on), crashkernel resources are
> inserted before their parent, which causes the parent disappear in
> /proc/iomem. So we defer the insertion of crashkernel resources to an
> early_initcall().
> 
> ...
>
> --- a/kernel/crash_core.c
> +++ b/kernel/crash_core.c
> @@ -377,7 +377,6 @@ static int __init reserve_crashkernel_low(unsigned long long low_size)
>  
>  	crashk_low_res.start = low_base;
>  	crashk_low_res.end   = low_base + low_size - 1;
> -	insert_resource(&iomem_resource, &crashk_low_res);
>  #endif
>  	return 0;
>  }
> @@ -459,8 +458,19 @@ void __init reserve_crashkernel_generic(char *cmdline,
>  
>  	crashk_res.start = crash_base;
>  	crashk_res.end = crash_base + crash_size - 1;
> -	insert_resource(&iomem_resource, &crashk_res);
>  }
> +
> +static __init int insert_crashkernel_resources(void)
> +{
> +	if (crashk_res.start < crashk_res.end)
> +		insert_resource(&iomem_resource, &crashk_res);
> +
> +	if (crashk_low_res.start < crashk_low_res.end)
> +		insert_resource(&iomem_resource, &crashk_low_res);
> +
> +	return 0;
> +}
> +early_initcall(insert_crashkernel_resources);
>  #endif
>  
>  int crash_prepare_elf64_headers(struct crash_mem *mem, int need_kernel_map,

I'm thinking 

Fixes: 0ab97169aa0 ("crash_core: add generic function to do reservation").

Also, is this a regression?  Were earlier kernels OK?

Re: [PATCH] kdump: Defer the insertion of crashkernel resources

[Baoquan He]

On 01/05/24 at 08:49am, Andrew Morton wrote:
> On Fri, 29 Dec 2023 16:02:13 +0800 Huacai Chen <chenhuacai@loongson.cn> wrote:
> 
> > In /proc/iomem, sub-regions should be inserted after their parent,
> > otherwise the insertion of parent resource fails. But after generic
> > crashkernel reservation applied, in both RISC-V and ARM64 (LoongArch
> > will also use generic reservation later on), crashkernel resources are
> > inserted before their parent, which causes the parent disappear in
> > /proc/iomem. So we defer the insertion of crashkernel resources to an
> > early_initcall().
> > 
> > ...
> >
> > --- a/kernel/crash_core.c
> > +++ b/kernel/crash_core.c
> > @@ -377,7 +377,6 @@ static int __init reserve_crashkernel_low(unsigned long long low_size)
> >  
> >  	crashk_low_res.start = low_base;
> >  	crashk_low_res.end   = low_base + low_size - 1;
> > -	insert_resource(&iomem_resource, &crashk_low_res);
> >  #endif
> >  	return 0;
> >  }
> > @@ -459,8 +458,19 @@ void __init reserve_crashkernel_generic(char *cmdline,
> >  
> >  	crashk_res.start = crash_base;
> >  	crashk_res.end = crash_base + crash_size - 1;
> > -	insert_resource(&iomem_resource, &crashk_res);
> >  }
> > +
> > +static __init int insert_crashkernel_resources(void)
> > +{
> > +	if (crashk_res.start < crashk_res.end)
> > +		insert_resource(&iomem_resource, &crashk_res);
> > +
> > +	if (crashk_low_res.start < crashk_low_res.end)
> > +		insert_resource(&iomem_resource, &crashk_low_res);
> > +
> > +	return 0;
> > +}
> > +early_initcall(insert_crashkernel_resources);
> >  #endif
> >  
> >  int crash_prepare_elf64_headers(struct crash_mem *mem, int need_kernel_map,
> 
> I'm thinking 
> 
> Fixes: 0ab97169aa0 ("crash_core: add generic function to do reservation").

Yes.

> 
> Also, is this a regression?  Were earlier kernels OK?

It's a regression, will impact arm64 in v6.6 kernel. Add below too?

Cc: <stable@vger.kernel.org> # 6.6.x