[PATCH 0/4] kdump: add parse_crashkernel_high_low() to replace parse_crashkernel_{high|low}()

STALE1683d

5 messages, 1 author, 2021-12-25 · open the first message on its own page

[PATCH 0/4] kdump: add parse_crashkernel_high_low() to replace parse_crashkernel_{high|low}()

From: Zhen Lei <hidden>
Date: 2021-12-25 07:26:38

The bootup command line option crashkernel=Y,low is valid only when
crashkernel=X,high is specified. Putting their parsing into a separate
function makes the code logic clearer and easier to understand the strong
dependencies between them. So add parse_crashkernel_high_low() and use it
to repalce parse_crashkernel_{high|low}(). Then make the latter static,
and reduce two confusing parameters 'system_ram' and 'crash_base' of them.

All four patches in this series do the cleanups, no functional change. This
patchset is also a preparation for supporting reserve crashkernel above 4G
on arm64, and share code with x86.

The main proposal was made by Borislav Petkov.

As I've already alluded to in another mail, ontop of this there should
be a patch or multiple patches which clean this up more and perhaps even
split it into separate functions doing stuff in this order:

1. Parse all crashkernel= cmdline options
2. Do all crash_base, crash_size etc checks
3. Do the memory reservations

And all that supplied with comments explaining why stuff is being done.


Zhen Lei (4):
  kdump: add helper parse_crashkernel_high_low()
  x86/setup: Use parse_crashkernel_high_low() to simplify code
  kdump: make parse_crashkernel_{high|low}() static
  kdump: reduce unnecessary parameters of parse_crashkernel_{high|low}()

 arch/x86/kernel/setup.c    | 21 +++++++--------
 include/linux/crash_core.h |  7 +++--
 kernel/crash_core.c        | 54 +++++++++++++++++++++++++++++++-------
 3 files changed, 56 insertions(+), 26 deletions(-)

-- 
2.25.1

[PATCH 1/4] kdump: add helper parse_crashkernel_high_low()

From: Zhen Lei <hidden>
Date: 2021-12-25 07:26:40

The bootup command line option crashkernel=Y,low is valid only when
crashkernel=X,high is specified. Putting their parsing into a separate
function makes the code logic clearer and easier to understand the strong
dependencies between them.

Signed-off-by: Zhen Lei <redacted>
---
 include/linux/crash_core.h |  3 +++
 kernel/crash_core.c        | 35 +++++++++++++++++++++++++++++++++++
 2 files changed, 38 insertions(+)

diff --git a/include/linux/crash_core.h b/include/linux/crash_core.h
index de62a722431e7db..2d3a64761d18998 100644
--- a/include/linux/crash_core.h
+++ b/include/linux/crash_core.h

@@ -83,5 +83,8 @@ int parse_crashkernel_high(char *cmdline, unsigned long long system_ram,
 		unsigned long long *crash_size, unsigned long long *crash_base);
 int parse_crashkernel_low(char *cmdline, unsigned long long system_ram,
 		unsigned long long *crash_size, unsigned long long *crash_base);
+int __init parse_crashkernel_high_low(char *cmdline,
+				      unsigned long long *high_size,
+				      unsigned long long *low_size);
 
 #endif /* LINUX_CRASH_CORE_H */

diff --git a/kernel/crash_core.c b/kernel/crash_core.c
index eb53f5ec62c900f..8ab59a0e04f178f 100644
--- a/kernel/crash_core.c
+++ b/kernel/crash_core.c

@@ -295,6 +295,41 @@ int __init parse_crashkernel_low(char *cmdline,
 				"crashkernel=", suffix_tbl[SUFFIX_LOW]);
 }
 
