On 2023/3/31 7:32, Baoquan He wrote:
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@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@offset" was passed
but reserve failed, should try again to allocate in high memory, regardless
the specified size@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@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.
Make sense, thanks.
Actually, in my previous tests, the result in this case is the same
as expectation, i.e. when allocating "crashkernel=size@offset" failed
on low memory, it would retry but return on failure. Since the
search_end is assigned with offset + size, which is lower than DMA32
limit, the second allocation is definitely invalid.
But for sure, to make the code easy to read and eradicate other
possible corner cases, I will check if !fixed_base first on retry.
