[PATCH -next v2 1/2] riscv: kdump: Implement crashkernel=X,[high,low]

[Baoquan He]

On 03/30/23 at 09:40pm, chenjiahao (C) wrote:
......
> Agreed, I will clean this up later in next version.
> > > +		if (ret || !crash_size)
> > > +			return;
> > > +
> > > +		/*
> > > +		 * crashkernel=Y,low is valid only when crashkernel=X,high
> > > +		 * is passed and high memory is reserved successful.
> > > +		 */
> > > +		ret = parse_crashkernel_low(boot_command_line, 0, &crash_low_size, &crash_base);
> > > +		if (ret == -ENOENT)
> > > +			crash_low_size = DEFAULT_CRASH_KERNEL_LOW_SIZE;
> > > +		else if (ret)
> > > +			return;
> > > +
> > > +		search_start = dma32_phys_limit;
> > > +	} else if (ret || !crash_size) {
> > > +		/* Invalid argument value specified */
> > >   		return;
> > > +	}
> > >   	crash_size = PAGE_ALIGN(crash_size);
> > > @@ -1201,16 +1246,26 @@ static void __init reserve_crashkernel(void)
> > >   	 */
> > >   	crash_base = memblock_phys_alloc_range(crash_size, PMD_SIZE,
> > >   					       search_start,
> > > -					       min(search_end, (unsigned long) SZ_4G));
> > > +					       min(search_end, (unsigned long)dma32_phys_limit));
> > >   	if (crash_base == 0) {
> > The above conditional check isn't right. If crashkernel=size at offset
> > specified, the reservation failure won't trigger retry. This seems to be
> > originally introduced by old commit, while this need be fixed firstly.
> 
> Just a little curious about the rule to cope with this specific case. If
> "crashkernel=size at offset" was passed
> 
> but reserve failed, should try again to allocate in high memory, regardless
> the specified size at offset,
> 
> or just throw a warning and return? Since I noticed the current logic here
> on Arm64 is to check if !fixed_base first

Yeah, we need mark the "crashkernel=size at offset" case and avoid to
retry. Because you won't succeed if  memblock has already failed to
reserve an unavailable memory region, retry is meaningless. This has
been done in x86, arm64.