public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/5] x86/setup/crash: Cleanup some code
@ 2015-10-15  8:20 Borislav Petkov
  2015-10-15  8:20 ` [PATCH 1/5] x86/setup: Cleanup crashkernel reservation functions Borislav Petkov
                   ` (6 more replies)
  0 siblings, 7 replies; 10+ messages in thread
From: Borislav Petkov @ 2015-10-15  8:20 UTC (permalink / raw)
  To: LKML
  Cc: Andrew Morton, Andy Lutomirski, Dave Young, H. Peter Anvin,
	Ingo Molnar, jerry_hoemann, Jiri Kosina, Joerg Roedel,
	Juergen Gross, Mark Salter, Thomas Gleixner, WANG Chao, x86-ml

From: Borislav Petkov <bp@suse.de>

Hi all,

this is ontop of Baoquan's fix from yesterday:
https://lkml.kernel.org/r/20151014104338.GA8235@pd.tnic

Looking at the crashkernel reservation code made a couple of style
problems wink at me so this short cleanup should address them. Please
take a look and complain if something's wrong.

Thanks.

Borislav Petkov (5):
  x86/setup: Cleanup crashkernel reservation functions
  x86/setup/crash: Remove alignment variable
  x86/setup/crash: Cleanup some more
  x86/setup/crash: Check memblock_reserve() retval
  kexec/crash: Say which char is the unrecognized

 arch/x86/kernel/setup.c | 81 +++++++++++++++++++++++++------------------------
 kernel/kexec_core.c     |  6 ++--
 2 files changed, 45 insertions(+), 42 deletions(-)

-- 
2.3.5


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

* [PATCH 1/5] x86/setup: Cleanup crashkernel reservation functions
  2015-10-15  8:20 [PATCH 0/5] x86/setup/crash: Cleanup some code Borislav Petkov
@ 2015-10-15  8:20 ` Borislav Petkov
  2015-10-15  8:20 ` [PATCH 2/5] x86/setup/crash: Remove alignment variable Borislav Petkov
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 10+ messages in thread
From: Borislav Petkov @ 2015-10-15  8:20 UTC (permalink / raw)
  To: LKML
  Cc: Andrew Morton, Andy Lutomirski, Dave Young, H. Peter Anvin,
	Ingo Molnar, jerry_hoemann, Jiri Kosina, Joerg Roedel,
	Juergen Gross, Mark Salter, Thomas Gleixner, WANG Chao, x86-ml

From: Borislav Petkov <bp@suse.de>

* Shorten variable names
* Realign code, space out for better readability

No code changed:

  # arch/x86/kernel/setup.o:

   text    data     bss     dec     hex filename
   4543    3096   69904   77543   12ee7 setup.o.before
   4543    3096   69904   77543   12ee7 setup.o.after

md5:
   8a1b7c6738a553ca207b56bd84a8f359  setup.o.before.asm
   8a1b7c6738a553ca207b56bd84a8f359  setup.o.after.asm

Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Dave Young <dyoung@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: jerry_hoemann@hp.com
Cc: Jiri Kosina <jkosina@suse.cz>
Cc: Joerg Roedel <jroedel@suse.de>
Cc: Juergen Gross <jgross@suse.com>
Cc: Mark Salter <msalter@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: WANG Chao <chaowang@redhat.com>
Cc: x86-ml <x86@kernel.org>
Signed-off-by: Borislav Petkov <bp@suse.de>
---
 arch/x86/kernel/setup.c | 43 +++++++++++++++++++++----------------------
 1 file changed, 21 insertions(+), 22 deletions(-)

diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 1b36839e41eb..fd9e178aa890 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -486,11 +486,11 @@ static void __init memblock_x86_reserve_range_setup_data(void)
  * On 64bit, old kexec-tools need to under 896MiB.
  */
 #ifdef CONFIG_X86_32
