Re: [PATCH v4 1/3] crash: Exclude crash kernel memory in crash core

[Jinjie Ruan <ruanjinjie@huawei.com>]

On 2026/2/10 20:30, Sourabh Jain wrote:
> Hello Jinjie,
> 
> On 09/02/26 15:29, Jinjie Ruan wrote:
>> The exclude of crashk_res, crashk_low_res and crashk_cma memory
>> are almost identical across different architectures, handling them
>> in the crash core would eliminate a lot of duplication, so do
>> them in the common code.
>>
>> And move the size calculation (and the realloc if needed) into the
>> generic crash core so that:
>>
>> - New CMA regions or future crash-memory types can automatically
>>    accounted for in crash core;
>>
>> - Each architecture no longer has to play whack-a-mole with
>>    its private array size.
>>
>> To achieve the above goal, 4 architecture-specific functions are
>> introduced:
>>
>> - arch_get_system_nr_ranges() and arch_prepare_elf64_ram_headers().
>>    The 1st function pre-counts the number of memory ranges, and
>>    the 2st function fill the memory ranges into the cmem->ranges[] array,
>>    and count the actual number of ranges filled. The default
>> implementation
>>    is consistent with arm64 and loongson.
>>
>> - arch_crash_exclude_mem_range(). Realloc for powerpc. The default
>>    implementation is crash_exclude_mem_range(), and use
>>    crash_exclude_mem_range_guarded() to implement the arch version
>>    for powerpc.
>>
>> - arch_get_crash_memory_ranges(). Get crash memory ranges for arch and
>>    the default implementation is generic across x86, arm64, riscv, and
>>    loongson by using the first two arch functions above. powerpc has its
>>    own implementation by calling get_crash_memory_ranges().
>>
>> Tested on x86, arm64 and riscv with QEMU.
>>
>> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
>> ---
>>   arch/arm64/kernel/machine_kexec_file.c     |  47 +--------
>>   arch/loongarch/kernel/machine_kexec_file.c |  45 +-------
>>   arch/powerpc/include/asm/kexec.h           |  13 +++
>>   arch/powerpc/kexec/crash.c                 |  52 ++++++----
>>   arch/powerpc/kexec/file_load_64.c          |  17 ++-
>>   arch/powerpc/kexec/ranges.c                |  18 +---
>>   arch/riscv/include/asm/kexec.h             |  10 ++
>>   arch/riscv/kernel/machine_kexec_file.c     |  37 ++-----
>>   arch/x86/include/asm/kexec.h               |  10 ++
>>   arch/x86/kernel/crash.c                    | 104 ++-----------------
>>   include/linux/crash_core.h                 | 114 +++++++++++++++++++--
>>   kernel/crash_core.c                        |  71 +++++++++++--
>>   12 files changed, 269 insertions(+), 269 deletions(-)
>>

[...]

