Skip to contentSkip to conversation

ia64 kdump: add kdump_on_fatal_mca

PATCH In review Hidetoshi Seto 5 participants 10 messages Apr 8, 2008 – Apr 9, 2008

lore
Share & tools
lore
#0 lore raw

PATCH 1/2 ia64 kdump: add kdump_on_fatal_mca

While it is convenient that we can invoke kdump by asserting INIT via button on chassis etc., there are some situations that invoking kdump on fatal MCA is not welcomed rather than rebooting fast without dump.

This patch adds a new flag 'kdump_on_fatal_mca' that is independent from 'kdump_on_init' currently available. Adding this flag enable us to turning on/off of kdump depend on the event, INIT and/or fatal MCA.

Thanks,
H.Seto

---
 arch/ia64/kernel/crash.c |   31 +++++++++++++++++++++++--------
 arch/ia64/kernel/mca.c   |    6 +-----
 2 files changed, 24 insertions(+), 13 deletions(-)

Index: GIT-torvalds/arch/ia64/kernel/crash.c =================================--- GIT-torvalds.orig/arch/ia64/kernel/crash.c +++ GIT-torvalds/arch/ia64/kernel/crash.c
@@ -24,6 +24,7 @@ int kdump_status[NR_CPUS];
static atomic_t kdump_cpu_frozen;
atomic_t kdump_in_progress;
static int kdump_on_init = 1;
+static int kdump_on_fatal_mca = 0;