-# define CRASH_KERNEL_ADDR_LOW_MAX	(512 << 20)
-# define CRASH_KERNEL_ADDR_HIGH_MAX	(512 << 20)
+# define CRASH_ADDR_LOW_MAX	(512 << 20)
+# define CRASH_ADDR_HIGH_MAX	(512 << 20)
 #else
-# define CRASH_KERNEL_ADDR_LOW_MAX	(896UL<<20)
-# define CRASH_KERNEL_ADDR_HIGH_MAX	MAXMEM
+# define CRASH_ADDR_LOW_MAX	(896UL << 20)
+# define CRASH_ADDR_HIGH_MAX	MAXMEM
 #endif
 
 static int __init reserve_crashkernel_low(void)
@@ -503,10 +503,10 @@ static int __init reserve_crashkernel_low(void)
 	bool auto_set = false;
 	int ret;
 
-	total_low_mem = memblock_mem_size(1UL<<(32-PAGE_SHIFT));
+	total_low_mem = memblock_mem_size(1UL << (32 - PAGE_SHIFT));
+
 	/* crashkernel=Y,low */
-	ret = parse_crashkernel_low(boot_command_line, total_low_mem,
-						&low_size, &base);
+	ret = parse_crashkernel_low(boot_command_line, total_low_mem, &low_size, &base);
 	if (ret != 0) {
 		/*
 		 * two parts from lib/swiotlb.c:
@@ -517,7 +517,7 @@ static int __init reserve_crashkernel_low(void)
 		 * make sure we allocate enough extra low memory so that we
 		 * don't run out of DMA buffers for 32-bit devices.
 		 */
-		low_size = max(swiotlb_size_or_default() + (8UL<<20), 256UL<<20);
+		low_size = max(swiotlb_size_or_default() + (8UL << 20), 256UL << 20);
 		auto_set = true;
 	} else {
 		/* passed with crashkernel=0,low ? */
@@ -525,9 +525,7 @@ static int __init reserve_crashkernel_low(void)
 			return 0;
 	}
 
-	low_base = memblock_find_in_range(low_size, (1ULL<<32),
-					low_size, alignment);
-
+	low_base = memblock_find_in_range(low_size, 1ULL << 32, low_size, alignment);
 	if (!low_base) {
 		pr_err("Cannot reserve %ldMB crashkernel low memory, please try smaller size.\n",
 		       (unsigned long)(low_size >> 20));
@@ -535,10 +533,12 @@ static int __init reserve_crashkernel_low(void)
 	}
 
 	memblock_reserve(low_base, low_size);
+
 	pr_info("Reserving %ldMB of low memory at %ldMB for crashkernel (System low RAM: %ldMB)\n",
-			(unsigned long)(low_size >> 20),
-			(unsigned long)(low_base >> 20),
-			(unsigned long)(total_low_mem >> 20));
+		(unsigned long)(low_size >> 20),
+		(unsigned long)(low_base >> 20),
+		(unsigned long)(total_low_mem >> 20));
+
 	crashk_low_res.start = low_base;
 	crashk_low_res.end   = low_base + low_size - 1;
 	insert_resource(&iomem_resource, &crashk_low_res);
@@ -557,12 +557,11 @@ static void __init reserve_crashkernel(void)
 	total_mem = memblock_phys_mem_size();
 
 	/* crashkernel=XM */
-	ret = parse_crashkernel(boot_command_line, total_mem,
-			&crash_size, &crash_base);
+	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);
+					     &crash_size, &crash_base);
 		if (ret != 0 || crash_size <= 0)
 			return;
 		high = true;
