LKML Archive mirror
 help / color / mirror / Atom feed
* [PATCH v6 0/2] arm64: kdump: simplify the reservation behaviour of crashkernel=,high
@ 2023-05-15  6:02 Baoquan He
  2023-05-15  6:02 ` [PATCH v6 1/2] " Baoquan He
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Baoquan He @ 2023-05-15  6:02 UTC (permalink / raw)
  To: linux-kernel
  Cc: catalin.marinas, will, horms, thunder.leizhen, John.p.donnelly,
	kexec, linux-arm-kernel, Baoquan He

In v5 patch, Catalin helped review and acked the patch. However, an
uninitialized local varilable is warned out by static checker when Will
tried to merge the patch. And Will complained the code flow in
reserve_crashkernel() is hard to follow, required to refactor. While
when I tried to do the refactory, I feel it's not easy, the existing
several cases causes that.

To make the code easier understand, I try my best to compose a document
to introduce the background, concept and implementation strategies of 
crashkernel reservation. Hope it can help people to understand the code
flow a little more easily.

[PATCH v5] arm64: kdump: simplify the reservation behaviour of crashkernel=,high
https://lore.kernel.org/all/20230407022419.19412-1-bhe@redhat.com/T/#u

v5->v6:
- Fix the warning reported by static checker about "uninitialized symbol
'search_base'".
- Add a document Documentation/arm64/kdump.rst to explain how to reserve
  crashkernel.

Baoquan He (2):
  arm64: kdump: simplify the reservation behaviour of crashkernel=,high
  Documentation: add kdump.rst to present crashkernel reservation on
    arm64

 Documentation/arm64/kdump.rst | 103 ++++++++++++++++++++++++++++++++++
 arch/arm64/mm/init.c          |  44 +++++++++++----
 2 files changed, 137 insertions(+), 10 deletions(-)
 create mode 100644 Documentation/arm64/kdump.rst

-- 
2.34.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v6 1/2] arm64: kdump: simplify the reservation behaviour of crashkernel=,high
  2023-05-15  6:02 [PATCH v6 0/2] arm64: kdump: simplify the reservation behaviour of crashkernel=,high Baoquan He
@ 2023-05-15  6:02 ` Baoquan He
  2023-05-15  9:54   ` [PATCH v6 RESEND " Baoquan He
  2023-05-15 10:07   ` [PATCH v6 " Baoquan He
  2023-05-15  6:02 ` [PATCH v6 2/2] Documentation: add kdump.rst to present crashkernel reservation on arm64 Baoquan He
  2023-06-09 19:30 ` [PATCH v6 0/2] arm64: kdump: simplify the reservation behaviour of crashkernel=,high Catalin Marinas
  2 siblings, 2 replies; 12+ messages in thread
From: Baoquan He @ 2023-05-15  6:02 UTC (permalink / raw)
  To: linux-kernel
  Cc: catalin.marinas, will, horms, thunder.leizhen, John.p.donnelly,
	kexec, linux-arm-kernel, Baoquan He

On arm64, reservation for 'crashkernel=xM,high' is taken by searching for
suitable memory region top down. If the 'xM' of crashkernel high memory
is reserved from high memory successfully, it will try to reserve
crashkernel low memory later accoringly. Otherwise, it will try to search
low memory area for the 'xM' suitable region. Please see the details in
Documentation/admin-guide/kernel-parameters.txt.

While we observed an unexpected case where a reserved region crosses the
high and low meomry boundary. E.g on a system with 4G as low memory end,
user added the kernel parameters like: 'crashkernel=512M,high', it could
finally have [4G-126M, 4G+386M], [1G, 1G+128M] regions in running kernel.
The crashkernel high region crossing low and high memory boudary will bring
issues:

1) For crashkernel=x,high, if getting crashkernel high region across
low and high memory boundary, then user will see two memory regions in
low memory, and one memory region in high memory. The two crashkernel
low memory regions are confusing as shown in above example.

2) If people explicityly specify "crashkernel=x,high crashkernel=y,low"
and y <= 128M, when crashkernel high region crosses low and high memory
boundary and the part of crashkernel high reservation below boundary is
bigger than y, the expected crahskernel low reservation will be skipped.
But the expected crashkernel high reservation is shrank and could not
satisfy user space requirement.

3) The crossing boundary behaviour of crahskernel high reservation is
different than x86 arch. On x86_64, the low memory end is 4G fixedly,
and the memory near 4G is reserved by system, e.g for mapping firmware,
pci mapping, so the crashkernel reservation crossing boundary never happens.
From distros point of view, this brings inconsistency and confusion. Users
need to dig into x86 and arm64 system details to find out why.

For kernel itself, the impact of issue 3) could be slight. While issue
1) and 2) cause actual impact because it brings obscure semantics and
behaviour to crashkernel=,high reservation.

Here, for crashkernel=xM,high, search the high memory for the suitable
region only in high memory. If failed, try reserving the suitable
region only in low memory. Like this, the crashkernel high region will
only exist in high memory, and crashkernel low region only exists in low
memory. The reservation behaviour for crashkernel=,high is clearer and
simpler.

Note: RPi4 has different zone ranges than normal memory. Its DMA zone is
0~1G, and DMA32 zone is 1G~4G if CONFIG_ZONE_DMA|DMA32 are enabled by
default. The low memory end is 1G in order to validate all devices, high
memory starts at 1G memory. However, for being consistent with normla
arm64 system, its low memory end is still 1G, while reserving crashkernel
high memory from 4G if crashkernel=size,high specified. This will remove
confusion.

With above change applied, summary of arm64 crashkernel reservation range:
1)
RPi4(zone DMA:0~1G; DMA32:1G~4G):
 crashkernel=size
  0~1G: low memory | 1G~top: high memory

 crashkernel=size,high
  0~1G: low memory | 4G~top: high memory

2)
Other normal system:
 crashkernel=size
 crashkernel=size,high
  0~4G: low memory | 4G~top: high memory

3)
Systems w/o zone DMA|DMA32
 crashkernel=size
 crashkernel=size,high
  0~top: low memory

Signed-off-by: Baoquan He <bhe@redhat.com>

arm64: kdump: fix warning reported by static checker
Signed-off-by: Baoquan He <bhe@redhat.com>
---
 arch/arm64/mm/init.c | 44 ++++++++++++++++++++++++++++++++++----------
 1 file changed, 34 insertions(+), 10 deletions(-)

diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index 66e70ca47680..c28c2c8483cc 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -69,6 +69,7 @@ phys_addr_t __ro_after_init arm64_dma_phys_limit;
 
 #define CRASH_ADDR_LOW_MAX		arm64_dma_phys_limit
 #define CRASH_ADDR_HIGH_MAX		(PHYS_MASK + 1)
+#define CRASH_HIGH_SEARCH_BASE		SZ_4G
 
 #define DEFAULT_CRASH_KERNEL_LOW_SIZE	(128UL << 20)
 
@@ -101,12 +102,13 @@ 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_low_size = 0;
+	unsigned long long crash_low_size = 0, search_base = 0;
 	unsigned long long crash_max = CRASH_ADDR_LOW_MAX;
+	unsigned long long crash_base, crash_size;
 	char *cmdline = boot_command_line;
-	int ret;
 	bool fixed_base = false;
+	bool high = false;
+	int ret;
 
 	if (!IS_ENABLED(CONFIG_KEXEC_CORE))
 		return;
@@ -129,7 +131,9 @@ static void __init reserve_crashkernel(void)
 		else if (ret)
 			return;
 
+		search_base = CRASH_HIGH_SEARCH_BASE;
 		crash_max = CRASH_ADDR_HIGH_MAX;
+		high = true;
 	} else if (ret || !crash_size) {
 		/* The specified value is invalid */
 		return;
@@ -140,31 +144,51 @@ 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
-		 * high memory, the minimum required low memory will be
-		 * reserved later.
+		 * For crashkernel=size[KMG]@offset[KMG], print out failure
+		 * message if can't reserve the specified region.
 		 */
-		if (!fixed_base && (crash_max == CRASH_ADDR_LOW_MAX)) {
+		if (fixed_base) {
+			pr_warn("crashkernel reservation failed - memory is in use.\n");
+			return;
+		}
+
+		/*
+		 * For crashkernel=size[KMG], if the first attempt was for
+		 * low memory, fall back to high memory, the minimum required
+		 * low memory will be reserved later.
+		 */
+		if (!high && 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;
 		}
 
+		/*
+		 * For crashkernel=size[KMG],high, if the first attempt was
+		 * for high memory, fall back to low memory.
+		 */
+		if (high && crash_max == CRASH_ADDR_HIGH_MAX) {
+			crash_max = CRASH_ADDR_LOW_MAX;
+			search_base = 0;
+			goto retry;
+		}
 		pr_warn("cannot allocate crashkernel (size:0x%llx)\n",
 			crash_size);
 		return;
 	}
 
-	if ((crash_base > CRASH_ADDR_LOW_MAX - crash_low_size) &&
-	     crash_low_size && reserve_crashkernel_low(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;
 	}
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v6 2/2] Documentation: add kdump.rst to present crashkernel reservation on arm64
  2023-05-15  6:02 [PATCH v6 0/2] arm64: kdump: simplify the reservation behaviour of crashkernel=,high Baoquan He
  2023-05-15  6:02 ` [PATCH v6 1/2] " Baoquan He
@ 2023-05-15  6:02 ` Baoquan He
  2023-06-06 12:08   ` Leizhen (ThunderTown)
  2023-06-09 19:30 ` [PATCH v6 0/2] arm64: kdump: simplify the reservation behaviour of crashkernel=,high Catalin Marinas
  2 siblings, 1 reply; 12+ messages in thread
From: Baoquan He @ 2023-05-15  6:02 UTC (permalink / raw)
  To: linux-kernel
  Cc: catalin.marinas, will, horms, thunder.leizhen, John.p.donnelly,
	kexec, linux-arm-kernel, Baoquan He

People complained the crashkernel reservation code flow is hard to
follow, so add this document to explain the background, concepts and
implementation of crashkernel reservation on arm64. Hope this can help
people to understand it more easily.

Signed-off-by: Baoquan He <bhe@redhat.com>
---
 Documentation/arm64/kdump.rst | 103 ++++++++++++++++++++++++++++++++++
 1 file changed, 103 insertions(+)
 create mode 100644 Documentation/arm64/kdump.rst

diff --git a/Documentation/arm64/kdump.rst b/Documentation/arm64/kdump.rst
new file mode 100644
index 000000000000..78b22017c490
--- /dev/null
+++ b/Documentation/arm64/kdump.rst
@@ -0,0 +1,103 @@
+=======================================
+crashkernel memory reservation on arm64
+=======================================
+
+Author: Baoquan He <bhe@redhat.com>
+
+Kdump mechanism is utilized to capture corrupted kernel's vmcore so
+that people can analyze it to get the root cause of corruption. In
+order to do that, a preliminarily reserved memory is needed to load
+in kdump kernel, and switch to kdump kernel to boot up and run if
+corruption happened.
+
+That reserved memory for kdump is adapted to be able to minimally
+accommodate kdump kernel to boot and run, and user space programs
+running to do the vmcore collecting.
+
+Kernel parameter
+================
+Through kernel parameter like below, memory can be reserved
+accordingly during early stage of 1st kernel's bootup so that
+continuous large chunk of memomy can be found and reserved. Meanwhile,
+the need of low memory need be considered if crashkernel is reserved
+in high memory area.
+
+- crashkernel=size@offset
+- crashkernel=size
+- crashkernel=size,high crashkernel=size,low
+
+Low memory and high memory
+===============
+What is low memory and high memory? In kdump reservation, low memory
+means the memory area under a specific limitation, and it's usually
+decided by the lowest addressing bits of PCI devices which kdump kernel
+need rely on to boot up and collect vmcore successfully. Those devices
+not related to vmcore dumping can be ignored, e.g on x86, those i2c may
+only be able to access 24bits addressing area, but kdump kernel still
+take 4G as the limitation because all known devices that kdump kernel
+cares about have 32bits addressing ability. On arm64, the low memory
+upper boundary is not fixed, it's 1G on RPi4 platform, while 4G on normal
+arm64 system. On the special system with CONFIG_ZONE_DMA|DMA32 disabled,
+the whole system RAM is low memory. Except of low memory, all the rest
+of system RAM is high memory which kernel and user space programs can
+require to allocate and use.
+
+Implementation
+==============
+1)crashkernel=size@offset
+-------------------------
+crashkernel memory must be reserved at the user specified region, otherwise
+fail if already occupied.
+
+
+2) crashkernel=size
+-------------------
+crashkernel memory region will be reserved in any available position
+according to searching order.
+
+Firstly, it searches the low memory area for an available region with specified
+size.
+
+Secondly, if searching low memory failed, fallback to search the high memory
+area with the specified size. Meanwhile, if the reservation in high memory
+succeeds, a default reservation in low memory will be done, the current default
+value is 128M which is satisfying the low memory needs, e.g pci device driver
+initialization.
+
+If both the above searching failed, the reservation will fail finally.
+
+Note: crashkernel=size is recommended option among crashkernel kernel
+parameters. With it, user doesn't need to know much about system memory
+information, just need to specify whatever memory kdump kernel needs to
+make vmcore dumping succeed.
+
+3) crashkernel=size,high crashkernel=size,low
+--------------------------------------------
+crashkernel=size,high is an important supplement to crashkernel=size. It
+allows user to precisely specify how much memory need be allocated from
+high memory, and how much memory is needed from low memory. On system
+with large memory, low memory is small and precious since some kernel
+feature and many devices can only request memory from the area, while
+requiring a large chunk of continuous memory from high memory area doesn't
+matter much and can satisfy most of kernel and almost all user space
+programs' requirement. In such case, only a small part of necessary memory
+from low memory area can satisfy needs. With it, the 1st kernel's normal
+running won't be impacted because of limited low memory resource.
+
+To reserve memory for crashkernel=size,high, firstly, searching is tried in
+high memory region. If reservation succeeds, low memory reservaton will be
+done subsequently.
+
+Secondly, if reservation in high memory failed, fallback to search the
+low memory with the specified size in crsahkernel=,high. If succeeds,
+everything is fine since no low memory is needed.
+
+Notes:
+- If crashkernel=,low is not specified, the default low memory reservation
+  will be done automically.
+
+- if crashkernel=0,low is specified, means that low memory reservation is
+  ommited intentionally.
+
+3)
+
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v6 RESEND 1/2] arm64: kdump: simplify the reservation behaviour of crashkernel=,high
  2023-05-15  6:02 ` [PATCH v6 1/2] " Baoquan He
@ 2023-05-15  9:54   ` Baoquan He
  2023-06-06 11:53     ` Leizhen (ThunderTown)
  2023-05-15 10:07   ` [PATCH v6 " Baoquan He
  1 sibling, 1 reply; 12+ messages in thread
From: Baoquan He @ 2023-05-15  9:54 UTC (permalink / raw)
  To: linux-kernel
  Cc: catalin.marinas, will, horms, thunder.leizhen, John.p.donnelly,
	kexec, linux-arm-kernel

On arm64, reservation for 'crashkernel=xM,high' is taken by searching for
suitable memory region top down. If the 'xM' of crashkernel high memory
is reserved from high memory successfully, it will try to reserve
crashkernel low memory later accoringly. Otherwise, it will try to search
low memory area for the 'xM' suitable region. Please see the details in
Documentation/admin-guide/kernel-parameters.txt.

While we observed an unexpected case where a reserved region crosses the
high and low meomry boundary. E.g on a system with 4G as low memory end,
user added the kernel parameters like: 'crashkernel=512M,high', it could
finally have [4G-126M, 4G+386M], [1G, 1G+128M] regions in running kernel.
The crashkernel high region crossing low and high memory boudary will bring
issues:

1) For crashkernel=x,high, if getting crashkernel high region across
low and high memory boundary, then user will see two memory regions in
low memory, and one memory region in high memory. The two crashkernel
low memory regions are confusing as shown in above example.

