Re: [PATCH 2/2] powerpc: avoid adjusting memory_limit for capture kernel memory reservation

[Mahesh Jagannath Salgaonkar <mahesh@linux.vnet.ibm.com>]

On 7/22/19 11:19 PM, Michal Suchánek wrote:
> On Fri, 28 Jun 2019 00:51:19 +0530
> Hari Bathini <hbathini@linux.ibm.com> wrote:
> 
>> Currently, if memory_limit is specified and it overlaps with memory to
>> be reserved for capture kernel, memory_limit is adjusted to accommodate
>> capture kernel. With memory reservation for capture kernel moved later
>> (after enforcing memory limit), this adjustment no longer holds water.
>> So, avoid adjusting memory_limit and error out instead.
> 
> Can you split out the memory limit adjustment out of memory reservation
> so it can still be adjusted?

Do you mean adjust the memory limit before we do the actual reservation ?

> 
> Thanks
> 
> Michal
>>
>> Signed-off-by: Hari Bathini <hbathini@linux.ibm.com>
>> ---
>>  arch/powerpc/kernel/fadump.c        |   16 ----------------
>>  arch/powerpc/kernel/machine_kexec.c |   22 +++++++++++-----------
>>  2 files changed, 11 insertions(+), 27 deletions(-)
>>
>> diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
>> index 4eab972..a784695 100644
>> --- a/arch/powerpc/kernel/fadump.c
>> +++ b/arch/powerpc/kernel/fadump.c
>> @@ -476,22 +476,6 @@ int __init fadump_reserve_mem(void)
>>  #endif
>>  	}
>>  
>> -	/*
>> -	 * Calculate the memory boundary.
>> -	 * If memory_limit is less than actual memory boundary then reserve
>> -	 * the memory for fadump beyond the memory_limit and adjust the
>> -	 * memory_limit accordingly, so that the running kernel can run with
>> -	 * specified memory_limit.
>> -	 */
>> -	if (memory_limit && memory_limit < memblock_end_of_DRAM()) {
>> -		size = get_fadump_area_size();
>> -		if ((memory_limit + size) < memblock_end_of_DRAM())
>> -			memory_limit += size;
>> -		else
>> -			memory_limit = memblock_end_of_DRAM();
>> -		printk(KERN_INFO "Adjusted memory_limit for firmware-assisted"
>> -				" dump, now %#016llx\n", memory_limit);
>> -	}
>>  	if (memory_limit)
>>  		memory_boundary = memory_limit;
>>  	else
>> diff --git a/arch/powerpc/kernel/machine_kexec.c b/arch/powerpc/kernel/machine_kexec.c
>> index c4ed328..fc5533b 100644
>> --- a/arch/powerpc/kernel/machine_kexec.c
>> +++ b/arch/powerpc/kernel/machine_kexec.c
>> @@ -125,10 +125,8 @@ void __init reserve_crashkernel(void)
>>  		crashk_res.end = crash_base + crash_size - 1;
>>  	}
>>  
>> -	if (crashk_res.end == crashk_res.start) {
>> -		crashk_res.start = crashk_res.end = 0;
>> -		return;
>> -	}
>> +	if (crashk_res.end == crashk_res.start)
>> +		goto error_out;
>>  
>>  	/* We might have got these values via the command line or the
>>  	 * device tree, either way sanitise them now. */
>> @@ -170,15 +168,13 @@ void __init reserve_crashkernel(void)
>>  	if (overlaps_crashkernel(__pa(_stext), _end - _stext)) {
>>  		printk(KERN_WARNING
>>  			"Crash kernel can not overlap current kernel\n");
>> -		crashk_res.start = crashk_res.end = 0;
>> -		return;
>> +		goto error_out;
>>  	}
>>  
>>  	/* Crash kernel trumps memory limit */
>>  	if (memory_limit && memory_limit <= crashk_res.end) {
>> -		memory_limit = crashk_res.end + 1;
>> -		printk("Adjusted memory limit for crashkernel, now 0x%llx\n",
>> -		       memory_limit);
>> +		pr_err("Crash kernel size can't exceed memory_limit\n");
>> +		goto error_out;
>>  	}
>>  
>>  	printk(KERN_INFO "Reserving %ldMB of memory at %ldMB "
>> @@ -190,9 +186,13 @@ void __init reserve_crashkernel(void)
>>  	if (!memblock_is_region_memory(crashk_res.start, crash_size) ||
>>  	    memblock_reserve(crashk_res.start, crash_size)) {
>>  		pr_err("Failed to reserve memory for crashkernel!\n");
>> -		crashk_res.start = crashk_res.end = 0;
>> -		return;
>> +		goto error_out;
>>  	}
>> +
>> +	return;
>> +error_out:
>> +	crashk_res.start = crashk_res.end = 0;
>> +	return;
>>  }
>>  
>>  int overlaps_crashkernel(unsigned long start, unsigned long size)
>>
>