@@ -574,10 +573,9 @@ static void __init reserve_crashkernel(void)
 		 *  kexec want bzImage is below CRASH_KERNEL_ADDR_MAX
 		 */
 		crash_base = memblock_find_in_range(alignment,
-					high ? CRASH_KERNEL_ADDR_HIGH_MAX :
-					       CRASH_KERNEL_ADDR_LOW_MAX,
-					crash_size, alignment);
-
+						    high ? CRASH_ADDR_HIGH_MAX
+							 : CRASH_ADDR_LOW_MAX,
+						    crash_size, alignment);
 		if (!crash_base) {
 			pr_info("crashkernel reservation failed - No suitable area found.\n");
 			return;
@@ -587,7 +585,8 @@ static void __init reserve_crashkernel(void)
 		unsigned long long start;
 
 		start = memblock_find_in_range(crash_base,
-				 crash_base + crash_size, crash_size, 1<<20);
+					       crash_base + crash_size,
+					       crash_size, 1 << 20);
 		if (start != crash_base) {
 			pr_info("crashkernel reservation failed - memory is in use.\n");
 			return;
-- 
2.3.5


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

* [PATCH 2/5] x86/setup/crash: Remove alignment variable
  2015-10-15  8:20 [PATCH 0/5] x86/setup/crash: Cleanup some code Borislav Petkov
  2015-10-15  8:20 ` [PATCH 1/5] x86/setup: Cleanup crashkernel reservation functions Borislav Petkov
@ 2015-10-15  8:20 ` Borislav Petkov
  2015-10-15  8:20 ` [PATCH 3/5] x86/setup/crash: Cleanup some more Borislav Petkov
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 10+ messages in thread
From: Borislav Petkov @ 2015-10-15  8:20 UTC (permalink / raw)
  To: LKML
  Cc: Andrew Morton, Andy Lutomirski, Dave Young, H. Peter Anvin,
	Ingo Molnar, jerry_hoemann, Jiri Kosina, Joerg Roedel,
	Juergen Gross, Mark Salter, Thomas Gleixner, WANG Chao, x86-ml

From: Borislav Petkov <bp@suse.de>

Use a macro instead. No functionality change.

Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Dave Young <dyoung@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: jerry_hoemann@hp.com
Cc: Jiri Kosina <jkosina@suse.cz>
Cc: Joerg Roedel <jroedel@suse.de>
Cc: Juergen Gross <jgross@suse.com>
Cc: Mark Salter <msalter@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: WANG Chao <chaowang@redhat.com>
Cc: x86-ml <x86@kernel.org>
Signed-off-by: Borislav Petkov <bp@suse.de>
---
 arch/x86/kernel/setup.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index fd9e178aa890..ea086dd8e821 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -480,6 +480,9 @@ static void __init memblock_x86_reserve_range_setup_data(void)
 
 #ifdef CONFIG_KEXEC_CORE
 
+/* 16M alignment for crash kernel regions */
+#define CRASH_ALIGN		(16 << 20)
+
 /*
  * Keep the crash kernel below this limit.  On 32 bits earlier kernels
  * would limit the kernel to the low 512 MiB due to mapping restrictions.
@@ -496,7 +499,6 @@ static void __init memblock_x86_reserve_range_setup_data(void)
 static int __init reserve_crashkernel_low(void)
 {
 #ifdef CONFIG_X86_64
-	const unsigned long long alignment = 16<<20;	/* 16M */
 	unsigned long long low_base = 0, low_size = 0;
 	unsigned long total_low_mem;
 	unsigned long long base;
@@ -525,7 +527,7 @@ static int __init reserve_crashkernel_low(void)
 			return 0;
 	}
 
-	low_base = memblock_find_in_range(low_size, 1ULL << 32, low_size, alignment);
+	low_base = memblock_find_in_range(low_size, 1ULL << 32, low_size, CRASH_ALIGN);
 	if (!low_base) {
 		pr_err("Cannot reserve %ldMB crashkernel low memory, please try smaller size.\n",
 		       (unsigned long)(low_size >> 20));
@@ -548,7 +550,6 @@ static int __init reserve_crashkernel_low(void)
 
 static void __init reserve_crashkernel(void)
 {
-	const unsigned long long alignment = 16<<20;	/* 16M */
 	unsigned long long total_mem;
 	unsigned long long crash_size, crash_base;
 	bool high = false;
@@ -572,10 +573,10 @@ static void __init reserve_crashkernel(void)
 		/*
 		 *  kexec want bzImage is below CRASH_KERNEL_ADDR_MAX
 		 */
-		crash_base = memblock_find_in_range(alignment,
+		crash_base = memblock_find_in_range(CRASH_ALIGN,
 						    high ? CRASH_ADDR_HIGH_MAX
 							 : CRASH_ADDR_LOW_MAX,
-						    crash_size, alignment);
+						    crash_size, CRASH_ALIGN);
 		if (!crash_base) {
 			pr_info("crashkernel reservation failed - No suitable area found.\n");
 			return;
-- 
2.3.5


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

* [PATCH 3/5] x86/setup/crash: Cleanup some more
  2015-10-15  8:20 [PATCH 0/5] x86/setup/crash: Cleanup some code Borislav Petkov
  2015-10-15  8:20 ` [PATCH 1/5] x86/setup: Cleanup crashkernel reservation functions Borislav Petkov
  2015-10-15  8:20 ` [PATCH 2/5] x86/setup/crash: Remove alignment variable Borislav Petkov
@ 2015-10-15  8:20 ` Borislav Petkov
  2015-10-15  8:20 ` [PATCH 4/5] x86/setup/crash: Check memblock_reserve() retval Borislav Petkov
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 10+ messages in thread
From: Borislav Petkov @ 2015-10-15  8:20 UTC (permalink / raw)
  To: LKML
  Cc: Andrew Morton, Andy Lutomirski, Dave Young, H. Peter Anvin,
	Ingo Molnar, jerry_hoemann, Jiri Kosina, Joerg Roedel,
	Juergen Gross, Mark Salter, Thomas Gleixner, WANG Chao, x86-ml

From: Borislav Petkov <bp@suse.de>

* Remove unused auto_set variable
* Cleanup local function variable declarations
* Reformat printk string and use pr_info()

No functionality change.

Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Dave Young <dyoung@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: jerry_hoemann@hp.com
Cc: Jiri Kosina <jkosina@suse.cz>
Cc: Joerg Roedel <jroedel@suse.de>
Cc: Juergen Gross <jgross@suse.com>
Cc: Mark Salter <msalter@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: WANG Chao <chaowang@redhat.com>
Cc: x86-ml <x86@kernel.org>
Signed-off-by: Borislav Petkov <bp@suse.de>
---
 arch/x86/kernel/setup.c | 19 +++++++------------
 1 file changed, 7 insertions(+), 12 deletions(-)

diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index ea086dd8e821..d4788719a1e2 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -499,17 +499,15 @@ static void __init memblock_x86_reserve_range_setup_data(void)
 static int __init reserve_crashkernel_low(void)
 {
 #ifdef CONFIG_X86_64
-	unsigned long long low_base = 0, low_size = 0;
+	unsigned long long base, low_base = 0, low_size = 0;
 	unsigned long total_low_mem;
-	unsigned long long base;
-	bool auto_set = false;
 	int ret;
 
 	total_low_mem = memblock_mem_size(1UL << (32 - PAGE_SHIFT));
 
 	/* crashkernel=Y,low */
 	ret = parse_crashkernel_low(boot_command_line, total_low_mem, &low_size, &base);
-	if (ret != 0) {
+	if (ret) {
 		/*
 		 * two parts from lib/swiotlb.c:
 		 * -swiotlb size: user-specified with swiotlb= or default.
@@ -520,7 +518,6 @@ static int __init reserve_crashkernel_low(void)
 		 * don't run out of DMA buffers for 32-bit devices.
 		 */
 		low_size = max(swiotlb_size_or_default() + (8UL << 20), 256UL << 20);
-		auto_set = true;
 	} else {
 		/* passed with crashkernel=0,low ? */
 		if (!low_size)
@@ -550,8 +547,7 @@ static int __init reserve_crashkernel_low(void)
 
 static void __init reserve_crashkernel(void)
 {
-	unsigned long long total_mem;
-	unsigned long long crash_size, crash_base;
+	unsigned long long crash_size, crash_base, total_mem;
 	bool high = false;
 	int ret;
 
@@ -600,11 +596,10 @@ static void __init reserve_crashkernel(void)
 		return;
 	}
 
-	printk(KERN_INFO "Reserving %ldMB of memory at %ldMB "
-			"for crashkernel (System RAM: %ldMB)\n",
-			(unsigned long)(crash_size >> 20),
-			(unsigned long)(crash_base >> 20),
-			(unsigned long)(total_mem >> 20));
+	pr_info("Reserving %ldMB of memory at %ldMB for crashkernel (System RAM: %ldMB)\n",
+		(unsigned long)(crash_size >> 20),
+		(unsigned long)(crash_base >> 20),
+		(unsigned long)(total_mem >> 20));
 
 	crashk_res.start = crash_base;
 	crashk_res.end   = crash_base + crash_size - 1;
-- 
2.3.5


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

* [PATCH 4/5] x86/setup/crash: Check memblock_reserve() retval
  2015-10-15  8:20 [PATCH 0/5] x86/setup/crash: Cleanup some code Borislav Petkov
                   ` (2 preceding siblings ...)
  2015-10-15  8:20 ` [PATCH 3/5] x86/setup/crash: Cleanup some more Borislav Petkov
@ 2015-10-15  8:20 ` Borislav Petkov
  2015-10-15  9:18   ` Dave Young
  2015-10-15  8:20 ` [PATCH 5/5] kexec/crash: Say which char is the unrecognized Borislav Petkov
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 10+ messages in thread
From: Borislav Petkov @ 2015-10-15  8:20 UTC (permalink / raw)
  To: LKML
  Cc: Andrew Morton, Andy Lutomirski, Dave Young, H. Peter Anvin,
	Ingo Molnar, jerry_hoemann, Jiri Kosina, Joerg Roedel,
	Juergen Gross, Mark Salter, Thomas Gleixner, WANG Chao, x86-ml

From: Borislav Petkov <bp@suse.de>

memblock_reserve() can fail but the crashkernel reservation code
doesn't check that and this can lead the user into believing that the
crashkernel region was actually reserved. Make sure we check that return
value and we exit early with a failure message in the error case.

Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Dave Young <dyoung@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: jerry_hoemann@hp.com
Cc: Jiri Kosina <jkosina@suse.cz>
Cc: Joerg Roedel <jroedel@suse.de>
Cc: Juergen Gross <jgross@suse.com>
Cc: Mark Salter <msalter@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: WANG Chao <chaowang@redhat.com>
Cc: x86-ml <x86@kernel.org>
Signed-off-by: Borislav Petkov <bp@suse.de>
---
 arch/x86/kernel/setup.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index d4788719a1e2..3f75297d5fd0 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -531,7 +531,11 @@ static int __init reserve_crashkernel_low(void)
 		return -ENOMEM;
 	}
 
