| Message ID | 20220711090319.1604-3-thunder.leizhen@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64: kdump: Function supplement and performance optimization | expand |
On Mon, Jul 11, 2022 at 05:03:19PM +0800, Zhen Lei wrote: > diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c > index 5390f361208ccf7..8539598f9e58b4d 100644 > --- a/arch/arm64/mm/init.c > +++ b/arch/arm64/mm/init.c > @@ -138,6 +138,7 @@ static void __init reserve_crashkernel(void) > unsigned long long crash_max = CRASH_ADDR_LOW_MAX; > char *cmdline = boot_command_line; > int ret; > + bool fixed_base; > > if (!IS_ENABLED(CONFIG_KEXEC_CORE)) > return; > @@ -166,15 +167,28 @@ static void __init reserve_crashkernel(void) > return; > } > > + fixed_base = !!crash_base; > crash_size = PAGE_ALIGN(crash_size); > > /* User specifies base address explicitly. */ > - if (crash_base) > + if (fixed_base) > crash_max = crash_base + crash_size; Not a fan of '!!', it is converted automatically. If you don't like the conversion, just initialise fixed_base to false and here: if (crash_base) { fixed_base = true; crash_max = crash_base + crash_size; } > +retry: > crash_base = memblock_phys_alloc_range(crash_size, CRASH_ALIGN, > crash_base, crash_max); > if (!crash_base) { > + /* > + * Attempt to fully allocate low memory failed, fall back > + * to high memory, the minimum required low memory will be > + * reserved later. > + */ I'm not sure this comment makes sense. If !crash_base, it doesn't mean the kernel failed to fully allocate low memory. crash_max here could be CRASH_ADDR_HIGH_MAX if crashkerne=X,high was specified. Maybe says something like "If the first attempt was for low memory, fall back to high ..." > + if (!fixed_base && (crash_max == CRASH_ADDR_LOW_MAX)) { > + crash_max = CRASH_ADDR_HIGH_MAX; > + crash_low_size = DEFAULT_CRASH_KERNEL_LOW_SIZE; > + goto retry; > + } The retry logic looks fine, it only happens once as crash_max is updated.
On 2022/11/8 1:13, Catalin Marinas wrote: > On Mon, Jul 11, 2022 at 05:03:19PM +0800, Zhen Lei wrote: >> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c >> index 5390f361208ccf7..8539598f9e58b4d 100644 >> --- a/arch/arm64/mm/init.c >> +++ b/arch/arm64/mm/init.c >> @@ -138,6 +138,7 @@ static void __init reserve_crashkernel(void) >> unsigned long long crash_max = CRASH_ADDR_LOW_MAX; >> char *cmdline = boot_command_line; >> int ret; >> + bool fixed_base; >> >> if (!IS_ENABLED(CONFIG_KEXEC_CORE)) >> return; >> @@ -166,15 +167,28 @@ static void __init reserve_crashkernel(void) >> return; >> } >> >> + fixed_base = !!crash_base; >> crash_size = PAGE_ALIGN(crash_size); >> >> /* User specifies base address explicitly. */ >> - if (crash_base) >> + if (fixed_base) >> crash_max = crash_base + crash_size; > > Not a fan of '!!', it is converted automatically. If you don't like the > conversion, just initialise fixed_base to false and here: > > if (crash_base) { > fixed_base = true; OK, This way would be better. > crash_max = crash_base + crash_size; > } > >> +retry: >> crash_base = memblock_phys_alloc_range(crash_size, CRASH_ALIGN, >> crash_base, crash_max); >> if (!crash_base) { >> + /* >> + * Attempt to fully allocate low memory failed, fall back >> + * to high memory, the minimum required low memory will be >> + * reserved later. >> + */ > > I'm not sure this comment makes sense. If !crash_base, it doesn't mean > the kernel failed to fully allocate low memory. crash_max here could be > CRASH_ADDR_HIGH_MAX if crashkerne=X,high was specified. Maybe says > something like "If the first attempt was for low memory, fall back to > high ..." This description is accurate. I'll update. Thanks. > >> + if (!fixed_base && (crash_max == CRASH_ADDR_LOW_MAX)) { >> + crash_max = CRASH_ADDR_HIGH_MAX; >> + crash_low_size = DEFAULT_CRASH_KERNEL_LOW_SIZE; >> + goto retry; >> + } > > The retry logic looks fine, it only happens once as crash_max is > updated. >
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt index 65a2c3a22a4b57d..cb6a346419a1fe0 100644 --- a/Documentation/admin-guide/kernel-parameters.txt +++ b/Documentation/admin-guide/kernel-parameters.txt @@ -823,7 +823,7 @@ memory region [offset, offset + size] for that kernel image. If '@offset' is omitted, then a suitable offset is selected automatically. - [KNL, X86-64] Select a region under 4G first, and + [KNL, X86-64, ARM64] Select a region under 4G first, and fall back to reserve region above 4G when '@offset' hasn't been specified. See Documentation/admin-guide/kdump/kdump.rst for further details. diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c index 5390f361208ccf7..8539598f9e58b4d 100644 --- a/arch/arm64/mm/init.c +++ b/arch/arm64/mm/init.c @@ -138,6 +138,7 @@ static void __init reserve_crashkernel(void) unsigned long long crash_max = CRASH_ADDR_LOW_MAX; char *cmdline = boot_command_line; int ret; + bool fixed_base; if (!IS_ENABLED(CONFIG_KEXEC_CORE)) return; @@ -166,15 +167,28 @@ static void __init reserve_crashkernel(void) return; } + fixed_base = !!crash_base; crash_size = PAGE_ALIGN(crash_size); /* User specifies base address explicitly. */ - if (crash_base) + if (fixed_base) crash_max = crash_base + crash_size; +retry: crash_base = memblock_phys_alloc_range(crash_size, CRASH_ALIGN, crash_base, crash_max); if (!crash_base) { + /* + * Attempt to fully allocate low memory failed, fall back + * to high memory, the minimum required low memory will be + * reserved later. + */ + if (!fixed_base && (crash_max == CRASH_ADDR_LOW_MAX)) { + crash_max = CRASH_ADDR_HIGH_MAX; + crash_low_size = DEFAULT_CRASH_KERNEL_LOW_SIZE; + goto retry; + } + pr_warn("cannot allocate crashkernel (size:0x%llx)\n", crash_size); return;