[PATCH v4 0/2] arm64: kdump: Function supplement and performance optimization

[PATCH v4 0/2] arm64: kdump: Function supplement and performance optimization

[Zhen Lei]

v3 --> v4:
1. Set DEFAULT_CRASH_KERNEL_LOW_SIZE to a fixed 128M.
2. Some lightweight code adjustments based on Catalin Marinas's comments

v2 --> v3:
1. Discard patch 3 in v2, a cleanup patch.

v1 --> v2:
1. Update the commit message of Patch 1, explicitly indicates that "crashkernel=X,high"
   is specified but "crashkernel=Y,low" is not specified.
2. Drop Patch 4-5. Currently, focus on function integrity, performance optimization
   will be considered in later versions.
3. Patch 3 is not mandatory, it's just a cleanup now, although it is a must for patch 4-5.
   But to avoid subsequent duplication of effort, I'm glad it was accepted.

v1:
After the basic functions of "support reserving crashkernel above 4G on arm64
kdump"(see https://lkml.org/lkml/2022/5/6/428) are implemented, we still have
three features to be improved.
1. When crashkernel=X,high is specified but crashkernel=Y,low is not specified,
   the default crash low memory size is provided.
2. For crashkernel=X without '@offset', if the low memory fails to be allocated,
   fall back to reserve region from high memory(above DMA zones).
3. If crashkernel=X,high is used, page mapping is performed only for the crash
   high memory, and block mapping is still used for other linear address spaces.
   Compared to the previous version:
   (1) For crashkernel=X[@offset], the memory above 4G is not changed to block
       mapping, leave it to the next time.
   (2) The implementation method is modified. Now the implementation is simpler
       and clearer.

Zhen Lei (2):
  arm64: kdump: Provide default size when crashkernel=Y,low is not
    specified
  arm64: kdump: Support crashkernel=X fall back to reserve region above
    DMA zones

 .../admin-guide/kernel-parameters.txt         | 15 +++++------
 arch/arm64/mm/init.c                          | 25 ++++++++++++++++---
 2 files changed, 28 insertions(+), 12 deletions(-)

-- 
2.25.1

[PATCH v4 1/2] arm64: kdump: Provide default size when crashkernel=Y,low is not specified

[Zhen Lei]

Try to allocate at least 128 MiB low memory automatically for the case
that crashkernel=,high is explicitly specified, while crashkenrel=,low
is omitted. This allows users to focus more on the high memory
requirements of their business rather than the low memory requirements
of the crash kernel booting.

Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
Acked-by: Baoquan He <bhe@redhat.com>
---
 Documentation/admin-guide/kernel-parameters.txt | 13 +++++--------
 arch/arm64/mm/init.c                            |  8 ++++++--
 2 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 7dea58f4a69cc8c..a7b7147447b8bf8 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -862,26 +862,23 @@
 			available.
 			It will be ignored if crashkernel=X is specified.
 	crashkernel=size[KMG],low
-			[KNL, X86-64] range under 4G. When crashkernel=X,high
+			[KNL, X86-64, ARM64] range under 4G. When crashkernel=X,high
 			is passed, kernel could allocate physical memory region
 			above 4G, that cause second kernel crash on system
 			that require some amount of low memory, e.g. swiotlb
 			requires at least 64M+32K low memory, also enough extra
 			low memory is needed to make sure DMA buffers for 32-bit
 			devices won't run out. Kernel would try to allocate
-			at least 256M below 4G automatically.
+			default	size of memory below 4G automatically. The default
+			size is	platform dependent.
+			  --> x86: max(swiotlb_size_or_default() + 8MiB, 256MiB)
+			  --> arm64: 128MiB
 			This one lets the user specify own low range under 4G
 			for second kernel instead.
 			0: to disable low allocation.
 			It will be ignored when crashkernel=X,high is not used
 			or memory reserved is below 4G.
 
-			[KNL, ARM64] range in low memory.
-			This one lets the user specify a low range in the
-			DMA zone for the crash dump kernel.
-			It will be ignored when crashkernel=X,high is not used
-			or memory reserved is located in the DMA zones.
-
 	cryptomgr.notests
 			[KNL] Disable crypto self-tests
 
diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index 4b4651ee47f271a..ba7227179822d10 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -96,6 +96,8 @@ phys_addr_t __ro_after_init arm64_dma_phys_limit = PHYS_MASK + 1;
 #define CRASH_ADDR_LOW_MAX		arm64_dma_phys_limit
 #define CRASH_ADDR_HIGH_MAX		(PHYS_MASK + 1)
 