-	memblock_reserve(low_base, low_size);
+	ret = memblock_reserve(low_base, low_size);
+	if (ret) {
+		pr_err("%s: Error reserving crashkernel low memblock.\n", __func__);
+		return ret;
+	}
 
 	pr_info("Reserving %ldMB of low memory at %ldMB for crashkernel (System low RAM: %ldMB)\n",
 		(unsigned long)(low_size >> 20),
@@ -589,7 +593,11 @@ static void __init reserve_crashkernel(void)
 			return;
 		}
 	}
-	memblock_reserve(crash_base, crash_size);
+	ret = memblock_reserve(crash_base, crash_size);
+	if (ret) {
+		pr_err("%s: Error reserving crashkernel memblock.\n", __func__);
+		return;
+	}
 
 	if (crash_base >= (1ULL << 32) && reserve_crashkernel_low()) {
 		memblock_free(crash_base, crash_size);
-- 
2.3.5


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

* [PATCH 5/5] kexec/crash: Say which char is the unrecognized
  2015-10-15  8:20 [PATCH 0/5] x86/setup/crash: Cleanup some code Borislav Petkov
                   ` (3 preceding siblings ...)
  2015-10-15  8:20 ` [PATCH 4/5] x86/setup/crash: Check memblock_reserve() retval Borislav Petkov
@ 2015-10-15  8:20 ` Borislav Petkov
  2015-10-15  9:14 ` [PATCH 0/5] x86/setup/crash: Cleanup some code Dave Young
  2015-10-15 15:33 ` Joerg Roedel
  6 siblings, 0 replies; 10+ messages in thread
From: Borislav Petkov @ 2015-10-15  8:20 UTC (permalink / raw)
  To: LKML
  Cc: Andrew Morton, Andy Lutomirski, Baoquan He, Dave Young,
	H. Peter Anvin, Ingo Molnar, jerry_hoemann, Jiri Kosina,
	Joerg Roedel, Juergen Gross, Mark Salter, Thomas Gleixner,
	Vivek Goyal, WANG Chao, x86-ml

From: Borislav Petkov <bp@suse.de>

It is helpful when the crashkernel cmdline parsing routines actually say
which character is the unrecognized one. Make them do so.

Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Baoquan He <bhe@redhat.com>
Cc: Dave Young <dyoung@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: jerry_hoemann@hp.com
Cc: Jiri Kosina <jkosina@suse.cz>
Cc: Joerg Roedel <jroedel@suse.de>
Cc: Juergen Gross <jgross@suse.com>
Cc: Mark Salter <msalter@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Vivek Goyal <vgoyal@redhat.com>
Cc: WANG Chao <chaowang@redhat.com>
Cc: x86-ml <x86@kernel.org>
Signed-off-by: Borislav Petkov <bp@suse.de>
---
 kernel/kexec_core.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
index 201b45327804..bd9f8a03cefa 100644
--- a/kernel/kexec_core.c
+++ b/kernel/kexec_core.c
@@ -1149,7 +1149,7 @@ static int __init parse_crashkernel_simple(char *cmdline,
 	if (*cur == '@')
 		*crash_base = memparse(cur+1, &cur);
 	else if (*cur != ' ' && *cur != '\0') {
-		pr_warn("crashkernel: unrecognized char\n");
+		pr_warn("crashkernel: unrecognized char: %c\n", *cur);
 		return -EINVAL;
 	}
 
@@ -1186,12 +1186,12 @@ static int __init parse_crashkernel_suffix(char *cmdline,
 
 	/* check with suffix */
 	if (strncmp(cur, suffix, strlen(suffix))) {
-		pr_warn("crashkernel: unrecognized char\n");
+		pr_warn("crashkernel: unrecognized char: %c\n", *cur);
 		return -EINVAL;
 	}
 	cur += strlen(suffix);
 	if (*cur != ' ' && *cur != '\0') {
-		pr_warn("crashkernel: unrecognized char\n");
+		pr_warn("crashkernel: unrecognized char: %c\n", *cur);
 		return -EINVAL;
 	}
 
-- 
2.3.5


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

* Re: [PATCH 0/5] x86/setup/crash: Cleanup some code
  2015-10-15  8:20 [PATCH 0/5] x86/setup/crash: Cleanup some code Borislav Petkov
                   ` (4 preceding siblings ...)
  2015-10-15  8:20 ` [PATCH 5/5] kexec/crash: Say which char is the unrecognized Borislav Petkov
@ 2015-10-15  9:14 ` Dave Young
  2015-10-15 15:33 ` Joerg Roedel
  6 siblings, 0 replies; 10+ messages in thread
From: Dave Young @ 2015-10-15  9:14 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: LKML, Andrew Morton, Andy Lutomirski, H. Peter Anvin, Ingo Molnar,
	jerry_hoemann, Jiri Kosina, Joerg Roedel, Juergen Gross,
	Mark Salter, Thomas Gleixner, WANG Chao, x86-ml

On 10/15/15 at 10:20am, Borislav Petkov wrote:
> From: Borislav Petkov <bp@suse.de>
> 
> Hi all,
> 
> this is ontop of Baoquan's fix from yesterday:
> https://lkml.kernel.org/r/20151014104338.GA8235@pd.tnic
> 
> Looking at the crashkernel reservation code made a couple of style
> problems wink at me so this short cleanup should address them. Please
> take a look and complain if something's wrong.
> 
> Thanks.
> 
> Borislav Petkov (5):
>   x86/setup: Cleanup crashkernel reservation functions
>   x86/setup/crash: Remove alignment variable
>   x86/setup/crash: Cleanup some more
>   x86/setup/crash: Check memblock_reserve() retval
>   kexec/crash: Say which char is the unrecognized
> 
>  arch/x86/kernel/setup.c | 81 +++++++++++++++++++++++++------------------------
>  kernel/kexec_core.c     |  6 ++--
>  2 files changed, 45 insertions(+), 42 deletions(-)
> 
> -- 
> 2.3.5
> 
> 

Reviewed-by: Dave Young <dyoung@redhat.com>

Thanks
Dave

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

* Re: [PATCH 4/5] x86/setup/crash: Check memblock_reserve() retval
  2015-10-15  8:20 ` [PATCH 4/5] x86/setup/crash: Check memblock_reserve() retval Borislav Petkov
@ 2015-10-15  9:18   ` Dave Young
  2015-10-15 10:03     ` Borislav Petkov
  0 siblings, 1 reply; 10+ messages in thread
From: Dave Young @ 2015-10-15  9:18 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: LKML, Andrew Morton, Andy Lutomirski, H. Peter Anvin, Ingo Molnar,
	jerry_hoemann, Jiri Kosina, Joerg Roedel, Juergen Gross,
	Mark Salter, Thomas Gleixner, WANG Chao, x86-ml

On 10/15/15 at 10:20am, Borislav Petkov wrote:
> From: Borislav Petkov <bp@suse.de>
> 
> memblock_reserve() can fail but the crashkernel reservation code
> doesn't check that and this can lead the user into believing that the
> crashkernel region was actually reserved. Make sure we check that return
> value and we exit early with a failure message in the error case.
> 
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Andy Lutomirski <luto@amacapital.net>
> Cc: Dave Young <dyoung@redhat.com>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: jerry_hoemann@hp.com
> Cc: Jiri Kosina <jkosina@suse.cz>
> Cc: Joerg Roedel <jroedel@suse.de>
> Cc: Juergen Gross <jgross@suse.com>
> Cc: Mark Salter <msalter@redhat.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: WANG Chao <chaowang@redhat.com>
> Cc: x86-ml <x86@kernel.org>
> Signed-off-by: Borislav Petkov <bp@suse.de>
> ---
>  arch/x86/kernel/setup.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> index d4788719a1e2..3f75297d5fd0 100644
> --- a/arch/x86/kernel/setup.c
> +++ b/arch/x86/kernel/setup.c
> @@ -531,7 +531,11 @@ static int __init reserve_crashkernel_low(void)
>  		return -ENOMEM;
>  	}
>  
> -	memblock_reserve(low_base, low_size);
> +	ret = memblock_reserve(low_base, low_size);
> +	if (ret) {
> +		pr_err("%s: Error reserving crashkernel low memblock.\n", __func__);
> +		return ret;
> +	}
>  

