Re: [PATCH] powerpc/crash: sort crash memory ranges before preparing elfcorehdr

[Ritesh Harjani (IBM) <ritesh.list@gmail.com>]

Sourabh Jain <sourabhjain@linux.ibm.com> writes:

> During a memory hot-remove event, the elfcorehdr is rebuilt to exclude
> the removed memory. While updating the crash memory ranges for this
> operation, the crash memory ranges array can become unsorted. This
> happens because remove_mem_range() may split a memory range into two
> parts and append the higher-address part as a separate range at the end
> of the array.
>
> So far, no issues have been observed due to the unsorted crash memory
> ranges. However, this could lead to problems once crash memory range
> removal is handled by generic code, as introduced in the upcoming
> patches in this series.

Do you have the link of the discussion, to where you have pointed about,
what sort of problems this unsorted memory ranges might bring up with
the new patch series?


>
> Currently, powerpc uses a platform-specific function,
> remove_mem_range(), to exclude hot-removed memory from the crash memory
> ranges. This function performs the same task as the generic
> crash_exclude_mem_range() in crash_core.c. The generic helper also
> ensures that the crash memory ranges remain sorted. So remove the
> redundant powerpc-specific implementation and instead call
> crash_exclude_mem_range_guarded() (which internally calls
> crash_exclude_mem_range()) to exclude the hot-removed memory ranges.
>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Baoquan he <bhe@redhat.com>
> Cc: Jinjie Ruan <ruanjinjie@huawei.com>
> Cc: Hari Bathini <hbathini@linux.ibm.com>
> Cc: Madhavan Srinivasan <maddy@linux.ibm.com>
> Cc: Mahesh Salgaonkar <mahesh@linux.ibm.com>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
> Cc: Shivang Upadhyay <shivangu@linux.ibm.com>
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>
> ---
>
> Requesting this patch to part of the below patch series.
> https://lore.kernel.org/all/20260209095931.2813152-1-ruanjinjie@huawei.com/
>
> ---
>  arch/powerpc/include/asm/kexec_ranges.h |  4 +-
>  arch/powerpc/kexec/crash.c              |  5 +-
>  arch/powerpc/kexec/ranges.c             | 87 +------------------------
>  3 files changed, 7 insertions(+), 89 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/kexec_ranges.h b/arch/powerpc/include/asm/kexec_ranges.h
> index 14055896cbcb..ad95e3792d10 100644
> --- a/arch/powerpc/include/asm/kexec_ranges.h
> +++ b/arch/powerpc/include/asm/kexec_ranges.h
> @@ -7,7 +7,9 @@
>  void sort_memory_ranges(struct crash_mem *mrngs, bool merge);
>  struct crash_mem *realloc_mem_ranges(struct crash_mem **mem_ranges);
>  int add_mem_range(struct crash_mem **mem_ranges, u64 base, u64 size);
> -int remove_mem_range(struct crash_mem **mem_ranges, u64 base, u64 size);
> +int crash_exclude_mem_range_guarded(struct crash_mem **mem_ranges,
> +				    unsigned long long mstart,
> +				    unsigned long long mend);
>  int get_exclude_memory_ranges(struct crash_mem **mem_ranges);
>  int get_reserved_memory_ranges(struct crash_mem **mem_ranges);
>  int get_crash_memory_ranges(struct crash_mem **mem_ranges);
> diff --git a/arch/powerpc/kexec/crash.c b/arch/powerpc/kexec/crash.c
> index a325c1c02f96..31462e8415ff 100644
> --- a/arch/powerpc/kexec/crash.c
> +++ b/arch/powerpc/kexec/crash.c
> @@ -431,7 +431,7 @@ static void update_crash_elfcorehdr(struct kimage *image, struct memory_notify *
>  	struct crash_mem *cmem = NULL;
>  	struct kexec_segment *ksegment;
>  	void *ptr, *mem, *elfbuf = NULL;
> -	unsigned long elfsz, memsz, base_addr, size;
> +	unsigned long elfsz, memsz, base_addr, size, end;
>  
>  	ksegment = &image->segment[image->elfcorehdr_index];
>  	mem = (void *) ksegment->mem;
> @@ -450,7 +450,8 @@ static void update_crash_elfcorehdr(struct kimage *image, struct memory_notify *
>  	if (image->hp_action == KEXEC_CRASH_HP_REMOVE_MEMORY) {
>  		base_addr = PFN_PHYS(mn->start_pfn);
>  		size = mn->nr_pages * PAGE_SIZE;
> -		ret = remove_mem_range(&cmem, base_addr, size);
> +		end = base_addr - size - 1;


end should be "base_addr + size - 1", isn't it?


> +		ret = crash_exclude_mem_range_guarded(&cmem, base_addr, end);
>  		if (ret) {
>  			pr_err("Failed to remove hot-unplugged memory from crash memory ranges\n");
>  			goto out;

-ritesh