2) If people explicityly specify "crashkernel=x,high crashkernel=y,low"
and y <= 128M, when crashkernel high region crosses low and high memory
boundary and the part of crashkernel high reservation below boundary is
bigger than y, the expected crahskernel low reservation will be skipped.
But the expected crashkernel high reservation is shrank and could not
satisfy user space requirement.

3) The crossing boundary behaviour of crahskernel high reservation is
different than x86 arch. On x86_64, the low memory end is 4G fixedly,
and the memory near 4G is reserved by system, e.g for mapping firmware,
pci mapping, so the crashkernel reservation crossing boundary never happens.
From distros point of view, this brings inconsistency and confusion. Users
need to dig into x86 and arm64 system details to find out why.

For kernel itself, the impact of issue 3) could be slight. While issue
1) and 2) cause actual impact because it brings obscure semantics and
behaviour to crashkernel=,high reservation.

Here, for crashkernel=xM,high, search the high memory for the suitable
region only in high memory. If failed, try reserving the suitable
region only in low memory. Like this, the crashkernel high region will
only exist in high memory, and crashkernel low region only exists in low
memory. The reservation behaviour for crashkernel=,high is clearer and
simpler.

Note: RPi4 has different zone ranges than normal memory. Its DMA zone is
0~1G, and DMA32 zone is 1G~4G if CONFIG_ZONE_DMA|DMA32 are enabled by
default. The low memory end is 1G in order to validate all devices, high
memory starts at 1G memory. However, for being consistent with normla
arm64 system, its low memory end is still 1G, while reserving crashkernel
high memory from 4G if crashkernel=size,high specified. This will remove
confusion.

With above change applied, summary of arm64 crashkernel reservation range:
1)
RPi4(zone DMA:0~1G; DMA32:1G~4G):
 crashkernel=size
  0~1G: low memory | 1G~top: high memory

 crashkernel=size,high
  0~1G: low memory | 4G~top: high memory

2)
Other normal system:
 crashkernel=size
 crashkernel=size,high
  0~4G: low memory | 4G~top: high memory

3)
Systems w/o zone DMA|DMA32
 crashkernel=size
 crashkernel=size,high
  0~top: low memory

Signed-off-by: Baoquan He <bhe@redhat.com>
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
---
v6-RESEND:
 - Remove the relic of local patch merging at the end of patch log.
 - Add Catalin's Reviewed-by tag.

 arch/arm64/mm/init.c | 44 ++++++++++++++++++++++++++++++++++----------
 1 file changed, 34 insertions(+), 10 deletions(-)

diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index 66e70ca47680..c28c2c8483cc 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -69,6 +69,7 @@ phys_addr_t __ro_after_init arm64_dma_phys_limit;
 
 #define CRASH_ADDR_LOW_MAX		arm64_dma_phys_limit
 #define CRASH_ADDR_HIGH_MAX		(PHYS_MASK + 1)
+#define CRASH_HIGH_SEARCH_BASE		SZ_4G
 
 #define DEFAULT_CRASH_KERNEL_LOW_SIZE	(128UL << 20)
 
