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

[Baoquan He]

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. 

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
>