On 2026/2/4 20:32, Sourabh Jain wrote:
> 
> 
> On 04/02/26 15:07, Jinjie Ruan wrote:
>> The exclude of crashk_res, crashk_low_res and crashk_cma memory
>> are almost identical across different architectures, so handling them
>> in the crash core would eliminate a lot of duplication, so do
>> them in the common code.
>>
>> Signed-off-by: Jinjie Ruan <[email protected]>
>> ---
>>   arch/arm64/kernel/machine_kexec_file.c     | 12 -------
>>   arch/loongarch/kernel/machine_kexec_file.c | 12 -------
>>   arch/powerpc/kexec/ranges.c                | 16 ++-------
>>   arch/riscv/kernel/machine_kexec_file.c     |  5 +--
>>   arch/x86/kernel/crash.c                    | 39 ++--------------------
>>   kernel/crash_core.c                        | 28 ++++++++++++++++
>>   6 files changed, 34 insertions(+), 78 deletions(-)
>>

[...]

>> -static int crash_exclude_mem_range_guarded(struct crash_mem
>> **mem_ranges,
>> -                       unsigned long long mstart,
>> -                       unsigned long long mend)
>> +static int crash_realloc_mem_range_guarded(struct crash_mem
>> **mem_ranges)
>>   {
>>       struct crash_mem *tmem = *mem_ranges;
>>   @@ -566,7 +564,7 @@ static int
>> crash_exclude_mem_range_guarded(struct crash_mem **mem_ranges,
>>               return -ENOMEM;
>>       }
>>   -    return crash_exclude_mem_range(tmem, mstart, mend);
>> +    return 0;
>>   }
>>     /**
>> @@ -604,18 +602,10 @@ int get_crash_memory_ranges(struct crash_mem
>> **mem_ranges)
>>               sort_memory_ranges(*mem_ranges, true);
>>       }
>>   -    /* Exclude crashkernel region */
>> -    ret = crash_exclude_mem_range_guarded(mem_ranges,
>> crashk_res.start, crashk_res.end);
>> +    ret = crash_realloc_mem_range_guarded(mem_ranges);
> 
> What if max_nr_ranges - nr_ranges = 1, then no realloc will happen here.
> And in
> elf_header_exclude_ranges we may not enough space to store additional
> memory ranges needed while excluding one or more CMA ranges.

You're absolutely right — if max_nr_ranges - nr_ranges == 1 we skip the
realloc, yet elf_header_exclude_ranges() can easily need more than one
extra slot.

Thanks for catching this.
Jinjie

> 
>>       if (ret)
>>           goto out;
>>   -    for (i = 0; i < crashk_cma_cnt; ++i) {
>> -        ret = crash_exclude_mem_range_guarded(mem_ranges,
>> crashk_cma_ranges[i].start,
>> -                          crashk_cma_ranges[i].end);
>> -        if (ret)
>> -            goto out;
>> -    }
>> -
>>       /*
>>        * FIXME: For now, stay in parity with kexec-tools but if RTAS/OPAL
>>        *        regions are exported to save their context at the time of

[...]

>>   +static int crash_exclude_mem_ranges(struct crash_mem *cmem)
>> +{
>> +    int ret, i;
>> +
>> +    /* Exclude crashkernel region */
>> +    ret = crash_exclude_mem_range(cmem, crashk_res.start,
>> crashk_res.end);
>> +    if (ret)
>> +        return ret;
>> +
>> +    if (crashk_low_res.end) {
>> +        ret = crash_exclude_mem_range(cmem, crashk_low_res.start,
>> crashk_low_res.end);
>> +        if (ret)
>> +            return ret;
>> +    }
>>   +    for (i = 0; i < crashk_cma_cnt; ++i) {
>> +        ret = crash_exclude_mem_range(cmem, crashk_cma_ranges[i].start,
>> +                          crashk_cma_ranges[i].end);
>> +        if (ret)
>> +            return ret;
>> +    }
>>   +    return ret;
>> +}
>>     int crash_prepare_elf64_headers(struct crash_mem *mem, int
>> need_kernel_map,
>>                 void **addr, unsigned long *sz)
>> @@ -174,6 +197,11 @@ int crash_prepare_elf64_headers(struct crash_mem
>> *mem, int need_kernel_map,
>>       unsigned int cpu, i;
>>       unsigned long long notes_addr;
>>       unsigned long mstart, mend;
>> +    int ret;
>> +
>> +    ret = crash_exclude_mem_ranges(mem);
> 
> I think the assumption here is that mem should have enough space
> to hold the extra ranges created while excluding crash memory ranges.
> Right now, this is not happening on powerpc for the case I mentioned
> in the above comment.

Yes, as you mentioned above.

> 
> Also, if crashk_cma_cnt changes in the future, or if a new type of
> crash memory is added, then every architecture would need to adjust
> the mem allocation accordingly. Instead, could we handle this in
> generic code rather than in architecture-specific code, so that we
> always ensure mem has enough space?

I agree — hard-coding the worst-case count in every arch is a
maintenance trap.
Let's move the size calculation (and the realloc if needed) into the
generic crash core so that:

- New CMA regions or future crash-memory types are automatically
accounted for;

- Each architecture no longer has to play whack-a-mole with its private
array size.

Thanks for the suggestion.

> 
>> +    if (ret)
>> +        return ret;
>>         /* extra phdr for vmcoreinfo ELF note */
>>       nr_phdr = nr_cpus + 1;
> 

Reply via email to