[PATCH] mtd: mtdoops: mtdoops works with kexec correctly

[PATCH] mtd: mtdoops: mtdoops works with kexec correctly

[KOSAKI Motohiro]

commit 2e386e4ba(mtdoops: refactor as a kmsg_dumper) is very good patch.
but unfortunately it doesn't works with kexec and few embedded user need
to use kexec. there is no reason to don't implement it. it isn't so
hard.

FAQ
---
 Q. Why KMSG_DUMP_OOPS is insufficient?
 A. Because crash_kexec() is called before KMSG_DUMP_OOPS if
    panic_on_oops==1.

 Q. Why do you move KMSG_DUMP_PANIC call place?
 A. Otherwise kmsg_dump() is called twice if kexec enabled.
    it's annoy.

Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
Cc: Simon Kagstrom <simon.kagstrom@netinsight.net>
Cc: Anders Grafstrom <anders.grafstrom@netinsight.net>
Cc: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
Cc: David Woodhouse <David.Woodhouse@intel.com>
---
 drivers/mtd/mtdoops.c     |    2 +-
 include/linux/kmsg_dump.h |    1 +
 kernel/kexec.c            |    4 ++++
 kernel/panic.c            |    3 ++-
 kernel/printk.c           |    1 +
 5 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/mtdoops.c b/drivers/mtd/mtdoops.c