@@ -101,12 +102,13 @@ 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_low_size = 0;
+	unsigned long long crash_low_size = 0, search_base = 0;
 	unsigned long long crash_max = CRASH_ADDR_LOW_MAX;
+	unsigned long long crash_base, crash_size;
 	char *cmdline = boot_command_line;
-	int ret;
 	bool fixed_base = false;
+	bool high = false;
+	int ret;
 
 	if (!IS_ENABLED(CONFIG_KEXEC_CORE))
 		return;
@@ -129,7 +131,9 @@ static void __init reserve_crashkernel(void)
 		else if (ret)
 			return;
 
+		search_base = CRASH_HIGH_SEARCH_BASE;
 		crash_max = CRASH_ADDR_HIGH_MAX;
+		high = true;
 	} else if (ret || !crash_size) {
 		/* The specified value is invalid */
 		return;
@@ -140,31 +144,51 @@ 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
-		 * high memory, the minimum required low memory will be
-		 * reserved later.
+		 * For crashkernel=size[KMG]@offset[KMG], print out failure
+		 * message if can't reserve the specified region.
 		 */
-		if (!fixed_base && (crash_max == CRASH_ADDR_LOW_MAX)) {
+		if (fixed_base) {
+			pr_warn("crashkernel reservation failed - memory is in use.\n");
+			return;
+		}
+
+		/*
+		 * For crashkernel=size[KMG], if the first attempt was for
+		 * low memory, fall back to high memory, the minimum required
+		 * low memory will be reserved later.
+		 */
+		if (!high && 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;
 		}
 
+		/*
+		 * For crashkernel=size[KMG],high, if the first attempt was
+		 * for high memory, fall back to low memory.
+		 */
+		if (high && crash_max == CRASH_ADDR_HIGH_MAX) {
+			crash_max = CRASH_ADDR_LOW_MAX;
+			search_base = 0;
+			goto retry;
+		}
 		pr_warn("cannot allocate crashkernel (size:0x%llx)\n",
 			crash_size);
 		return;
 	}
 
-	if ((crash_base > CRASH_ADDR_LOW_MAX - crash_low_size) &&
-	     crash_low_size && reserve_crashkernel_low(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;
 	}
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v6 1/2] arm64: kdump: simplify the reservation behaviour of crashkernel=,high
  2023-05-15  6:02 ` [PATCH v6 1/2] " Baoquan He
  2023-05-15  9:54   ` [PATCH v6 RESEND " Baoquan He
@ 2023-05-15 10:07   ` Baoquan He
  1 sibling, 0 replies; 12+ messages in thread
From: Baoquan He @ 2023-05-15 10:07 UTC (permalink / raw)
  To: linux-kernel
  Cc: catalin.marinas, will, horms, thunder.leizhen, John.p.donnelly,
	kexec, linux-arm-kernel

On 05/15/23 at 02:02pm, Baoquan He wrote:
> On arm64, reservation for 'crashkernel=xM,high' is taken by searching for
> suitable memory region top down. If the 'xM' of crashkernel high memory
> is reserved from high memory successfully, it will try to reserve
> crashkernel low memory later accoringly. Otherwise, it will try to search
> low memory area for the 'xM' suitable region. Please see the details in
> Documentation/admin-guide/kernel-parameters.txt.
> 
> While we observed an unexpected case where a reserved region crosses the
> high and low meomry boundary. E.g on a system with 4G as low memory end,
> user added the kernel parameters like: 'crashkernel=512M,high', it could
> finally have [4G-126M, 4G+386M], [1G, 1G+128M] regions in running kernel.
> The crashkernel high region crossing low and high memory boudary will bring
> issues:
> 
> 1) For crashkernel=x,high, if getting crashkernel high region across
> low and high memory boundary, then user will see two memory regions in
> low memory, and one memory region in high memory. The two crashkernel
> low memory regions are confusing as shown in above example.
> 
> 2) If people explicityly specify "crashkernel=x,high crashkernel=y,low"
> and y <= 128M, when crashkernel high region crosses low and high memory
> boundary and the part of crashkernel high reservation below boundary is
> bigger than y, the expected crahskernel low reservation will be skipped.
> But the expected crashkernel high reservation is shrank and could not
> satisfy user space requirement.
> 
> 3) The crossing boundary behaviour of crahskernel high reservation is
> different than x86 arch. On x86_64, the low memory end is 4G fixedly,
> and the memory near 4G is reserved by system, e.g for mapping firmware,
> pci mapping, so the crashkernel reservation crossing boundary never happens.
> From distros point of view, this brings inconsistency and confusion. Users
> need to dig into x86 and arm64 system details to find out why.
> 
> For kernel itself, the impact of issue 3) could be slight. While issue
> 1) and 2) cause actual impact because it brings obscure semantics and
> behaviour to crashkernel=,high reservation.
> 
> Here, for crashkernel=xM,high, search the high memory for the suitable
> region only in high memory. If failed, try reserving the suitable
> region only in low memory. Like this, the crashkernel high region will
> only exist in high memory, and crashkernel low region only exists in low
> memory. The reservation behaviour for crashkernel=,high is clearer and
> simpler.
> 
> Note: RPi4 has different zone ranges than normal memory. Its DMA zone is
> 0~1G, and DMA32 zone is 1G~4G if CONFIG_ZONE_DMA|DMA32 are enabled by
> default. The low memory end is 1G in order to validate all devices, high
> memory starts at 1G memory. However, for being consistent with normla
> arm64 system, its low memory end is still 1G, while reserving crashkernel
> high memory from 4G if crashkernel=size,high specified. This will remove
> confusion.
> 
> With above change applied, summary of arm64 crashkernel reservation range:
> 1)
> RPi4(zone DMA:0~1G; DMA32:1G~4G):
>  crashkernel=size
>   0~1G: low memory | 1G~top: high memory
> 
>  crashkernel=size,high
>   0~1G: low memory | 4G~top: high memory
> 
> 2)
> Other normal system:
>  crashkernel=size
>  crashkernel=size,high
>   0~4G: low memory | 4G~top: high memory
> 
> 3)
> Systems w/o zone DMA|DMA32
>  crashkernel=size
>  crashkernel=size,high
>   0~top: low memory
> 
> Signed-off-by: Baoquan He <bhe@redhat.com>
> 
> arm64: kdump: fix warning reported by static checker
> Signed-off-by: Baoquan He <bhe@redhat.com>

Sorry, forgot cleaning up this relic of local patch merging, have resent
one to remove it, and add Catalin's Reviewed-by tag.

Thanks
Baoquan


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v6 RESEND 1/2] arm64: kdump: simplify the reservation behaviour of crashkernel=,high
  2023-05-15  9:54   ` [PATCH v6 RESEND " Baoquan He
@ 2023-06-06 11:53     ` Leizhen (ThunderTown)
  0 siblings, 0 replies; 12+ messages in thread
From: Leizhen (ThunderTown) @ 2023-06-06 11:53 UTC (permalink / raw)
  To: Baoquan He, linux-kernel
  Cc: catalin.marinas, will, horms, John.p.donnelly, kexec,
	linux-arm-kernel



On 2023/5/15 17:54, Baoquan He wrote:
> On arm64, reservation for 'crashkernel=xM,high' is taken by searching for
> suitable memory region top down. If the 'xM' of crashkernel high memory
> is reserved from high memory successfully, it will try to reserve
> crashkernel low memory later accoringly. Otherwise, it will try to search
> low memory area for the 'xM' suitable region. Please see the details in
> Documentation/admin-guide/kernel-parameters.txt.
> 
> While we observed an unexpected case where a reserved region crosses the
> high and low meomry boundary. E.g on a system with 4G as low memory end,
> user added the kernel parameters like: 'crashkernel=512M,high', it could
> finally have [4G-126M, 4G+386M], [1G, 1G+128M] regions in running kernel.
> The crashkernel high region crossing low and high memory boudary will bring
> issues:
> 
> 1) For crashkernel=x,high, if getting crashkernel high region across
> low and high memory boundary, then user will see two memory regions in
> low memory, and one memory region in high memory. The two crashkernel
> low memory regions are confusing as shown in above example.
> 
> 2) If people explicityly specify "crashkernel=x,high crashkernel=y,low"
> and y <= 128M, when crashkernel high region crosses low and high memory
> boundary and the part of crashkernel high reservation below boundary is
> bigger than y, the expected crahskernel low reservation will be skipped.
> But the expected crashkernel high reservation is shrank and could not
> satisfy user space requirement.
> 
> 3) The crossing boundary behaviour of crahskernel high reservation is
> different than x86 arch. On x86_64, the low memory end is 4G fixedly,
> and the memory near 4G is reserved by system, e.g for mapping firmware,
> pci mapping, so the crashkernel reservation crossing boundary never happens.
>>From distros point of view, this brings inconsistency and confusion. Users
> need to dig into x86 and arm64 system details to find out why.
> 
> For kernel itself, the impact of issue 3) could be slight. While issue
> 1) and 2) cause actual impact because it brings obscure semantics and
> behaviour to crashkernel=,high reservation.
> 
> Here, for crashkernel=xM,high, search the high memory for the suitable
> region only in high memory. If failed, try reserving the suitable
> region only in low memory. Like this, the crashkernel high region will
> only exist in high memory, and crashkernel low region only exists in low
> memory. The reservation behaviour for crashkernel=,high is clearer and
> simpler.
> 
> Note: RPi4 has different zone ranges than normal memory. Its DMA zone is
> 0~1G, and DMA32 zone is 1G~4G if CONFIG_ZONE_DMA|DMA32 are enabled by
> default. The low memory end is 1G in order to validate all devices, high
> memory starts at 1G memory. However, for being consistent with normla

normla --> normal

> arm64 system, its low memory end is still 1G, while reserving crashkernel
> high memory from 4G if crashkernel=size,high specified. This will remove
> confusion.

Reviewed-by: Zhen Lei <thunder.leizhen@huawei.com>

> 
> With above change applied, summary of arm64 crashkernel reservation range:
> 1)
> RPi4(zone DMA:0~1G; DMA32:1G~4G):
>  crashkernel=size
>   0~1G: low memory | 1G~top: high memory
> 
>  crashkernel=size,high
>   0~1G: low memory | 4G~top: high memory
> 
> 2)
> Other normal system:
>  crashkernel=size
>  crashkernel=size,high
>   0~4G: low memory | 4G~top: high memory
> 
> 3)
> Systems w/o zone DMA|DMA32
>  crashkernel=size
>  crashkernel=size,high
>   0~top: low memory
> 
> Signed-off-by: Baoquan He <bhe@redhat.com>
> Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
> ---
> v6-RESEND:
>  - Remove the relic of local patch merging at the end of patch log.
>  - Add Catalin's Reviewed-by tag.
> 
>  arch/arm64/mm/init.c | 44 ++++++++++++++++++++++++++++++++++----------
>  1 file changed, 34 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> index 66e70ca47680..c28c2c8483cc 100644
> --- a/arch/arm64/mm/init.c
> +++ b/arch/arm64/mm/init.c
> @@ -69,6 +69,7 @@ phys_addr_t __ro_after_init arm64_dma_phys_limit;
>  
>  #define CRASH_ADDR_LOW_MAX		arm64_dma_phys_limit
>  #define CRASH_ADDR_HIGH_MAX		(PHYS_MASK + 1)
> +#define CRASH_HIGH_SEARCH_BASE		SZ_4G
>  
>  #define DEFAULT_CRASH_KERNEL_LOW_SIZE	(128UL << 20)
>  
> @@ -101,12 +102,13 @@ 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_low_size = 0;
> +	unsigned long long crash_low_size = 0, search_base = 0;
>  	unsigned long long crash_max = CRASH_ADDR_LOW_MAX;
> +	unsigned long long crash_base, crash_size;
>  	char *cmdline = boot_command_line;
> -	int ret;
>  	bool fixed_base = false;
> +	bool high = false;
> +	int ret;
>  
>  	if (!IS_ENABLED(CONFIG_KEXEC_CORE))
>  		return;
> @@ -129,7 +131,9 @@ static void __init reserve_crashkernel(void)
>  		else if (ret)
>  			return;
>  
> +		search_base = CRASH_HIGH_SEARCH_BASE;
>  		crash_max = CRASH_ADDR_HIGH_MAX;
> +		high = true;
>  	} else if (ret || !crash_size) {
>  		/* The specified value is invalid */
>  		return;
> @@ -140,31 +144,51 @@ 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
> -		 * high memory, the minimum required low memory will be
> -		 * reserved later.
> +		 * For crashkernel=size[KMG]@offset[KMG], print out failure
> +		 * message if can't reserve the specified region.
>  		 */
> -		if (!fixed_base && (crash_max == CRASH_ADDR_LOW_MAX)) {
> +		if (fixed_base) {
> +			pr_warn("crashkernel reservation failed - memory is in use.\n");
> +			return;
> +		}
> +
> +		/*
> +		 * For crashkernel=size[KMG], if the first attempt was for
> +		 * low memory, fall back to high memory, the minimum required
> +		 * low memory will be reserved later.
> +		 */
> +		if (!high && 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;
>  		}
>  
> +		/*
> +		 * For crashkernel=size[KMG],high, if the first attempt was
> +		 * for high memory, fall back to low memory.
> +		 */
> +		if (high && crash_max == CRASH_ADDR_HIGH_MAX) {
> +			crash_max = CRASH_ADDR_LOW_MAX;
> +			search_base = 0;
> +			goto retry;
> +		}
>  		pr_warn("cannot allocate crashkernel (size:0x%llx)\n",
>  			crash_size);
>  		return;
>  	}
>  
> -	if ((crash_base > CRASH_ADDR_LOW_MAX - crash_low_size) &&
> -	     crash_low_size && reserve_crashkernel_low(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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v6 2/2] Documentation: add kdump.rst to present crashkernel reservation on arm64
  2023-05-15  6:02 ` [PATCH v6 2/2] Documentation: add kdump.rst to present crashkernel reservation on arm64 Baoquan He
@ 2023-06-06 12:08   ` Leizhen (ThunderTown)
  0 siblings, 0 replies; 12+ messages in thread
From: Leizhen (ThunderTown) @ 2023-06-06 12:08 UTC (permalink / raw)
  To: Baoquan He, linux-kernel
  Cc: catalin.marinas, will, horms, John.p.donnelly, kexec,
	linux-arm-kernel



On 2023/5/15 14:02, Baoquan He wrote:
> People complained the crashkernel reservation code flow is hard to
> follow, so add this document to explain the background, concepts and
> implementation of crashkernel reservation on arm64. Hope this can help
> people to understand it more easily.

Reviewed-by: Zhen Lei <thunder.leizhen@huawei.com>

> 
> Signed-off-by: Baoquan He <bhe@redhat.com>
> ---
>  Documentation/arm64/kdump.rst | 103 ++++++++++++++++++++++++++++++++++
>  1 file changed, 103 insertions(+)
>  create mode 100644 Documentation/arm64/kdump.rst
> 
> diff --git a/Documentation/arm64/kdump.rst b/Documentation/arm64/kdump.rst
> new file mode 100644
> index 000000000000..78b22017c490
> --- /dev/null
> +++ b/Documentation/arm64/kdump.rst
> @@ -0,0 +1,103 @@
> +=======================================
> +crashkernel memory reservation on arm64
> +=======================================
> +
> +Author: Baoquan He <bhe@redhat.com>
> +
> +Kdump mechanism is utilized to capture corrupted kernel's vmcore so
> +that people can analyze it to get the root cause of corruption. In
> +order to do that, a preliminarily reserved memory is needed to load
> +in kdump kernel, and switch to kdump kernel to boot up and run if
> +corruption happened.
> +
> +That reserved memory for kdump is adapted to be able to minimally
> +accommodate kdump kernel to boot and run, and user space programs
> +running to do the vmcore collecting.
> +
> +Kernel parameter
> +================
> +Through kernel parameter like below, memory can be reserved
> +accordingly during early stage of 1st kernel's bootup so that
> +continuous large chunk of memomy can be found and reserved. Meanwhile,
> +the need of low memory need be considered if crashkernel is reserved
> +in high memory area.
> +
> +- crashkernel=size@offset
> +- crashkernel=size
> +- crashkernel=size,high crashkernel=size,low
> +
> +Low memory and high memory
> +===============
> +What is low memory and high memory? In kdump reservation, low memory
> +means the memory area under a specific limitation, and it's usually
> +decided by the lowest addressing bits of PCI devices which kdump kernel
> +need rely on to boot up and collect vmcore successfully. Those devices
> +not related to vmcore dumping can be ignored, e.g on x86, those i2c may
> +only be able to access 24bits addressing area, but kdump kernel still
> +take 4G as the limitation because all known devices that kdump kernel
> +cares about have 32bits addressing ability. On arm64, the low memory
> +upper boundary is not fixed, it's 1G on RPi4 platform, while 4G on normal
> +arm64 system. On the special system with CONFIG_ZONE_DMA|DMA32 disabled,
> +the whole system RAM is low memory. Except of low memory, all the rest
> +of system RAM is high memory which kernel and user space programs can
> +require to allocate and use.
> +
> +Implementation
> +==============
> +1)crashkernel=size@offset
> +-------------------------
> +crashkernel memory must be reserved at the user specified region, otherwise
> +fail if already occupied.
> +
> +
> +2) crashkernel=size
> +-------------------
> +crashkernel memory region will be reserved in any available position
> +according to searching order.
> +
> +Firstly, it searches the low memory area for an available region with specified
> +size.
> +
> +Secondly, if searching low memory failed, fallback to search the high memory
> +area with the specified size. Meanwhile, if the reservation in high memory
> +succeeds, a default reservation in low memory will be done, the current default
> +value is 128M which is satisfying the low memory needs, e.g pci device driver
> +initialization.
> +
> +If both the above searching failed, the reservation will fail finally.
> +
> +Note: crashkernel=size is recommended option among crashkernel kernel
> +parameters. With it, user doesn't need to know much about system memory
> +information, just need to specify whatever memory kdump kernel needs to
> +make vmcore dumping succeed.
> +
> +3) crashkernel=size,high crashkernel=size,low
> +--------------------------------------------
> +crashkernel=size,high is an important supplement to crashkernel=size. It
> +allows user to precisely specify how much memory need be allocated from
> +high memory, and how much memory is needed from low memory. On system
> +with large memory, low memory is small and precious since some kernel
> +feature and many devices can only request memory from the area, while
> +requiring a large chunk of continuous memory from high memory area doesn't
> +matter much and can satisfy most of kernel and almost all user space
> +programs' requirement. In such case, only a small part of necessary memory
> +from low memory area can satisfy needs. With it, the 1st kernel's normal
> +running won't be impacted because of limited low memory resource.
> +
> +To reserve memory for crashkernel=size,high, firstly, searching is tried in
> +high memory region. If reservation succeeds, low memory reservaton will be
> +done subsequently.
> +
> +Secondly, if reservation in high memory failed, fallback to search the
> +low memory with the specified size in crsahkernel=,high. If succeeds,
> +everything is fine since no low memory is needed.
> +
> +Notes:
> +- If crashkernel=,low is not specified, the default low memory reservation
> +  will be done automically.
> +
> +- if crashkernel=0,low is specified, means that low memory reservation is
> +  ommited intentionally.

ommited --> omitted

> +
> +3)

This line seems to be deleted.

> +
> 

-- 
Regards,
  Zhen Lei

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v6 0/2] arm64: kdump: simplify the reservation behaviour of crashkernel=,high
  2023-05-15  6:02 [PATCH v6 0/2] arm64: kdump: simplify the reservation behaviour of crashkernel=,high Baoquan He
  2023-05-15  6:02 ` [PATCH v6 1/2] " Baoquan He
  2023-05-15  6:02 ` [PATCH v6 2/2] Documentation: add kdump.rst to present crashkernel reservation on arm64 Baoquan He
@ 2023-06-09 19:30 ` Catalin Marinas
  2023-06-11  0:35   ` Baoquan He
  2023-06-11 12:15   ` Baoquan He
  2 siblings, 2 replies; 12+ messages in thread
From: Catalin Marinas @ 2023-06-09 19:30 UTC (permalink / raw)
  To: linux-kernel, Baoquan He
  Cc: Will Deacon, horms, thunder.leizhen, John.p.donnelly, kexec,
	linux-arm-kernel

On Mon, 15 May 2023 14:02:57 +0800, Baoquan He wrote:
> In v5 patch, Catalin helped review and acked the patch. However, an
> uninitialized local varilable is warned out by static checker when Will
> tried to merge the patch. And Will complained the code flow in
> reserve_crashkernel() is hard to follow, required to refactor. While
> when I tried to do the refactory, I feel it's not easy, the existing
> several cases causes that.
> 
> [...]

Applied to arm64 (for-next/kdump).

I reworte some of the paragraphs in the documentation patch, removed
some sentences to make it easier to read (some details were pretty
obvious). Please have a look, if you think I missed something important,
just send a patch on top. Thanks.

[1/2] arm64: kdump: simplify the reservation behaviour of crashkernel=,high
      https://git.kernel.org/arm64/c/6c4dcaddbd36
[2/2] Documentation: add kdump.rst to present crashkernel reservation on arm64
      https://git.kernel.org/arm64/c/03dc0e05407f

-- 
Catalin


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v6 0/2] arm64: kdump: simplify the reservation behaviour of crashkernel=,high
  2023-06-09 19:30 ` [PATCH v6 0/2] arm64: kdump: simplify the reservation behaviour of crashkernel=,high Catalin Marinas
@ 2023-06-11  0:35   ` Baoquan He
  2023-06-11 12:15   ` Baoquan He
  1 sibling, 0 replies; 12+ messages in thread
From: Baoquan He @ 2023-06-11  0:35 UTC (permalink / raw)
  To: Catalin Marinas
  Cc: linux-kernel, Will Deacon, horms, thunder.leizhen,
	John.p.donnelly, kexec, linux-arm-kernel

On 06/09/23 at 08:30pm, Catalin Marinas wrote:
> On Mon, 15 May 2023 14:02:57 +0800, Baoquan He wrote:
> > In v5 patch, Catalin helped review and acked the patch. However, an
> > uninitialized local varilable is warned out by static checker when Will
> > tried to merge the patch. And Will complained the code flow in
> > reserve_crashkernel() is hard to follow, required to refactor. While
> > when I tried to do the refactory, I feel it's not easy, the existing
> > several cases causes that.
> > 
> > [...]
> 
> Applied to arm64 (for-next/kdump).
> 
> I reworte some of the paragraphs in the documentation patch, removed
> some sentences to make it easier to read (some details were pretty
> obvious). Please have a look, if you think I missed something important,
> just send a patch on top. Thanks.
> 
> [1/2] arm64: kdump: simplify the reservation behaviour of crashkernel=,high
>       https://git.kernel.org/arm64/c/6c4dcaddbd36
> [2/2] Documentation: add kdump.rst to present crashkernel reservation on arm64
>       https://git.kernel.org/arm64/c/03dc0e05407f

Thanks a lot, Catalin. The rewriting looks great!


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v6 0/2] arm64: kdump: simplify the reservation behaviour of crashkernel=,high
  2023-06-09 19:30 ` [PATCH v6 0/2] arm64: kdump: simplify the reservation behaviour of crashkernel=,high Catalin Marinas
  2023-06-11  0:35   ` Baoquan He
@ 2023-06-11 12:15   ` Baoquan He
  2023-06-11 14:31     ` Catalin Marinas
  1 sibling, 1 reply; 12+ messages in thread
From: Baoquan He @ 2023-06-11 12:15 UTC (permalink / raw)
  To: Catalin Marinas
  Cc: linux-kernel, Will Deacon, horms, thunder.leizhen,
	John.p.donnelly, kexec, linux-arm-kernel

Hi Catalin,

On 06/09/23 at 08:30pm, Catalin Marinas wrote:
> On Mon, 15 May 2023 14:02:57 +0800, Baoquan He wrote:
> > In v5 patch, Catalin helped review and acked the patch. However, an
> > uninitialized local varilable is warned out by static checker when Will
> > tried to merge the patch. And Will complained the code flow in
> > reserve_crashkernel() is hard to follow, required to refactor. While
> > when I tried to do the refactory, I feel it's not easy, the existing
> > several cases causes that.
> > 
> > [...]
> 
> Applied to arm64 (for-next/kdump).
> 
> I reworte some of the paragraphs in the documentation patch, removed
> some sentences to make it easier to read (some details were pretty
> obvious). Please have a look, if you think I missed something important,
> just send a patch on top. Thanks.
> 
> [1/2] arm64: kdump: simplify the reservation behaviour of crashkernel=,high
>       https://git.kernel.org/arm64/c/6c4dcaddbd36
> [2/2] Documentation: add kdump.rst to present crashkernel reservation on arm64
>       https://git.kernel.org/arm64/c/03dc0e05407f

Could you help add below code change into the document patch commit? I
forgot adding it and got warning report from lkp test robot.

https://lore.kernel.org/oe-kbuild-all/202306110549.ynH2Juok-lkp@intel.com/


diff --git a/Documentation/arm64/index.rst b/Documentation/arm64/index.rst
index ae21f8118830..dcfebddb6088 100644
--- a/Documentation/arm64/index.rst
+++ b/Documentation/arm64/index.rst
@@ -25,6 +25,7 @@ ARM64 Architecture
     sve
     tagged-address-abi
     tagged-pointers
+    kdump
 
     features
 


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v6 0/2] arm64: kdump: simplify the reservation behaviour of crashkernel=,high
  2023-06-11 12:15   ` Baoquan He
@ 2023-06-11 14:31     ` Catalin Marinas
  2023-06-11 23:06       ` Baoquan He
  0 siblings, 1 reply; 12+ messages in thread
From: Catalin Marinas @ 2023-06-11 14:31 UTC (permalink / raw)
  To: Baoquan He
  Cc: linux-kernel, Will Deacon, horms, thunder.leizhen,
	John.p.donnelly, kexec, linux-arm-kernel

On Sun, Jun 11, 2023 at 08:15:07PM +0800, Baoquan He wrote:
> On 06/09/23 at 08:30pm, Catalin Marinas wrote:
> > On Mon, 15 May 2023 14:02:57 +0800, Baoquan He wrote:
> > > In v5 patch, Catalin helped review and acked the patch. However, an
> > > uninitialized local varilable is warned out by static checker when Will
> > > tried to merge the patch. And Will complained the code flow in
> > > reserve_crashkernel() is hard to follow, required to refactor. While
> > > when I tried to do the refactory, I feel it's not easy, the existing
> > > several cases causes that.
> > > 
> > > [...]
> > 
> > Applied to arm64 (for-next/kdump).
> > 
> > I reworte some of the paragraphs in the documentation patch, removed
> > some sentences to make it easier to read (some details were pretty
> > obvious). Please have a look, if you think I missed something important,
> > just send a patch on top. Thanks.
> > 
> > [1/2] arm64: kdump: simplify the reservation behaviour of crashkernel=,high
> >       https://git.kernel.org/arm64/c/6c4dcaddbd36
> > [2/2] Documentation: add kdump.rst to present crashkernel reservation on arm64
> >       https://git.kernel.org/arm64/c/03dc0e05407f
> 
> Could you help add below code change into the document patch commit? I
> forgot adding it and got warning report from lkp test robot.
> 
> https://lore.kernel.org/oe-kbuild-all/202306110549.ynH2Juok-lkp@intel.com/
> 
> diff --git a/Documentation/arm64/index.rst b/Documentation/arm64/index.rst
> index ae21f8118830..dcfebddb6088 100644
> --- a/Documentation/arm64/index.rst
> +++ b/Documentation/arm64/index.rst
> @@ -25,6 +25,7 @@ ARM64 Architecture
>      sve
>      tagged-address-abi
>      tagged-pointers
> +    kdump

I've seen the warning as well. Please send a patch fixing this as I try
to avoid rebasing. Also we keep this part in alphabetical order.

Thanks.

-- 
Catalin

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v6 0/2] arm64: kdump: simplify the reservation behaviour of crashkernel=,high
  2023-06-11 14:31     ` Catalin Marinas
@ 2023-06-11 23:06       ` Baoquan He
  0 siblings, 0 replies; 12+ messages in thread
From: Baoquan He @ 2023-06-11 23:06 UTC (permalink / raw)
  To: Catalin Marinas
  Cc: linux-kernel, Will Deacon, horms, thunder.leizhen,
	John.p.donnelly, kexec, linux-arm-kernel

On 06/11/23 at 03:31pm, Catalin Marinas wrote:
> On Sun, Jun 11, 2023 at 08:15:07PM +0800, Baoquan He wrote:
> > On 06/09/23 at 08:30pm, Catalin Marinas wrote:
> > > On Mon, 15 May 2023 14:02:57 +0800, Baoquan He wrote:
> > > > In v5 patch, Catalin helped review and acked the patch. However, an
> > > > uninitialized local varilable is warned out by static checker when Will
> > > > tried to merge the patch. And Will complained the code flow in
> > > > reserve_crashkernel() is hard to follow, required to refactor. While
> > > > when I tried to do the refactory, I feel it's not easy, the existing
> > > > several cases causes that.
> > > > 
> > > > [...]
> > > 
> > > Applied to arm64 (for-next/kdump).
> > > 
> > > I reworte some of the paragraphs in the documentation patch, removed
> > > some sentences to make it easier to read (some details were pretty
> > > obvious). Please have a look, if you think I missed something important,
> > > just send a patch on top. Thanks.
> > > 
> > > [1/2] arm64: kdump: simplify the reservation behaviour of crashkernel=,high
> > >       https://git.kernel.org/arm64/c/6c4dcaddbd36
> > > [2/2] Documentation: add kdump.rst to present crashkernel reservation on arm64
> > >       https://git.kernel.org/arm64/c/03dc0e05407f
> > 
> > Could you help add below code change into the document patch commit? I
> > forgot adding it and got warning report from lkp test robot.
> > 
> > https://lore.kernel.org/oe-kbuild-all/202306110549.ynH2Juok-lkp@intel.com/
> > 
> > diff --git a/Documentation/arm64/index.rst b/Documentation/arm64/index.rst
> > index ae21f8118830..dcfebddb6088 100644
> > --- a/Documentation/arm64/index.rst
> > +++ b/Documentation/arm64/index.rst
> > @@ -25,6 +25,7 @@ ARM64 Architecture
> >      sve
> >      tagged-address-abi
> >      tagged-pointers
> > +    kdump
> 
> I've seen the warning as well. Please send a patch fixing this as I try
> to avoid rebasing. Also we keep this part in alphabetical order.

Has sent a patch to fix that, thanks.


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2023-06-11 23:07 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-15  6:02 [PATCH v6 0/2] arm64: kdump: simplify the reservation behaviour of crashkernel=,high Baoquan He
2023-05-15  6:02 ` [PATCH v6 1/2] " Baoquan He
2023-05-15  9:54   ` [PATCH v6 RESEND " Baoquan He
2023-06-06 11:53     ` Leizhen (ThunderTown)
2023-05-15 10:07   ` [PATCH v6 " Baoquan He
2023-05-15  6:02 ` [PATCH v6 2/2] Documentation: add kdump.rst to present crashkernel reservation on arm64 Baoquan He
2023-06-06 12:08   ` Leizhen (ThunderTown)
2023-06-09 19:30 ` [PATCH v6 0/2] arm64: kdump: simplify the reservation behaviour of crashkernel=,high Catalin Marinas
2023-06-11  0:35   ` Baoquan He
2023-06-11 12:15   ` Baoquan He
2023-06-11 14:31     ` Catalin Marinas
2023-06-11 23:06       ` Baoquan He

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).