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

[Sourabh Jain <sourabhjain@linux.ibm.com>]

On 11/02/26 09:05, Ritesh Harjani (IBM) wrote:
> 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?

No, we didn’t discuss this anywhere. This occurred to me while I
was reviewing [1].

The issue is that [1] moves the exclusion of crashkernel reserved
memory into a generic function. With [1] included, the architecture
prepares the initial set of crash_mem ranges, and later the generic
function removes the crashkernel memory range from those crash_mem
ranges by calling crash_exclude_mem_range().

crash_exclude_mem_range() expects the crash_mem ranges to be sorted,
but that may not be true on powerpc if the ranges come from the
hotplug remove path, where remove_mem_range() is called to exclude
the hot-removed memory range.


[1] 
https://lore.kernel.org/all/20260209095931.2813152-1-ruanjinjie@huawei.com/
>
>
>> 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?

oops, my bad. I will fix it in v2.


Thanks for the review, Ritesh.
>
>
>> +		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