Re: [PATCH v5] powerpc/kdump: Add support for crashkernel CMA reservation

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

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

> On 04/11/25 15:48, Ritesh Harjani (IBM) wrote:
>> Sourabh Jain <sourabhjain@linux.ibm.com> writes:
>>
>>
>>> I would like to keep kdump_cma_reserve() as is it because of two reasons:
>>>
>>> - It keeps setup_arch() free from kdump #ifdefs
>> Not really.
>>
>> Instead of kdump_cma_reserve(crashk_cma_size), one could call
>>
>> reserve_crashkernel_cma(crashk_cma_size) directly in setup_arch().
>
>
> reserve_crashkernel_cma() is not available unless the kernel is built 
> with CONFIG_CRASH_RESERVE.
> So, wouldn’t calling reserve_crashkernel_cma() directly from 
> setup_arch() lead to a build failure? Or
> am I missing something?
>

OOps.. I was assuming the #else CRASHKERNEL_CMA definition should get
called, but all of that logic itself is protected in
CONFIG_CRASH_RESERVE :(

Right to avoid #ifdef or IS_ENABLED in setup_arch..
it's better to have kdump_cma_reserve()

Thanks for pointing that out.


<snip>
obj-$(CONFIG_CRASH_RESERVE) += crash_reserve.o

kernel/crash_reserve.c

#ifdef CRASHKERNEL_CMA
int crashk_cma_cnt;
void __init reserve_crashkernel_cma(unsigned long long cma_size)
{
        ... 
}
#else /* CRASHKERNEL_CMA */
void __init reserve_crashkernel_cma(unsigned long long cma_size)
{
	if (cma_size)
		pr_warn("crashkernel CMA reservation not supported\n");
}
#endif

-ritesh


>>
>>> - In case if we want to add some condition on this reservation it would
>>> straight forward.
>>>
>> Make sense.
>>
>>> So lets keep kdump_cma_reserve as is, unless you have strong opinion on
>>> not to.
>>>
>> No strong opinion, as I said it was a minor nit. Feel free to keep the
>> function kdump_cma_reserve() as is then.
>>
>> -ritesh
>>