>>   extern void crash_ipi_callback(struct pt_regs *regs);
>> diff --git a/arch/powerpc/kexec/crash.c b/arch/powerpc/kexec/crash.c
>> index a325c1c02f96..5ade9a853fb0 100644
>> --- a/arch/powerpc/kexec/crash.c
>> +++ b/arch/powerpc/kexec/crash.c
>> @@ -419,30 +419,21 @@ unsigned int arch_crash_get_elfcorehdr_size(void)
>>       return sizeof(struct elfhdr) + (phdr_cnt * sizeof(Elf64_Phdr));
>>   }
>>   -/**
>> - * update_crash_elfcorehdr() - Recreate the elfcorehdr and replace it
>> with old
>> - *                   elfcorehdr in the kexec segment array.
>> - * @image: the active struct kimage
>> - * @mn: struct memory_notify data handler
>> - */
>> -static void update_crash_elfcorehdr(struct kimage *image, struct
>> memory_notify *mn)
>> +int arch_get_crash_memory_ranges(struct crash_mem **cmem, unsigned
>> long *nr_mem_ranges,
>> +                 struct kimage *image, struct memory_notify *mn)
>>   {
>> +    unsigned long base_addr, size;
>>       int ret;
>> -    struct crash_mem *cmem = NULL;
>> -    struct kexec_segment *ksegment;
>> -    void *ptr, *mem, *elfbuf = NULL;
>> -    unsigned long elfsz, memsz, base_addr, size;
>>   -    ksegment = &image->segment[image->elfcorehdr_index];
>> -    mem = (void *) ksegment->mem;
>> -    memsz = ksegment->memsz;
>> -
>> -    ret = get_crash_memory_ranges(&cmem);
>> +    ret = get_crash_memory_ranges(cmem);
>>       if (ret) {
>>           pr_err("Failed to get crash mem range\n");
>> -        return;
>> +        return ret;
>>       }
>>   +    if (!image || !mn)
>> +        return 0;
>> +
>>       /*
>>        * The hot unplugged memory is part of crash memory ranges,
>>        * remove it here.
>> @@ -450,14 +441,34 @@ 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);
>> +        ret = remove_mem_range(cmem, base_addr, size);
> 
> I like the overall design for handling crashkernel memory exclusion
> in this patch series, especially the way you managed to free the
> crash_mem object (mem) in the generic code (crash_prepare_elf64_headers()).

Thanks for the review.

> 
> However, the way crash memory is prepared after a memory hotplug
> event on powerpc by calling remove_mem_range(), can leave the crash
> memory ranges unsorted. This can cause issues in the generic code
> when excluding crashkernel memory, because crash_exclude_mem_range()
> expects crash_mem to be sorted.

You are absolutely correct.

> 
> So I wrote a simple patch to cover this scenario. Including the
> patch below as the first patch in this series would be helpful.
> https://lore.kernel.org/all/20260210120803.433978-1-sourabhjain@linux.ibm.com/

Thanks for the additional patch. I'll add it as the first patch in the
next revision to ensure crash_mem remains sorted after memory hotplug
events on powerpc.

Best regards,
Jinjie

> 
> Jinjie, will it be possible for you to include the above patch in this
> patch series?

My pleasure.

>>           if (ret) {
>>               pr_err("Failed to remove hot-unplugged memory from crash
>> memory ranges\n");
>> -            goto out;
>> +            return ret;
>>           }
>>       }
>>   -    ret = crash_prepare_elf64_headers(cmem, false, &elfbuf, &elfsz);
>> +    return 0;
>> +}
>> +
>> +/**
>> + * update_crash_elfcorehdr() - Recreate the elfcorehdr and replace it
>> with old
>> + *                   elfcorehdr in the kexec segment array.
>> + * @image: the active struct kimage
>> + * @mn: struct memory_notify data handler
>> + */
>> +static void update_crash_elfcorehdr(struct kimage *image, struct
>> memory_notify *mn)
>> +{
>> +    void *ptr, *mem, *elfbuf = NULL;
>> +    struct kexec_segment *ksegment;
>> +    unsigned long elfsz, memsz;
>> +    int ret;
>> +
>> +    ksegment = &image->segment[image->elfcorehdr_index];
>> +    mem = (void *) ksegment->mem;
>> +    memsz = ksegment->memsz;
>> +
>> +    ret = crash_prepare_elf64_headers(false, &elfbuf, &elfsz, NULL,
>> image, mn);
>>       if (ret) {
>>           pr_err("Failed to prepare elf header\n");
>>           goto out;
>> @@ -486,7 +497,6 @@ static void update_crash_elfcorehdr(struct kimage
>> *image, struct memory_notify *
>>           xchg(&kexec_crash_image, image);
>>       }
>>   out:
>> -    kvfree(cmem);
>>       kvfree(elfbuf);
>>   }

[...]

>> +int crash_prepare_elf64_headers(int need_kernel_map, void **addr,
>> +                unsigned long *sz, unsigned long *nr_mem_ranges,
>> +                struct kimage *image, struct memory_notify *mn)
>>   {
>> -    Elf64_Ehdr *ehdr;
>> -    Elf64_Phdr *phdr;
>>       unsigned long nr_cpus = num_possible_cpus(), nr_phdr, elf_sz;
>> -    unsigned char *buf;
>> -    unsigned int cpu, i;
>>       unsigned long long notes_addr;
>> +    struct crash_mem *mem = NULL;
>>       unsigned long mstart, mend;
>> +    unsigned int cpu, i;
>> +    unsigned char *buf;
>> +    Elf64_Ehdr *ehdr;
>> +    Elf64_Phdr *phdr;
>> +    int ret = 0;
>> +
>> +    ret = arch_get_crash_memory_ranges(&mem, nr_mem_ranges, image, mn);
>> +    if (ret)
>> +        return ret;
>> +
>> +    if (mem) {
>> +        ret = crash_exclude_mem_ranges(mem, nr_mem_ranges);
>> +        if (ret)
>> +            goto out;
>> +    }
>>         /* extra phdr for vmcoreinfo ELF note */
>>       nr_phdr = nr_cpus + 1;
>> @@ -192,8 +240,10 @@ int crash_prepare_elf64_headers(struct crash_mem
>> *mem, int need_kernel_map,
>>       elf_sz = ALIGN(elf_sz, ELF_CORE_HEADER_ALIGN);
>>         buf = vzalloc(elf_sz);
>> -    if (!buf)
>> -        return -ENOMEM;
>> +    if (!buf) {
>> +        ret = -ENOMEM;
>> +        goto out;
>> +    }
>>         ehdr = (Elf64_Ehdr *)buf;
>>       phdr = (Elf64_Phdr *)(ehdr + 1);
>> @@ -262,7 +312,10 @@ int crash_prepare_elf64_headers(struct crash_mem
>> *mem, int need_kernel_map,
>>         *addr = buf;
>>       *sz = elf_sz;
>> -    return 0;
>> +
>> +out:
>> +    kvfree(mem);
>> +    return ret;
>>   }
>>     /**
>