Seems there's no checking for other callback to memblock_reserve in setup.c
Need another cleanup?

BTW, a further cleanup is reasonable to me, there's a lot of below patter:
memblock_find_in_range
error checking
memblock_reserve
error checking

So a new function memblock_reserve_in_range is reasonable.

Thanks
Dave

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

* Re: [PATCH 4/5] x86/setup/crash: Check memblock_reserve() retval
  2015-10-15  9:18   ` Dave Young
@ 2015-10-15 10:03     ` Borislav Petkov
  0 siblings, 0 replies; 10+ messages in thread
From: Borislav Petkov @ 2015-10-15 10:03 UTC (permalink / raw)
  To: Dave Young
  Cc: LKML, Andrew Morton, Andy Lutomirski, H. Peter Anvin, Ingo Molnar,
	jerry_hoemann, Jiri Kosina, Joerg Roedel, Juergen Gross,
	Mark Salter, Thomas Gleixner, WANG Chao, x86-ml

On Thu, Oct 15, 2015 at 05:18:26PM +0800, Dave Young wrote:
> Seems there's no checking for other callback to memblock_reserve in setup.c
> Need another cleanup?

True story. It sure does.

> BTW, a further cleanup is reasonable to me, there's a lot of below patter:
> memblock_find_in_range
> error checking
> memblock_reserve
> error checking
> 
> So a new function memblock_reserve_in_range is reasonable.