index a714ec4..92e12df 100644
--- a/drivers/mtd/mtdoops.c
+++ b/drivers/mtd/mtdoops.c
@@ -322,7 +322,7 @@ static void mtdoops_do_dump(struct kmsg_dumper *dumper,
 	memcpy(dst + l1_cpy, s2 + s2_start, l2_cpy);
 
 	/* Panics must be written immediately */
-	if (reason == KMSG_DUMP_PANIC) {
+	if (reason != KMSG_DUMP_OOPS) {
 		if (!cxt->mtd->panic_write)
 			printk(KERN_ERR "mtdoops: Cannot write from panic without panic_write\n");
 		else
diff --git a/include/linux/kmsg_dump.h b/include/linux/kmsg_dump.h
index e32aa26..24b4414 100644
--- a/include/linux/kmsg_dump.h
+++ b/include/linux/kmsg_dump.h
@@ -17,6 +17,7 @@
 enum kmsg_dump_reason {
 	KMSG_DUMP_OOPS,
 	KMSG_DUMP_PANIC,
+	KMSG_DUMP_KEXEC,
 };
 
 /**
diff --git a/kernel/kexec.c b/kernel/kexec.c
index a9a93d9..ef077fb 100644
--- a/kernel/kexec.c
+++ b/kernel/kexec.c
@@ -32,6 +32,7 @@
 #include <linux/console.h>
 #include <linux/vmalloc.h>
 #include <linux/swap.h>
+#include <linux/kmsg_dump.h>
 
 #include <asm/page.h>
 #include <asm/uaccess.h>
@@ -1074,6 +1075,9 @@ void crash_kexec(struct pt_regs *regs)
 	if (mutex_trylock(&kexec_mutex)) {
 		if (kexec_crash_image) {
 			struct pt_regs fixed_regs;
+
+			kmsg_dump(KMSG_DUMP_KEXEC);
+
 			crash_setup_regs(&fixed_regs, regs);
 			crash_save_vmcoreinfo();
 			machine_crash_shutdown(&fixed_regs);
diff --git a/kernel/panic.c b/kernel/panic.c
index 5827f7b..c787333 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -75,7 +75,6 @@ NORET_TYPE void panic(const char * fmt, ...)
 	dump_stack();
 #endif
 
-	kmsg_dump(KMSG_DUMP_PANIC);
 	/*
 	 * If we have crashed and we have a crash kernel loaded let it handle
 	 * everything else.
@@ -83,6 +82,8 @@ NORET_TYPE void panic(const char * fmt, ...)
 	 */
 	crash_kexec(NULL);
 
+	kmsg_dump(KMSG_DUMP_PANIC);
+
 	/*
 	 * Note smp_send_stop is the usual smp shutdown function, which
 	 * unfortunately means it may not be hardened to work in a panic
diff --git a/kernel/printk.c b/kernel/printk.c
index 17463ca..1751c45 100644
--- a/kernel/printk.c
+++ b/kernel/printk.c
@@ -1467,6 +1467,7 @@ EXPORT_SYMBOL_GPL(kmsg_dump_unregister);
 static const char const *kmsg_reasons[] = {
 	[KMSG_DUMP_OOPS]	= "oops",
 	[KMSG_DUMP_PANIC]	= "panic",
+	[KMSG_DUMP_KEXEC]	= "kexec",
 };
 
 static const char *kmsg_to_str(enum kmsg_dump_reason reason)
-- 
1.6.5.2

Re: [PATCH] mtd: mtdoops: mtdoops works with kexec correctly

[Simon Kagstrom]

On Mon, 21 Dec 2009 19:20:35 +0900 (JST)
KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com> wrote:

> commit 2e386e4ba(mtdoops: refactor as a kmsg_dumper) is very good patch.
> but unfortunately it doesn't works with kexec and few embedded user need
> to use kexec. there is no reason to don't implement it. it isn't so
> hard.
> 
> FAQ
> ---
>  Q. Why KMSG_DUMP_OOPS is insufficient?
>  A. Because crash_kexec() is called before KMSG_DUMP_OOPS if
>     panic_on_oops==1.
> 
>  Q. Why do you move KMSG_DUMP_PANIC call place?
>  A. Otherwise kmsg_dump() is called twice if kexec enabled.
>     it's annoy.

I think it would be better to describe these comments in text instead
of as FAQs. Because of the --- it's also removed from the git log.
Maybe something like

	kmsg_dump: Dump on crash_kexec as well

	crash_kexec gets called before kmsg_dump(KMSG_DUMP_OOPS) if
	panic_on_oops is set, so the kernel log buffer is not stored
	for this case.

	This patch adds a KMSG_DUMP_KEXEC dump type which gets called
	when crash_kexec() is invoked. To avoid getting double dumps,
	the old KMSG_DUMP_PANIC is moved below crash_kexec(). The
	mtdoops driver is modified to handle KMSG_DUMP_KEXEC in the
	same way as a panic.


Other than that, it looks fine to me. I've done basic testing against
2.6.33-rc1, but unfortunately without kexec support.

Acked-by: Simon Kagstrom <simon.kagstrom@netinsight.net>

// Simon

Re: [PATCH] mtd: mtdoops: mtdoops works with kexec correctly

[KOSAKI Motohiro]

> On Mon, 21 Dec 2009 19:20:35 +0900 (JST)
> KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com> wrote:
> 
> > commit 2e386e4ba(mtdoops: refactor as a kmsg_dumper) is very good patch.
> > but unfortunately it doesn't works with kexec and few embedded user need
> > to use kexec. there is no reason to don't implement it. it isn't so
> > hard.
> > 
> > FAQ
> > ---
> >  Q. Why KMSG_DUMP_OOPS is insufficient?
> >  A. Because crash_kexec() is called before KMSG_DUMP_OOPS if
> >     panic_on_oops==1.
> > 
> >  Q. Why do you move KMSG_DUMP_PANIC call place?
> >  A. Otherwise kmsg_dump() is called twice if kexec enabled.
> >     it's annoy.
> 
> I think it would be better to describe these comments in text instead
> of as FAQs. Because of the --- it's also removed from the git log.
> Maybe something like
> 
> 	kmsg_dump: Dump on crash_kexec as well
> 
> 	crash_kexec gets called before kmsg_dump(KMSG_DUMP_OOPS) if
> 	panic_on_oops is set, so the kernel log buffer is not stored
> 	for this case.
> 
> 	This patch adds a KMSG_DUMP_KEXEC dump type which gets called
> 	when crash_kexec() is invoked. To avoid getting double dumps,
> 	the old KMSG_DUMP_PANIC is moved below crash_kexec(). The
> 	mtdoops driver is modified to handle KMSG_DUMP_KEXEC in the
> 	same way as a panic.
> 
> 
> Other than that, it looks fine to me. I've done basic testing against
> 2.6.33-rc1, but unfortunately without kexec support.
> 
> Acked-by: Simon Kagstrom <simon.kagstrom@netinsight.net>

Thanks. Your description is obviously better.

[PATCH v2] kmsg_dump: Dump on crash_kexec as well

[KOSAKI Motohiro]

David, can you please pick this patch into your tree?

======================
crash_kexec gets called before kmsg_dump(KMSG_DUMP_OOPS) if
panic_on_oops is set, so the kernel log buffer is not stored
for this case.

This patch adds a KMSG_DUMP_KEXEC dump type which gets called
when crash_kexec() is invoked. To avoid getting double dumps,
the old KMSG_DUMP_PANIC is moved below crash_kexec(). The
mtdoops driver is modified to handle KMSG_DUMP_KEXEC in the
same way as a panic.

Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
Acked-by: Simon Kagstrom <simon.kagstrom@netinsight.net>
Cc: Anders Grafstrom <anders.grafstrom@netinsight.net>
Cc: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
Cc: David Woodhouse <David.Woodhouse@intel.com>
---
 drivers/mtd/mtdoops.c     |    2 +-
 include/linux/kmsg_dump.h |    1 +
 kernel/kexec.c            |    4 ++++
 kernel/panic.c            |    3 ++-
 kernel/printk.c           |    1 +
 5 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/mtdoops.c b/drivers/mtd/mtdoops.c
index a714ec4..92e12df 100644
--- a/drivers/mtd/mtdoops.c
+++ b/drivers/mtd/mtdoops.c
@@ -322,7 +322,7 @@ static void mtdoops_do_dump(struct kmsg_dumper *dumper,
 	memcpy(dst + l1_cpy, s2 + s2_start, l2_cpy);
 
 	/* Panics must be written immediately */
-	if (reason == KMSG_DUMP_PANIC) {
+	if (reason != KMSG_DUMP_OOPS) {
 		if (!cxt->mtd->panic_write)
 			printk(KERN_ERR "mtdoops: Cannot write from panic without panic_write\n");
 		else
diff --git a/include/linux/kmsg_dump.h b/include/linux/kmsg_dump.h
index e32aa26..24b4414 100644
--- a/include/linux/kmsg_dump.h
+++ b/include/linux/kmsg_dump.h
@@ -17,6 +17,7 @@
 enum kmsg_dump_reason {
 	KMSG_DUMP_OOPS,
 	KMSG_DUMP_PANIC,
+	KMSG_DUMP_KEXEC,
 };
 
 /**
diff --git a/kernel/kexec.c b/kernel/kexec.c
index a9a93d9..ef077fb 100644
--- a/kernel/kexec.c
+++ b/kernel/kexec.c
@@ -32,6 +32,7 @@
 #include <linux/console.h>
 #include <linux/vmalloc.h>
 #include <linux/swap.h>
+#include <linux/kmsg_dump.h>
 
 #include <asm/page.h>
 #include <asm/uaccess.h>
@@ -1074,6 +1075,9 @@ void crash_kexec(struct pt_regs *regs)
 	if (mutex_trylock(&kexec_mutex)) {
 		if (kexec_crash_image) {
 			struct pt_regs fixed_regs;
+
+			kmsg_dump(KMSG_DUMP_KEXEC);
+
 			crash_setup_regs(&fixed_regs, regs);
 			crash_save_vmcoreinfo();
 			machine_crash_shutdown(&fixed_regs);
diff --git a/kernel/panic.c b/kernel/panic.c
index 5827f7b..c787333 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -75,7 +75,6 @@ NORET_TYPE void panic(const char * fmt, ...)
 	dump_stack();
 #endif
 
-	kmsg_dump(KMSG_DUMP_PANIC);
 	/*
 	 * If we have crashed and we have a crash kernel loaded let it handle
 	 * everything else.
@@ -83,6 +82,8 @@ NORET_TYPE void panic(const char * fmt, ...)
 	 */
 	crash_kexec(NULL);
 
+	kmsg_dump(KMSG_DUMP_PANIC);
+
 	/*
 	 * Note smp_send_stop is the usual smp shutdown function, which
 	 * unfortunately means it may not be hardened to work in a panic
diff --git a/kernel/printk.c b/kernel/printk.c
index 17463ca..1751c45 100644
--- a/kernel/printk.c
+++ b/kernel/printk.c
@@ -1467,6 +1467,7 @@ EXPORT_SYMBOL_GPL(kmsg_dump_unregister);
 static const char const *kmsg_reasons[] = {
 	[KMSG_DUMP_OOPS]	= "oops",
 	[KMSG_DUMP_PANIC]	= "panic",
+	[KMSG_DUMP_KEXEC]	= "kexec",
 };
 
 static const char *kmsg_to_str(enum kmsg_dump_reason reason)
-- 
1.6.5.2