+/**
+ * parse_crashkernel_high_low - Parsing "crashkernel=X,high" and possible
+ *				"crashkernel=Y,low".
+ * @cmdline:	The bootup command line.
+ * @high_size:	Save the memory size specified by "crashkernel=X,high".
+ * @low_size:	Save the memory size specified by "crashkernel=Y,low" or "-1"
+		if it's not specified.
+ *
+ * Returns 0 on success, else a negative status code.
+ */
+int __init parse_crashkernel_high_low(char *cmdline,
+				      unsigned long long *high_size,
+				      unsigned long long *low_size)
+{
+	int ret;
+	unsigned long long base;
+
+	BUG_ON(!high_size || !low_size);
+
+	/* crashkernel=X,high */
+	ret = parse_crashkernel_high(cmdline, 0, high_size, &base);
+	if (ret)
+		return ret;
+
+	if (*high_size <= 0)
+		return -EINVAL;
+
+	/* crashkernel=Y,low */
+	ret = parse_crashkernel_low(cmdline, 0, low_size, &base);
+	if (ret)
+		*low_size = -1;
+
+	return 0;
+}
+
 Elf_Word *append_elf_note(Elf_Word *buf, char *name, unsigned int type,
 			  void *data, size_t data_len)
 {

-- 
2.25.1

[PATCH 3/4] kdump: make parse_crashkernel_{high|low}() static

From: Zhen Lei <hidden>
Date: 2021-12-25 07:26:48

Make parse_crashkernel_{high|low}() static, they are only referenced by
parse_crashkernel_high_low() in the same file. The latter is recommended.

Signed-off-by: Zhen Lei <redacted>
---
 include/linux/crash_core.h | 4 ----
 kernel/crash_core.c        | 4 ++--
 2 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/include/linux/crash_core.h b/include/linux/crash_core.h
index 2d3a64761d18998..598fd55d83c169e 100644
--- a/include/linux/crash_core.h
+++ b/include/linux/crash_core.h

@@ -79,10 +79,6 @@ void final_note(Elf_Word *buf);
 
 int __init parse_crashkernel(char *cmdline, unsigned long long system_ram,
 		unsigned long long *crash_size, unsigned long long *crash_base);
-int parse_crashkernel_high(char *cmdline, unsigned long long system_ram,
-		unsigned long long *crash_size, unsigned long long *crash_base);
-int parse_crashkernel_low(char *cmdline, unsigned long long system_ram,
-		unsigned long long *crash_size, unsigned long long *crash_base);
 int __init parse_crashkernel_high_low(char *cmdline,
 				      unsigned long long *high_size,
 				      unsigned long long *low_size);

diff --git a/kernel/crash_core.c b/kernel/crash_core.c
index 8ab59a0e04f178f..97001820396295e 100644
--- a/kernel/crash_core.c
+++ b/kernel/crash_core.c

@@ -277,7 +277,7 @@ int __init parse_crashkernel(char *cmdline,
 					"crashkernel=", NULL);
 }
 
-int __init parse_crashkernel_high(char *cmdline,
+static int __init parse_crashkernel_high(char *cmdline,
 			     unsigned long long system_ram,
 			     unsigned long long *crash_size,
 			     unsigned long long *crash_base)

@@ -286,7 +286,7 @@ int __init parse_crashkernel_high(char *cmdline,
 				"crashkernel=", suffix_tbl[SUFFIX_HIGH]);
 }
 
