Re: [PATCH v6 03/10] crash_core: fold duplicated memblock arch hooks into the weak default

[Baoquan He]

On 09/18/26 at 04:45pm, Wandun wrote:
> 
> 
> On 9/18/26 15:30, Baoquan He wrote:
> > On 09/02/26 at 03:31pm, Wandun Chen wrote:
> >> From: Wandun Chen <chenwandun@xxxxxxxxxxx>
> >>
> >> arm64, loongarch and riscv open-code the same memblock walk in
> >> arch_get_system_nr_ranges() and arch_crash_populate_cmem(). Move it into
> >> the __weak defaults in kernel/crash_core.c and delete the arch copies.
> >>
> >> No functional change.
> > 
> > In one version, you also tried to remove these functions on x86, I could
> > remember it wrong. 
> 
> Yes, the x86 implementation can also remove these functions and use the same
> approach as arm64, RISC-V, and LoongArch. However since this change would not
> reduce the vmcore size on x86 at all, I did not include it in the current series.
> 
> I plan to refactor this part on x86 after this series.

OK, that's fine. I remember I saw the code, but couldn't find it in this
patch. Thanks for telling.

> 
> 
> 
> > 
> > Anyway, this looks good to me.
> > 
> > Acked-by: Baoquan He <baoquan.he@xxxxxxxxx>
> > 
> >>
> >> Signed-off-by: Wandun Chen <chenwandun@xxxxxxxxxxx>
> >> Tested-by: Meijing Zhao <zhaomeijing@xxxxxxxxxxx>
> >> ---
> >>  arch/arm64/kernel/machine_kexec_file.c     | 29 ----------------------
> >>  arch/loongarch/kernel/machine_kexec_file.c | 27 --------------------
> >>  arch/riscv/kernel/machine_kexec_file.c     | 26 -------------------
> >>  kernel/crash_core.c                        | 25 +++++++++++++++++--
> >>  4 files changed, 23 insertions(+), 84 deletions(-)
> >>
> >> diff --git a/arch/arm64/kernel/machine_kexec_file.c b/arch/arm64/kernel/machine_kexec_file.c
> >> index 854d872dfd0f..34d944d3f22f 100644
> >> --- a/arch/arm64/kernel/machine_kexec_file.c
> >> +++ b/arch/arm64/kernel/machine_kexec_file.c
> >> @@ -14,7 +14,6 @@
> >>  #include <linux/kernel.h>
> >>  #include <linux/kexec.h>
> >>  #include <linux/libfdt.h>
> >> -#include <linux/memblock.h>
> >>  #include <linux/of.h>
> >>  #include <linux/of_fdt.h>
> >>  #include <linux/slab.h>
> >> @@ -39,34 +38,6 @@ int arch_kimage_file_post_load_cleanup(struct kimage *image)
> >>  	return kexec_image_post_load_cleanup_default(image);
> >>  }
> >>  
> >> -#ifdef CONFIG_CRASH_DUMP
> >> -unsigned int arch_get_system_nr_ranges(void)
> >> -{
> >> -	unsigned int nr_ranges = 2 + crashk_cma_cnt; /* for exclusion of crashkernel region */
> >> -	phys_addr_t start, end;
> >> -	u64 i;
> >> -
> >> -	for_each_mem_range(i, &start, &end)
> >> -		nr_ranges++;
> >> -
> >> -	return nr_ranges;
> >> -}
> >> -
> >> -int arch_crash_populate_cmem(struct crash_mem *cmem)
> >> -{
> >> -	phys_addr_t start, end;
> >> -	u64 i;
> >> -
> >> -	for_each_mem_range(i, &start, &end) {
> >> -		cmem->ranges[cmem->nr_ranges].start = start;
> >> -		cmem->ranges[cmem->nr_ranges].end = end - 1;
> >> -		cmem->nr_ranges++;
> >> -	}
> >> -
> >> -	return 0;
> >> -}
> >> -#endif
> >> -
> >>  /*
> >>   * Tries to add the initrd and DTB to the image. If it is not possible to find
> >>   * valid locations, this function will undo changes to the image and return non
> >> diff --git a/arch/loongarch/kernel/machine_kexec_file.c b/arch/loongarch/kernel/machine_kexec_file.c
> >> index 5412aa9f3568..481b8f906a56 100644
> >> --- a/arch/loongarch/kernel/machine_kexec_file.c
> >> +++ b/arch/loongarch/kernel/machine_kexec_file.c
> >> @@ -13,7 +13,6 @@
> >>  #include <linux/ioport.h>
> >>  #include <linux/kernel.h>
> >>  #include <linux/kexec.h>
> >> -#include <linux/memblock.h>
> >>  #include <linux/slab.h>
> >>  #include <linux/string.h>
> >>  #include <linux/types.h>
> >> @@ -56,32 +55,6 @@ static void cmdline_add_initrd(struct kimage *image, unsigned long *cmdline_tmpl
> >>  }
> >>  
> >>  #ifdef CONFIG_CRASH_DUMP
> >> -unsigned int arch_get_system_nr_ranges(void)
> >> -{
> >> -	int nr_ranges = 2; /* for exclusion of crashkernel region */
> >> -	phys_addr_t start, end;
> >> -	uint64_t i;
> >> -
> >> -	for_each_mem_range(i, &start, &end)
> >> -		nr_ranges++;
> >> -
> >> -	return nr_ranges;
> >> -}
> >> -
> >> -int arch_crash_populate_cmem(struct crash_mem *cmem)
> >> -{
> >> -	phys_addr_t start, end;
> >> -	uint64_t i;
> >> -
> >> -	for_each_mem_range(i, &start, &end) {
> >> -		cmem->ranges[cmem->nr_ranges].start = start;
> >> -		cmem->ranges[cmem->nr_ranges].end = end - 1;
> >> -		cmem->nr_ranges++;
> >> -	}
> >> -
> >> -	return 0;
> >> -}
> >> -
> >>  /*
> >>   * Add the "mem=size@start" command line parameter to command line, indicating the
> >>   * memory region the new kernel can use to boot into.
> >> diff --git a/arch/riscv/kernel/machine_kexec_file.c b/arch/riscv/kernel/machine_kexec_file.c
> >> index 808e872f4063..fdc6695e8588 100644
> >> --- a/arch/riscv/kernel/machine_kexec_file.c
> >> +++ b/arch/riscv/kernel/machine_kexec_file.c
> >> @@ -37,32 +37,6 @@ int arch_kimage_file_post_load_cleanup(struct kimage *image)
> >>  }
> >>  
> >>  #ifdef CONFIG_CRASH_DUMP
> >> -unsigned int arch_get_system_nr_ranges(void)
> >> -{
> >> -	unsigned int nr_ranges = 2 + crashk_cma_cnt; /* For exclusion of crashkernel region */
> >> -	u64 i;
> >> -	phys_addr_t start, end;
> >> -
> >> -	for_each_mem_range(i, &start, &end)
> >> -		nr_ranges++;
> >> -
> >> -	return nr_ranges;
> >> -}
> >> -
> >> -int arch_crash_populate_cmem(struct crash_mem *cmem)
> >> -{
> >> -	u64 i;
> >> -	phys_addr_t start, end;
> >> -
> >> -	for_each_mem_range(i, &start, &end) {
> >> -		cmem->ranges[cmem->nr_ranges].start = start;
> >> -		cmem->ranges[cmem->nr_ranges].end = end - 1;
> >> -		cmem->nr_ranges++;
> >> -	}
> >> -
> >> -	return 0;
> >> -}
> >> -
> >>  static char *setup_kdump_cmdline(struct kimage *image, char *cmdline,
> >>  				 unsigned long cmdline_len)
> >>  {
> >> diff --git a/kernel/crash_core.c b/kernel/crash_core.c
> >> index 2b36aa9fade0..77285ae3ce60 100644
> >> --- a/kernel/crash_core.c
> >> +++ b/kernel/crash_core.c
> >> @@ -281,8 +281,29 @@ static struct crash_mem *alloc_cmem(unsigned int nr_ranges)
> >>  	return cmem;
> >>  }
> >>  
> >> -unsigned int __weak arch_get_system_nr_ranges(void) { return 0; }
> >> -int __weak arch_crash_populate_cmem(struct crash_mem *cmem) { return -1; }
> >> +unsigned int __weak arch_get_system_nr_ranges(void)
> >> +{
> >> +	unsigned int nr_ranges = 2 + crashk_cma_cnt; /* crashk_res + crashk_low_res, +CMA splits */
> >> +	phys_addr_t start, end;
> >> +	u64 i;
> >> +
> >> +	for_each_mem_range(i, &start, &end)
> >> +		nr_ranges++;
> >> +	return nr_ranges;
> >> +}
> >> +
> >> +int __weak arch_crash_populate_cmem(struct crash_mem *cmem)
> >> +{
> >> +	phys_addr_t start, end;
> >> +	u64 i;
> >> +
> >> +	for_each_mem_range(i, &start, &end) {
> >> +		cmem->ranges[cmem->nr_ranges].start = start;
> >> +		cmem->ranges[cmem->nr_ranges].end = end - 1;
> >> +		cmem->nr_ranges++;
> >> +	}
> >> +	return 0;
> >> +}
> >>  int __weak arch_crash_exclude_ranges(struct crash_mem *cmem) { return 0; }
> >>  
> >>  int __weak arch_crash_exclude_mem_range(struct crash_mem **mem,
> >> -- 
> >> 2.43.0
> >>
>