+#define DEFAULT_CRASH_KERNEL_LOW_SIZE	(128UL << 20)
+
 static int __init reserve_crashkernel_low(unsigned long long low_size)
 {
 	unsigned long long low_base;
@@ -147,7 +149,9 @@ static void __init reserve_crashkernel(void)
 		 * is not allowed.
 		 */
 		ret = parse_crashkernel_low(cmdline, 0, &crash_low_size, &crash_base);
-		if (ret && (ret != -ENOENT))
+		if (ret == -ENOENT)
+			crash_low_size = DEFAULT_CRASH_KERNEL_LOW_SIZE;
+		else if (ret)
 			return;
 
 		crash_max = CRASH_ADDR_HIGH_MAX;
@@ -170,7 +174,7 @@ static void __init reserve_crashkernel(void)
 		return;
 	}
 
-	if ((crash_base >= CRASH_ADDR_LOW_MAX) &&
+	if ((crash_base > CRASH_ADDR_LOW_MAX - crash_low_size) &&
 	     crash_low_size && reserve_crashkernel_low(crash_low_size)) {
 		memblock_phys_free(crash_base, crash_size);
 		return;
-- 
2.25.1

[PATCH v4 2/2] arm64: kdump: Support crashkernel=X fall back to reserve region above DMA zones

[Zhen Lei]

For crashkernel=X without '@offset', select a region within DMA zones
first, and fall back to reserve region above DMA zones. This allows
users to use the same configuration on multiple platforms.

Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
Acked-by: Baoquan He <bhe@redhat.com>
---
 Documentation/admin-guide/kernel-parameters.txt |  2 +-
 arch/arm64/mm/init.c                            | 17 ++++++++++++++++-
 2 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index a7b7147447b8bf8..ef6d922ed26b9dc 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -842,7 +842,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 ba7227179822d10..58a0bb2c17f18cf 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -132,6 +132,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 = false;
 
 	if (!IS_ENABLED(CONFIG_KEXEC_CORE))
 		return;
@@ -163,12 +164,26 @@ static void __init reserve_crashkernel(void)
 	crash_size = PAGE_ALIGN(crash_size);
 
 	/* User specifies base address explicitly. */
-	if (crash_base)
+	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) {
+		/*
+		 * If the first attempt was for low memory, 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;
-- 
2.25.1

Re: [PATCH v4 1/2] arm64: kdump: Provide default size when crashkernel=Y,low is not specified

[Catalin Marinas]

On Wed, Nov 16, 2022 at 08:10:43PM +0800, Zhen Lei wrote:
> Try to allocate at least 128 MiB low memory automatically for the case
> that crashkernel=,high is explicitly specified, while crashkenrel=,low
> is omitted. This allows users to focus more on the high memory
> requirements of their business rather than the low memory requirements
> of the crash kernel booting.
> 
> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
> Acked-by: Baoquan He <bhe@redhat.com>

Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>

Re: [PATCH v4 2/2] arm64: kdump: Support crashkernel=X fall back to reserve region above DMA zones

[Catalin Marinas]

On Wed, Nov 16, 2022 at 08:10:44PM +0800, Zhen Lei wrote:
> For crashkernel=X without '@offset', select a region within DMA zones
> first, and fall back to reserve region above DMA zones. This allows
> users to use the same configuration on multiple platforms.
> 
> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
> Acked-by: Baoquan He <bhe@redhat.com>

Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>

Re: [PATCH v4 0/2] arm64: kdump: Function supplement and performance optimization

[Will Deacon]

On Wed, 16 Nov 2022 20:10:42 +0800, Zhen Lei wrote:
> v3 --> v4:
> 1. Set DEFAULT_CRASH_KERNEL_LOW_SIZE to a fixed 128M.
> 2. Some lightweight code adjustments based on Catalin Marinas's comments
> 
> v2 --> v3:
> 1. Discard patch 3 in v2, a cleanup patch.
> 
> [...]

Applied to arm64 (for-next/kdump), thanks!

[1/2] arm64: kdump: Provide default size when crashkernel=Y,low is not specified
      https://git.kernel.org/arm64/c/a149cf00b158
[2/2] arm64: kdump: Support crashkernel=X fall back to reserve region above DMA zones
      https://git.kernel.org/arm64/c/a9ae89df7377

Cheers,
-- 
Will

https://fixes.arm64.dev
https://next.arm64.dev
https://will.arm64.dev

Re: [PATCH v4 2/2] arm64: kdump: Support crashkernel=X fall back to reserve region above DMA zones

[Baoquan He]

On 11/16/22 at 08:10pm, Zhen Lei wrote:
> For crashkernel=X without '@offset', select a region within DMA zones
> first, and fall back to reserve region above DMA zones. This allows
> users to use the same configuration on multiple platforms.
> 
> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
> Acked-by: Baoquan He <bhe@redhat.com>
> ---
>  Documentation/admin-guide/kernel-parameters.txt |  2 +-
>  arch/arm64/mm/init.c                            | 17 ++++++++++++++++-
>  2 files changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index a7b7147447b8bf8..ef6d922ed26b9dc 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -842,7 +842,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 ba7227179822d10..58a0bb2c17f18cf 100644
> --- a/arch/arm64/mm/init.c
> +++ b/arch/arm64/mm/init.c
> @@ -132,6 +132,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 = false;
>  
>  	if (!IS_ENABLED(CONFIG_KEXEC_CORE))
>  		return;
> @@ -163,12 +164,26 @@ static void __init reserve_crashkernel(void)
>  	crash_size = PAGE_ALIGN(crash_size);
>  
>  	/* User specifies base address explicitly. */
> -	if (crash_base)
> +	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);

This pachset looks good to me. While we observed a unexpected case,
where a reserved region cross the high and low meomry region. I noticed
Catalin has pointed that out. Even with the suggested code, we could
have the kernel parameters like: crashkernel=512M,high
crashkernel=128M,low, we finally have [4G-126M, 4G+386M], [1G, 1G+128M]
regions in running kernel. This looks strange.

I am wondering if we can specify explicit search_base in
memblock_phys_alloc_range() to avoid above case. Like this,
crashkernel,high region will only exist in high memory, crashkernel,low
region only exists in low memory region. I made a draft patch based on
this patchset to present what the code looks like.

diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index 58a0bb2c17f1..fd9d35e17a62 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -127,7 +127,7 @@ static int __init reserve_crashkernel_low(unsigned long long low_size)
  */
 static void __init reserve_crashkernel(void)
 {
-	unsigned long long crash_base, crash_size;
+	unsigned long long crash_base, crash_size, search_base;
 	unsigned long long crash_low_size = 0;
 	unsigned long long crash_max = CRASH_ADDR_LOW_MAX;
 	char *cmdline = boot_command_line;
@@ -155,6 +155,7 @@ static void __init reserve_crashkernel(void)
 		else if (ret)
 			return;
 
+		search_base = CRASH_ADDR_LOW_MAX;
 		crash_max = CRASH_ADDR_HIGH_MAX;
 	} else if (ret || !crash_size) {
 		/* The specified value is invalid */
@@ -166,12 +167,13 @@ static void __init reserve_crashkernel(void)
 	/* User specifies base address explicitly. */
 	if (crash_base) {
 		fixed_base = true;
+		search_base = crash_base;
 		crash_max = crash_base + crash_size;
 	}
 
 retry:
 	crash_base = memblock_phys_alloc_range(crash_size, CRASH_ALIGN,
-					       crash_base, crash_max);
+					       search_base, crash_max);
 	if (!crash_base) {
 		/*
 		 * If the first attempt was for low memory, fall back to
@@ -180,6 +182,7 @@ static void __init reserve_crashkernel(void)
 		 */
 		if (!fixed_base && (crash_max == CRASH_ADDR_LOW_MAX)) {
 			crash_max = CRASH_ADDR_HIGH_MAX;
+			search_base = CRASH_ADDR_LOW_MAX;
 			crash_low_size = DEFAULT_CRASH_KERNEL_LOW_SIZE;
 			goto retry;
 		}
@@ -189,7 +192,7 @@ static void __init reserve_crashkernel(void)
 		return;
 	}
 
-	if ((crash_base > CRASH_ADDR_LOW_MAX - crash_low_size) &&
+	if ((crash_base > CRASH_ADDR_LOW_MAX) &&
 	     crash_low_size && reserve_crashkernel_low(crash_low_size)) {
 		memblock_phys_free(crash_base, crash_size);
 		return;

Re: [PATCH v4 2/2] arm64: kdump: Support crashkernel=X fall back to reserve region above DMA zones

[Leizhen (ThunderTown)]

On 2022/11/21 16:31, Baoquan He wrote:
> On 11/16/22 at 08:10pm, Zhen Lei wrote:
>> For crashkernel=X without '@offset', select a region within DMA zones
>> first, and fall back to reserve region above DMA zones. This allows
>> users to use the same configuration on multiple platforms.
>>
>> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
>> Acked-by: Baoquan He <bhe@redhat.com>
>> ---
>>  Documentation/admin-guide/kernel-parameters.txt |  2 +-
>>  arch/arm64/mm/init.c                            | 17 ++++++++++++++++-
>>  2 files changed, 17 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
>> index a7b7147447b8bf8..ef6d922ed26b9dc 100644
>> --- a/Documentation/admin-guide/kernel-parameters.txt
>> +++ b/Documentation/admin-guide/kernel-parameters.txt
>> @@ -842,7 +842,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 ba7227179822d10..58a0bb2c17f18cf 100644
>> --- a/arch/arm64/mm/init.c
>> +++ b/arch/arm64/mm/init.c
>> @@ -132,6 +132,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 = false;
>>  
>>  	if (!IS_ENABLED(CONFIG_KEXEC_CORE))
>>  		return;
>> @@ -163,12 +164,26 @@ static void __init reserve_crashkernel(void)
>>  	crash_size = PAGE_ALIGN(crash_size);
>>  
>>  	/* User specifies base address explicitly. */
>> -	if (crash_base)
>> +	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);
> 
> This pachset looks good to me. While we observed a unexpected case,
> where a reserved region cross the high and low meomry region. I noticed
> Catalin has pointed that out. Even with the suggested code, we could
> have the kernel parameters like: crashkernel=512M,high
> crashkernel=128M,low, we finally have [4G-126M, 4G+386M], [1G, 1G+128M]
> regions in running kernel. This looks strange.

Is high-end memory fragmented? Add boot option memblock=debug and watch?

> 
> I am wondering if we can specify explicit search_base in
> memblock_phys_alloc_range() to avoid above case. Like this,
> crashkernel,high region will only exist in high memory, crashkernel,low
> region only exists in low memory region. I made a draft patch based on
> this patchset to present what the code looks like.

Looks good to me.

> 
> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> index 58a0bb2c17f1..fd9d35e17a62 100644
> --- a/arch/arm64/mm/init.c
> +++ b/arch/arm64/mm/init.c
> @@ -127,7 +127,7 @@ static int __init reserve_crashkernel_low(unsigned long long low_size)
>   */
>  static void __init reserve_crashkernel(void)
>  {
> -	unsigned long long crash_base, crash_size;
> +	unsigned long long crash_base, crash_size, search_base;

search_base needs to be initialized to 0.

>  	unsigned long long crash_low_size = 0;
>  	unsigned long long crash_max = CRASH_ADDR_LOW_MAX;
>  	char *cmdline = boot_command_line;
> @@ -155,6 +155,7 @@ static void __init reserve_crashkernel(void)
>  		else if (ret)
>  			return;
>  
> +		search_base = CRASH_ADDR_LOW_MAX;
>  		crash_max = CRASH_ADDR_HIGH_MAX;
>  	} else if (ret || !crash_size) {
>  		/* The specified value is invalid */
> @@ -166,12 +167,13 @@ static void __init reserve_crashkernel(void)
>  	/* User specifies base address explicitly. */
>  	if (crash_base) {
>  		fixed_base = true;
> +		search_base = crash_base;
>  		crash_max = crash_base + crash_size;
>  	}
>  
>  retry:
>  	crash_base = memblock_phys_alloc_range(crash_size, CRASH_ALIGN,
> -					       crash_base, crash_max);
> +					       search_base, crash_max);
>  	if (!crash_base) {
>  		/*
>  		 * If the first attempt was for low memory, fall back to
> @@ -180,6 +182,7 @@ static void __init reserve_crashkernel(void)
>  		 */
>  		if (!fixed_base && (crash_max == CRASH_ADDR_LOW_MAX)) {
>  			crash_max = CRASH_ADDR_HIGH_MAX;
> +			search_base = CRASH_ADDR_LOW_MAX;
>  			crash_low_size = DEFAULT_CRASH_KERNEL_LOW_SIZE;
>  			goto retry;
>  		}
> @@ -189,7 +192,7 @@ static void __init reserve_crashkernel(void)
>  		return;
>  	}
>  
> -	if ((crash_base > CRASH_ADDR_LOW_MAX - crash_low_size) &&
> +	if ((crash_base > CRASH_ADDR_LOW_MAX) &&
>  	     crash_low_size && reserve_crashkernel_low(crash_low_size)) {
>  		memblock_phys_free(crash_base, crash_size);
>  		return;
> 
> .
> 

-- 
Regards,
  Zhen Lei