-int __init parse_crashkernel_low(char *cmdline,
+static int __init parse_crashkernel_low(char *cmdline,
 			     unsigned long long system_ram,
 			     unsigned long long *crash_size,
 			     unsigned long long *crash_base)

-- 
2.25.1

[PATCH 4/4] kdump: reduce unnecessary parameters of parse_crashkernel_{high|low}()

From: Zhen Lei <hidden>
Date: 2021-12-25 07:26:48

Delete confusing parameters 'system_ram' and 'crash_base' of
parse_crashkernel_{high|low}(), they are only needed by the case of
"crashkernel=X@[offset]".

Signed-off-by: Zhen Lei <redacted>
---
 kernel/crash_core.c | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/kernel/crash_core.c b/kernel/crash_core.c
index 97001820396295e..0ebf5efce3119c5 100644
--- a/kernel/crash_core.c
+++ b/kernel/crash_core.c

@@ -278,20 +278,20 @@ int __init parse_crashkernel(char *cmdline,
 }
 
 static int __init parse_crashkernel_high(char *cmdline,
-			     unsigned long long system_ram,
-			     unsigned long long *crash_size,
-			     unsigned long long *crash_base)
+					 unsigned long long *crash_size)
 {
-	return __parse_crashkernel(cmdline, system_ram, crash_size, crash_base,
+	unsigned long long base;
+
+	return __parse_crashkernel(cmdline, 0, crash_size, &base,
 				"crashkernel=", suffix_tbl[SUFFIX_HIGH]);
 }
 
 static int __init parse_crashkernel_low(char *cmdline,
-			     unsigned long long system_ram,
-			     unsigned long long *crash_size,
-			     unsigned long long *crash_base)
+					unsigned long long *crash_size)
 {
-	return __parse_crashkernel(cmdline, system_ram, crash_size, crash_base,
+	unsigned long long base;
+
+	return __parse_crashkernel(cmdline, 0, crash_size, &base,
 				"crashkernel=", suffix_tbl[SUFFIX_LOW]);
 }

@@ -310,12 +310,11 @@ int __init parse_crashkernel_high_low(char *cmdline,
 				      unsigned long long *low_size)
 {
 	int ret;
-	unsigned long long base;
 
 	BUG_ON(!high_size || !low_size);
 
 	/* crashkernel=X,high */
-	ret = parse_crashkernel_high(cmdline, 0, high_size, &base);
+	ret = parse_crashkernel_high(cmdline, high_size);
 	if (ret)
 		return ret;

@@ -323,7 +322,7 @@ int __init parse_crashkernel_high_low(char *cmdline,
 		return -EINVAL;
 
 	/* crashkernel=Y,low */
-	ret = parse_crashkernel_low(cmdline, 0, low_size, &base);
+	ret = parse_crashkernel_low(cmdline, low_size);
 	if (ret)
 		*low_size = -1;

-- 
2.25.1

[PATCH 2/4] x86/setup: Use parse_crashkernel_high_low() to simplify code

From: Zhen Lei <hidden>
Date: 2021-12-25 07:26:48

Use parse_crashkernel_high_low() to bring the parsing of
"crashkernel=X,high" and the parsing of "crashkernel=Y,low" together, they
are strongly dependent, make code logic clear and more readable.

Suggested-by: Borislav Petkov <bp@alien8.de>
Signed-off-by: Zhen Lei <redacted>
---
 arch/x86/kernel/setup.c | 21 +++++++++------------
 1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 6a190c7f4d71b05..93d78aae1937db3 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c

@@ -416,18 +416,16 @@ static void __init memblock_x86_reserve_range_setup_data(void)
 # define CRASH_ADDR_HIGH_MAX	SZ_64T
 #endif
 
-static int __init reserve_crashkernel_low(void)
+static int __init reserve_crashkernel_low(unsigned long long low_size)
 {
 #ifdef CONFIG_X86_64
-	unsigned long long base, low_base = 0, low_size = 0;
+	unsigned long long low_base = 0;
 	unsigned long low_mem_limit;
-	int ret;
 
 	low_mem_limit = min(memblock_phys_mem_size(), CRASH_ADDR_LOW_MAX);
 
-	/* crashkernel=Y,low */
-	ret = parse_crashkernel_low(boot_command_line, low_mem_limit, &low_size, &base);
-	if (ret) {
+	/* crashkernel=Y,low is not specified */
+	if ((long)low_size < 0) {
 		/*
 		 * two parts from kernel/dma/swiotlb.c:
 		 * -swiotlb size: user-specified with swiotlb= or default.

@@ -465,7 +463,7 @@ static int __init reserve_crashkernel_low(void)
 
 static void __init reserve_crashkernel(void)
 {
-	unsigned long long crash_size, crash_base, total_mem;
+	unsigned long long crash_size, crash_base, total_mem, low_size;
 	bool high = false;
 	int ret;

@@ -474,10 +472,9 @@ static void __init reserve_crashkernel(void)
 	/* crashkernel=XM */
 	ret = parse_crashkernel(boot_command_line, total_mem, &crash_size, &crash_base);
 	if (ret != 0 || crash_size <= 0) {
-		/* crashkernel=X,high */
-		ret = parse_crashkernel_high(boot_command_line, total_mem,
-					     &crash_size, &crash_base);
-		if (ret != 0 || crash_size <= 0)
+		/* crashkernel=X,high and possible crashkernel=Y,low */
+		ret = parse_crashkernel_high_low(boot_command_line, &crash_size, &low_size);
+		if (ret)
 			return;
 		high = true;
 	}

@@ -520,7 +517,7 @@ static void __init reserve_crashkernel(void)
 		}
 	}
 
-	if (crash_base >= (1ULL << 32) && reserve_crashkernel_low()) {
+	if (crash_base >= (1ULL << 32) && reserve_crashkernel_low(low_size)) {
 		memblock_phys_free(crash_base, crash_size);
 		return;
 	}

-- 
2.25.1

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help