2015-05-15 11:13:25

by Hatayama, Daisuke

[permalink] [raw]
Subject: [PATCH v3 0/2] kernel/panic/kexec: fix "crash_kexec_post_notifiers" option issue in oops path

This patch set fixes a bug introduced in the commit
f06e5153f4ae2e2f3b0300f0e260e40cb7fefd45.

For the v2 discussion, please follow the following thread:

http://thread.gmane.org/gmane.linux.kernel/1902488

The v3 patch newly adds a patch suggested by Ingo Molnar.

---

HATAYAMA Daisuke (2):
kernel/panic: call the 2nd crash_kexec() only if crash_kexec_post_notifiers is enabled
kernel/panic/kexec: fix "crash_kexec_post_notifiers" option issue in oops path


include/linux/kernel.h | 3 +++
kernel/kexec.c | 11 +++++++++++
kernel/panic.c | 5 +++--
3 files changed, 17 insertions(+), 2 deletions(-)

--
Thanks.
HATAYAMA, Daisuke


2015-05-15 11:12:09

by Hatayama, Daisuke

[permalink] [raw]
Subject: [PATCH v3 1/2] kernel/panic: call the 2nd crash_kexec() only if crash_kexec_post_notifiers is enabled

For compatibility with the behaviour before the commit
f06e5153f4ae2e2f3b0300f0e260e40cb7fefd45, the 2nd crash_kexec() should
be called only if crash_kexec_post_notifiers is enabled.

Note that crash_kexec() returns immediately if kdump crash kernel is
not loaded, so in this case, this patch makes no functionality change,
but the point is to make it explicit, from the caller panic() side,
that the 2nd crash_kexec() does nothing.

Signed-off-by: HATAYAMA Daisuke <[email protected]>
Suggested-by: Ingo Molnar <[email protected]>
---
kernel/panic.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/kernel/panic.c b/kernel/panic.c
index 8136ad7..774614f 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -142,7 +142,8 @@ void panic(const char *fmt, ...)
* Note: since some panic_notifiers can make crashed kernel
* more unstable, it can increase risks of the kdump failure too.
*/
- crash_kexec(NULL);
+ if (crash_kexec_post_notifiers)
+ crash_kexec(NULL);

bust_spinlocks(0);

2015-05-15 11:12:23

by Hatayama, Daisuke

[permalink] [raw]
Subject: [PATCH v3 2/2] kernel/panic/kexec: fix "crash_kexec_post_notifiers" option issue in oops path

The commit f06e5153f4ae2e2f3b0300f0e260e40cb7fefd45 introduced
"crash_kexec_post_notifiers" kernel boot option, which toggles
wheather panic() calls crash_kexec() before panic_notifiers and dump
kmsg or after.

The problem is that the commit overlooks panic_on_oops kernel boot
option. If it is enabled, crash_kexec() is called directly without
going through panic() in oops path.

To fix this issue, this patch adds a check to
"crash_kexec_post_notifiers" in the condition of kexec_should_crash().

Also, put a comment in kexec_should_crash() to explain not obvious
things on this patch.

Signed-off-by: HATAYAMA Daisuke <[email protected]>
Acked-by: Baoquan He <[email protected]>
Tested-by: Hidehiro Kawai <[email protected]>
Reviewed-by: Masami Hiramatsu <[email protected]>
---
include/linux/kernel.h | 3 +++
kernel/kexec.c | 11 +++++++++++
kernel/panic.c | 2 +-
3 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 3a5b48e..bd5331c 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -438,6 +438,9 @@ extern int panic_on_unrecovered_nmi;
extern int panic_on_io_nmi;
extern int panic_on_warn;
extern int sysctl_panic_on_stackoverflow;
+
+extern bool crash_kexec_post_notifiers;
+
/*
* Only to be used by arch init code. If the user over-wrote the default
* CONFIG_PANIC_TIMEOUT, honor it.
diff --git a/kernel/kexec.c b/kernel/kexec.c
index 7a36fdc..a785c10 100644
--- a/kernel/kexec.c
+++ b/kernel/kexec.c
@@ -84,6 +84,17 @@ struct resource crashk_low_res = {

int kexec_should_crash(struct task_struct *p)
{
+ /*
+ * If crash_kexec_post_notifiers is enabled, don't run
+ * crash_kexec() here yet, which must be run after panic
+ * notifiers in panic().
+ */
+ if (crash_kexec_post_notifiers)
+ return 0;
+ /*
+ * There are 4 panic() calls in do_exit() path, each of which
+ * corresponds to each of these 4 conditions.
+ */
if (in_interrupt() || !p->pid || is_global_init(p) || panic_on_oops)
return 1;
return 0;
diff --git a/kernel/panic.c b/kernel/panic.c
index 774614f..04e91ff 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -32,7 +32,7 @@ static unsigned long tainted_mask;
static int pause_on_oops;
static int pause_on_oops_flag;
static DEFINE_SPINLOCK(pause_on_oops_lock);
-static bool crash_kexec_post_notifiers;
+bool crash_kexec_post_notifiers;
int panic_on_warn __read_mostly;

int panic_timeout = CONFIG_PANIC_TIMEOUT;