Well, in some of the callsites, the first "error checking" issues
a specific message, depending on the subsystem. The following
memblock_reserve() is mostly unchecked though. At least that should be
fixed...

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.

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

* Re: [PATCH 0/5] x86/setup/crash: Cleanup some code
  2015-10-15  8:20 [PATCH 0/5] x86/setup/crash: Cleanup some code Borislav Petkov
                   ` (5 preceding siblings ...)
  2015-10-15  9:14 ` [PATCH 0/5] x86/setup/crash: Cleanup some code Dave Young
@ 2015-10-15 15:33 ` Joerg Roedel
  6 siblings, 0 replies; 10+ messages in thread
From: Joerg Roedel @ 2015-10-15 15:33 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: LKML, Andrew Morton, Andy Lutomirski, Dave Young, H. Peter Anvin,
	Ingo Molnar, jerry_hoemann, Jiri Kosina, Juergen Gross,
	Mark Salter, Thomas Gleixner, WANG Chao, x86-ml

On Thu, Oct 15, 2015 at 10:20:26AM +0200, Borislav Petkov wrote:
> this is ontop of Baoquan's fix from yesterday:
> https://lkml.kernel.org/r/20151014104338.GA8235@pd.tnic
> 
> Looking at the crashkernel reservation code made a couple of style
> problems wink at me so this short cleanup should address them. Please
> take a look and complain if something's wrong.
> 
> Thanks.
> 
> Borislav Petkov (5):
>   x86/setup: Cleanup crashkernel reservation functions
>   x86/setup/crash: Remove alignment variable
>   x86/setup/crash: Cleanup some more
>   x86/setup/crash: Check memblock_reserve() retval
>   kexec/crash: Say which char is the unrecognized
> 
>  arch/x86/kernel/setup.c | 81 +++++++++++++++++++++++++------------------------
>  kernel/kexec_core.c     |  6 ++--
>  2 files changed, 45 insertions(+), 42 deletions(-)

Looks good to me.

Reviewed-by: Joerg Roedel <jroedel@suse.de>


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

end of thread, other threads:[~2015-10-15 15:33 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-10-15  8:20 [PATCH 0/5] x86/setup/crash: Cleanup some code Borislav Petkov
2015-10-15  8:20 ` [PATCH 1/5] x86/setup: Cleanup crashkernel reservation functions Borislav Petkov
2015-10-15  8:20 ` [PATCH 2/5] x86/setup/crash: Remove alignment variable Borislav Petkov
2015-10-15  8:20 ` [PATCH 3/5] x86/setup/crash: Cleanup some more Borislav Petkov
2015-10-15  8:20 ` [PATCH 4/5] x86/setup/crash: Check memblock_reserve() retval Borislav Petkov
2015-10-15  9:18   ` Dave Young
2015-10-15 10:03     ` Borislav Petkov
2015-10-15  8:20 ` [PATCH 5/5] kexec/crash: Say which char is the unrecognized Borislav Petkov
2015-10-15  9:14 ` [PATCH 0/5] x86/setup/crash: Cleanup some code Dave Young
2015-10-15 15:33 ` Joerg Roedel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox