Re: [RFC PATCH 1/3] powernv/mce: reduce mce console logs to lesser lines.
From: Michael Ellerman <mpe@ellerman.id.au>
Date: 2019-03-29 00:22:39
Hi Mahesh, Thanks for doing this series. Mahesh J Salgaonkar [off-list ref] writes:
From: Mahesh Salgaonkar <redacted> Also add cpu number while displaying mce log. This will help cleaner logs when mce hits on multiple cpus simultaneously.
Can you include some examples of the output before and after, so it's easier to compare what the changes are. I think you have an example in patch 3, but it would be good to have it here.
quoted hunk ↗ jump to hunk
diff --git a/arch/powerpc/kernel/mce.c b/arch/powerpc/kernel/mce.c index b5fec1f9751a..44614462cb34 100644 --- a/arch/powerpc/kernel/mce.c +++ b/arch/powerpc/kernel/mce.c@@ -384,101 +387,100 @@ void machine_check_print_event_info(struct machine_check_event *evt, break; } - printk("%s%s Machine check interrupt [%s]\n", level, sevstr,
I think I'd still like the first line at least to include "machine check" somewhere, I'm not sure everyone will understand what "MCE" means. ...
+ + if (ea && evt->srr0 != ea) + sprintf(dar_str, "DAR: %016llx ", ea); + else + memset(dar_str, 0, sizeof(dar_str));
Just dar_str[0] = '\0' would work wouldn't it?
+ if (in_guest || user_mode) {
+ printk("%sMCE: CPU%d: (%s) %s %s %s at %016llx %s[%s]\n",
+ level, evt->cpu, sevstr,
+ in_guest ? "Guest" : "Host",
+ err_type, subtype, evt->srr0, dar_str,
+ evt->disposition == MCE_DISPOSITION_RECOVERED ?
+ "Recovered" : "Not recovered");
+ printk("%sMCE: CPU%d: PID: %d Comm: %s\n",
+ level, evt->cpu, current->pid, current->comm);
+ } else {
+ printk("%sMCE: CPU%d: (%s) Host %s %s at %016llx %s[%s]\n",
+ level, evt->cpu, sevstr, err_type, subtype, evt->srr0,
+ dar_str,
+ evt->disposition == MCE_DISPOSITION_RECOVERED ?
+ "Recovered" : "Not recovered");
+ printk("%sMCE: CPU%d: NIP: [%016llx] %pS\n",
+ level, evt->cpu, evt->srr0, (void *)evt->srr0);
+ }
The first printf in the two cases is quite similar, seems like they
could be consolidated.
I also think it'd be clearer to print the NIP on the 2nd line in all
cases, rather than the first.
What about (untested) ?
printk("%sMCE: CPU%d: (%s) %s %s %s %s[%s]\n",
level, evt->cpu, sevstr,
in_guest ? "Guest" : "Host",
err_type, subtype, dar_str,
evt->disposition == MCE_DISPOSITION_RECOVERED ?
"Recovered" : "Not recovered");
if (in_guest || user_mode) {
printk("%sMCE: CPU%d: PID: %d Comm: %s %sNIP: [%016llx]\n",
level, evt->cpu, current->pid, current->comm,
in_guest ? "Guest " : "", evt->srr0);
} else {
printk("%sMCE: CPU%d: NIP: [%016llx] %pS\n",
level, evt->cpu, evt->srr0, (void *)evt->srr0);
}
cheers