static inline Elf64_Word
*append_elf_note(Elf64_Word *buf, char *name, unsigned type, void *data,
@@ -148,7 +149,7 @@ kdump_init_notifier(struct notifier_bloc
struct ia64_mca_notify_die *nd;
struct die_args *args = data;

- if (!kdump_on_init)
+ if (!kdump_on_init && !kdump_on_fatal_mca)
return NOTIFY_DONE;

if (!ia64_kimage) {
@@ -174,11 +175,14 @@ kdump_init_notifier(struct notifier_bloc

switch (val) {
case DIE_INIT_MONARCH_PROCESS:
- atomic_set(&kdump_in_progress, 1);
- *(nd->monarch_cpu) = -1;
+ if (kdump_on_init) {
+ atomic_set(&kdump_in_progress, 1);
+ *(nd->monarch_cpu) = -1;
+ }
break;
case DIE_INIT_MONARCH_LEAVE:
- machine_kdump_on_init();
+ if (kdump_on_init)
+ machine_kdump_on_init();
break;
case DIE_INIT_SLAVE_LEAVE:
if (atomic_read(&kdump_in_progress))
@@ -189,16 +193,19 @@ kdump_init_notifier(struct notifier_bloc
unw_init_running(kdump_cpu_freeze, NULL);
break;
case DIE_MCA_MONARCH_LEAVE:
- /* die_register->signr indicate if MCA is recoverable */
- if (!args->signr)
+ /* die_register->signr indicate if MCA is recoverable */
+ if (kdump_on_fatal_mca && !args->signr) {
+ atomic_set(&kdump_in_progress, 1);
+ *(nd->monarch_cpu) = -1;
machine_kdump_on_init();
+ }
break;
}
return NOTIFY_DONE;
}

#ifdef CONFIG_SYSCTL
-static ctl_table kdump_on_init_table[] = {
+static ctl_table kdump_ctl_table[] = {
{
.ctl_name = CTL_UNNUMBERED,
.procname = "kdump_on_init",
@@ -207,6 +214,14 @@ static ctl_table kdump_on_init_table[] .mode = 0644,
.proc_handler = &proc_dointvec,
},
+ {
+ .ctl_name = CTL_UNNUMBERED,
+ .procname = "kdump_on_fatal_mca",
+ .data = &kdump_on_fatal_mca,
+ .maxlen = sizeof(int),
+ .mode = 0644,
+ .proc_handler = &proc_dointvec,
+ },
{ .ctl_name = 0 }
};

@@ -215,7 +230,7 @@ static ctl_table sys_table[] = {
.ctl_name = CTL_KERN,
.procname = "kernel",
.mode = 0555,
- .child = kdump_on_init_table,
+ .child = kdump_ctl_table,
},
{ .ctl_name = 0 }
};
Index: GIT-torvalds/arch/ia64/kernel/mca.c =================================--- GIT-torvalds.orig/arch/ia64/kernel/mca.c +++ GIT-torvalds/arch/ia64/kernel/mca.c
@@ -1266,16 +1266,12 @@ ia64_mca_handler(struct pt_regs *regs, s
} else {
/* Dump buffered message to console */ ia64_mlogbuf_finish(1);
-#ifdef CONFIG_KEXEC
- atomic_set(&kdump_in_progress, 1);
- monarch_cpu = -1;
-#endif
}
+
if (notify_die(DIE_MCA_MONARCH_LEAVE, "MCA", regs, (long)&nd, 0, recover)
= NOTIFY_STOP)
ia64_mca_spin(__func__);

-
if (atomic_dec_return(&mca_count) > 0) {
int i;

#1 lore raw
Quoted 24 lines · Hidetoshi Seto

While it is convenient that we can invoke kdump by asserting INIT via button on chassis etc., there are some situations that invoking kdump on fatal MCA is not welcomed rather than rebooting fast without dump.

This patch adds a new flag 'kdump_on_fatal_mca' that is independent from 'kdump_on_init' currently available. Adding this flag enable us to turning on/off of kdump depend on the event, INIT and/or fatal MCA.

Thanks,
H.Seto

---
 arch/ia64/kernel/crash.c |   31 +++++++++++++++++++++++--------
 arch/ia64/kernel/mca.c   |    6 +-----
 2 files changed, 24 insertions(+), 13 deletions(-)

Index: GIT-torvalds/arch/ia64/kernel/crash.c =================================> --- GIT-torvalds.orig/arch/ia64/kernel/crash.c +++ GIT-torvalds/arch/ia64/kernel/crash.c

Review int kdump_status[NR_CPUS]; · line 27

Quoted 4 lines
(quoted diff) +1 -0
@@ -24,6 +24,7 @@ int kdump_status[NR_CPUS];
 static atomic_t kdump_cpu_frozen;
 atomic_t kdump_in_progress;
 static int kdump_on_init = 1;
+static int kdump_on_fatal_mca = 0;

I think kdump_on_fatal_mca should be set to 1 by default. Fatal mca is exactly the case where we need a dump to analyze the problem.

Thanhks,
- jay

Quoted 102 lines
(quoted diff) +22 -8
 static inline Elf64_Word
 *append_elf_note(Elf64_Word *buf, char *name, unsigned type, void *data,
@@ -148,7 +149,7 @@ kdump_init_notifier(struct notifier_bloc
 	struct ia64_mca_notify_die *nd;
 	struct die_args *args = data;
 
-	if (!kdump_on_init)
+	if (!kdump_on_init && !kdump_on_fatal_mca)
 		return NOTIFY_DONE;
 
 	if (!ia64_kimage) {
@@ -174,11 +175,14 @@ kdump_init_notifier(struct notifier_bloc
 
 	switch (val) {
 		case DIE_INIT_MONARCH_PROCESS:
-			atomic_set(&kdump_in_progress, 1);
-			*(nd->monarch_cpu) = -1;
+			if (kdump_on_init) {
+				atomic_set(&kdump_in_progress, 1);
+				*(nd->monarch_cpu) = -1;
+			}
 			break;
 		case DIE_INIT_MONARCH_LEAVE:
-			machine_kdump_on_init();
+			if (kdump_on_init)
+				machine_kdump_on_init();
 			break;
 		case DIE_INIT_SLAVE_LEAVE:
 			if (atomic_read(&kdump_in_progress))
@@ -189,16 +193,19 @@ kdump_init_notifier(struct notifier_bloc
 				unw_init_running(kdump_cpu_freeze, NULL);
 			break;
 		case DIE_MCA_MONARCH_LEAVE:
-		     /* die_register->signr indicate if MCA is recoverable */
-			if (!args->signr)
+			/* die_register->signr indicate if MCA is recoverable */
+			if (kdump_on_fatal_mca && !args->signr) {
+				atomic_set(&kdump_in_progress, 1);
+				*(nd->monarch_cpu) = -1;
 				machine_kdump_on_init();
+			}
 			break;
 	}
 	return NOTIFY_DONE;
 }
 
 #ifdef CONFIG_SYSCTL
-static ctl_table kdump_on_init_table[] = {
+static ctl_table kdump_ctl_table[] = {
 	{
 		.ctl_name = CTL_UNNUMBERED,
 		.procname = "kdump_on_init",
@@ -207,6 +214,14 @@ static ctl_table kdump_on_init_table[] >  		.mode = 0644,
 		.proc_handler = &proc_dointvec,
 	},
+	{
+		.ctl_name = CTL_UNNUMBERED,
+		.procname = "kdump_on_fatal_mca",
+		.data = &kdump_on_fatal_mca,
+		.maxlen = sizeof(int),
+		.mode = 0644,
+		.proc_handler = &proc_dointvec,
+	},
 	{ .ctl_name = 0 }
 };
 
@@ -215,7 +230,7 @@ static ctl_table sys_table[] = {
 	  .ctl_name = CTL_KERN,
 	  .procname = "kernel",
 	  .mode = 0555,
-	  .child = kdump_on_init_table,
+	  .child = kdump_ctl_table,
 	},
 	{ .ctl_name = 0 }
 };

Index: GIT-torvalds/arch/ia64/kernel/mca.c =================================> --- GIT-torvalds.orig/arch/ia64/kernel/mca.c +++ GIT-torvalds/arch/ia64/kernel/mca.c

(quoted diff) +1 -6
@@ -1266,16 +1266,12 @@ ia64_mca_handler(struct pt_regs *regs, s
 	} else {
 		/* Dump buffered message to console */
 		ia64_mlogbuf_finish(1);
-#ifdef CONFIG_KEXEC
-		atomic_set(&kdump_in_progress, 1);
-		monarch_cpu = -1;
-#endif
 	}
+
 	if (notify_die(DIE_MCA_MONARCH_LEAVE, "MCA", regs, (long)&nd, 0, recover)
 			= NOTIFY_STOP)
 		ia64_mca_spin(__func__);
 
-
 	if (atomic_dec_return(&mca_count) > 0) {
 		int i;
 
 
--

To unsubscribe from this list: send the line "unsubscribe linux-ia64" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#2 lore raw
In reply to Hidetoshi Seto
Quoted 2 lines

I think kdump_on_fatal_mca should be set to 1 by default. Fatal mca is exactly the case where we need a dump to analyze the problem.

I'm not so sure. If the fatal MCA was caused by the s/w doing something wrong (e.g. accessing non-existant memory), then a dump is useful to find out what went wrong.

But if the MCA was caused by some h/w error (e.g. 2xECC bit error in kernel memory), then a dump won't help.

Perhaps the dump would help distinguish the s/w case from the h/w case?

-Tony

#3 lore raw
In reply to Hidetoshi Seto
Quoted 11 lines · Tony
Quoted 2 lines

I think kdump_on_fatal_mca should be set to 1 by default. Fatal mca is exactly the case where we need a dump to analyze the problem.

I'm not so sure. If the fatal MCA was caused by the s/w doing something wrong (e.g. accessing non-existant memory), then a dump is useful to find out what went wrong.

But if the MCA was caused by some h/w error (e.g. 2xECC bit error in kernel memory), then a dump won't help.

Perhaps the dump would help distinguish the s/w case from the h/w case?

With a dump, certainly it would be very clear, but that is not the point.

When one puts aside a big chunk of memory for crashkernel. i think he would like to get the dump when kernel crashes. Trying to recreate a kernel crash on a large production system is expensive and sometimes not that readily reproducible.

Taking a dump on fatal MCA is the (default) bahavior today, isn't it?

- jay

Quoted 6 lines

-Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-ia64" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#4 lore raw
In reply to Hidetoshi Seto
Quoted 11 lines · Tony
Quoted 2 lines

I think kdump_on_fatal_mca should be set to 1 by default. Fatal mca is exactly the case where we need a dump to analyze the problem.

I'm not so sure. If the fatal MCA was caused by the s/w doing something wrong (e.g. accessing non-existant memory), then a dump is useful to find out what went wrong.

But if the MCA was caused by some h/w error (e.g. 2xECC bit error in kernel memory), then a dump won't help.

Perhaps the dump would help distinguish the s/w case from the h/w case?

Yes. We generally try to take a dump after a crash to collect all the available data. The analysis of the data (to determine h/w or s/w) occurs after the reboot.

As an alternative, could kdump_on_fatal_mca be turned on by default in Altix (in the Altix specific boot code)? Then we could set our default without impacting other vendors.

Thanks,

Signature · 2 lines
Russ Anderson, OS RAS/Partitioning Project Lead  
SGI - Silicon Graphics Inc          [email protected]
#5 lore raw
In reply to Hidetoshi Seto
Quoted 12 lines · Russ Anderson
Quoted 10 lines · Tony
Quoted 2 lines

I think kdump_on_fatal_mca should be set to 1 by default. Fatal mca is exactly the case where we need a dump to analyze the problem.

I'm not so sure. If the fatal MCA was caused by the s/w doing something wrong (e.g. accessing non-existant memory), then a dump is useful to find out what went wrong.

But if the MCA was caused by some h/w error (e.g. 2xECC bit error in kernel memory), then a dump won't help.

Perhaps the dump would help distinguish the s/w case from the h/w case?

Quoted 7 lines

Yes. We generally try to take a dump after a crash to collect all the available data. The analysis of the data (to determine h/w or s/w) occurs after the reboot.

As an alternative, could kdump_on_fatal_mca be turned on by default in Altix (in the Altix specific boot code)? Then we could set our default without impacting other vendors.

Yes, Propack can set the configuration on Altix. I saw the proposed patch that changes the current behavior without mentioning it at the summary, so i brought it up. ;)

- jay

Quoted 2 lines

Thanks,

#6 lore raw
In reply to Hidetoshi Seto
Quoted 3 lines

As an alternative, could kdump_on_fatal_mca be turned on by default in Altix (in the Altix specific boot code)? Then we could set our default without impacting other vendors.

This is a sysctl variable ... are there any cases where we set these to different default values for different platforms?

It would seem better for the kernel to set the default value to the most useful one, and for system owners that don't want the default behaviour to just add an entry to /etc/sysctl.conf to set their preference.

I'm not sure how we decide what is the most useful default. So far SGI are voting for enabling dumping for MCA, and Fujitsu have implicitly voted to disable (as that is what Seto-san's patch sets). Any other opinions?

-Tony

#7 lore raw
In reply to Hidetoshi Seto
Quoted 3 lines
Quoted 3 lines

I saw the proposed
patch that changes the current behavior without mentioning it at the summary, so i brought it up. ;)

Sorry about that. It's my fault.

But as the result it was lucky that we can have a good discussion about "required action after a fatal MCA".

Quoted 3 lines

So far SGI are voting for enabling dumping for MCA, and Fujitsu have implicitly voted to disable (as that is what Seto-san's patch sets). Any other opinions?

Ah, my vote can be invalid.
AFAIK, Fujitsu have both situation where kdump_on_fatal_mca should be enabled and where it should be disabled. Therefore I don't mind which is the default.
Anyway we need to put a line to /etc/sysctl.conf of one of both.

So now "enable vs disable" is not 1:1 but 1:0 or 1.5:0.5.

Tony, (if there are no other votes until your poll closing) would you apply my patch modifying the default to 1? Or I need to resend you all just modified the one byte?

Thanks,
H.Seto

#8 lore raw
In reply to Hidetoshi Seto
Quoted 3 lines

Tony, (if there are no other votes until your poll closing) would you apply my patch modifying the default to 1? Or I need to resend you all just modified the one byte?

Change is not even one byte ... just one bit ('0' = 0x30, '1' = 0x31 :-)

I made the change and committed it to my "test" branch.

-Tony

#9 lore raw
In reply to Hidetoshi Seto
Quoted 12 lines · Tony
Quoted 3 lines

As an alternative, could kdump_on_fatal_mca be turned on by default in Altix (in the Altix specific boot code)? Then we could set our default without impacting other vendors.

This is a sysctl variable ... are there any cases where we set these to different default values for different platforms?

It would seem better for the kernel to set the default value to the most useful one, and for system owners that don't want the default behaviour to just add an entry to /etc/sysctl.conf to set their preference.

Quoted 4 lines

I'm not sure how we decide what is the most useful default. So far SGI are voting for enabling dumping for MCA, and Fujitsu have implicitly voted to disable (as that is what Seto-san's patch sets). Any other opinions?

I'd like dumping on MCA to be enabled by default.