Re: [PATCH] mips: kernel: setup: fix crash kernel resource allocation

[Ivan Khoronzhuk <ivan.khoronzhuk@gmail.com>]

On Sun, Feb 07, 2021 at 11:18:42AM +0200, Mike Rapoport wrote:
>On Sat, Feb 06, 2021 at 12:59:40PM +0000, Ivan Khoronzhuk wrote:
>> In order to avoid crash kernel corruption, its memory is reserved
>> early in memblock and as result, in time when resources are inited
>> it's not present in memblock.memory, so crash kernel memory is out
>> of ranges listed with for_each_mem_range(). To avoid it and still
>> keep memory reserved lets reseve it out of loop by inserting it in
>> iomem_resource.
>
>Unless I misread the code, the crash kernel memory is actually allocated
>from memblock (memblock_find_in_range + memblock_reserve), but for some
>reason memblock_reserve(<crash kernel>) is called outside
>mips_parse_crashkernel(). So the crash kernel memory is surely in both
>memblock.memory and memblock.reserved and it will be covered by
>for_each_mem_range().
>
>The mips_parse_crashkernel() function and the following reservation of
>crash kernel memory should be merged, IMO, and this can be further
>simplified with memblock_alloc() helpers.
>
>Is there a particular issue you are trying to fix?

Yes, sorry, according to local code, seems like memory was not added
(was reverted memblock_find_in_range for some other reson while
porting), so I had only reserve. The issue is in another place,
so ignoe this patch, for now.

>
>> Fixes: a94e4f24ec83 ("MIPS: init: Drop boot_mem_map")
>> Signed-off-by: Ivan Khoronzhuk <ikhoronz@cisco.com>
>> ---
>> Based on linux-next/master
>>
>>  arch/mips/kernel/setup.c | 8 +++++---
>>  1 file changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/mips/kernel/setup.c b/arch/mips/kernel/setup.c
>> index 3785c72bc3bc..25e376ef2f2a 100644
>> --- a/arch/mips/kernel/setup.c
>> +++ b/arch/mips/kernel/setup.c
>> @@ -473,14 +473,15 @@ static void __init mips_parse_crashkernel(void)
>>  	crashk_res.end	 = crash_base + crash_size - 1;
>>  }
>>
>> -static void __init request_crashkernel(struct resource *res)
>> +static void __init request_crashkernel(void)
>>  {
>>  	int ret;
>>
>>  	if (crashk_res.start == crashk_res.end)
>>  		return;
>>
>> -	ret = request_resource(res, &crashk_res);
>> +	/* The crashk resource shoud be located in normal mem */
>> +	ret = insert_resource(&iomem_resource, &crashk_res);
>>  	if (!ret)
>>  		pr_info("Reserving %ldMB of memory at %ldMB for crashkernel\n",
>>  			(unsigned long)(resource_size(&crashk_res) >> 20),
>> @@ -734,8 +735,9 @@ static void __init resource_init(void)
>>  		request_resource(res, &code_resource);
>>  		request_resource(res, &data_resource);
>>  		request_resource(res, &bss_resource);
>> -		request_crashkernel(res);
>>  	}
>> +
>> +	request_crashkernel();
>>  }
>>
>>  #ifdef CONFIG_SMP
>> --
>> 2.23.1
>>
>
>-- 
>Sincerely yours,
>Mike.

-- 
Regards,
Ivan Khoronzhuk