Re: [PATCH 5/5] mce: recover from "action required" errors reportedin data path in usermode
From: Minskey Guo
Date: Wed Sep 07 2011 - 23:07:44 EST
On 09/07/2011 02:05 PM, Chen Gong wrote:
ä 2011/9/1 6:26, Luck, Tony åé:
From: Tony Luck<tony.luck@xxxxxxxxx>
Two new entries in the mce severity table - one notes that data errors
observed by innocent bystanders (who happen to share a machine check
bank with the cpu experiencing the error) should be left alone by using
the "KEEP" severity.
Then inline in the do_machine_check() handler we process the user-mode
data error that was marked at MCE_AR_SEVERITY. Even though we are in
"machine check context" it is almost safe to do so. We have already
released all the other cpus from rendezvous and we know that the cpu
with the error was executing user code - so it cannot have interrupts
locked out, or hold any locks. I.e. this is almost equivalent to a
page fault. Only difference (and risk) is that on x86_64 we are still
on the machine check stack - so if another machine check arrives, we
are toast (we didn't clear MCG_STATUS - yet, so cpu will reset rather
than taking a nested machine check on the same stack).
Signed-off-by: Tony Luck<tony.luck@xxxxxxxxx>
---
Using the "KEEP" state avoids the complexity of my earlier solution
that sorted the cpus by severity and ran the more serious ones first.
arch/x86/kernel/cpu/mcheck/mce-severity.c | 14 ++++++++++-
arch/x86/kernel/cpu/mcheck/mce.c | 35
++++++++++++++++++++--------
2 files changed, 38 insertions(+), 11 deletions(-)
diff --git a/arch/x86/kernel/cpu/mcheck/mce-severity.c
b/arch/x86/kernel/cpu/mcheck/mce-severity.c
index 7395d5f..c4d8b24 100644
--- a/arch/x86/kernel/cpu/mcheck/mce-severity.c
+++ b/arch/x86/kernel/cpu/mcheck/mce-severity.c
@@ -54,6 +54,7 @@ static struct severity {
#define MASK(x, y) .mask = x, .result = y
#define MCI_UC_S (MCI_STATUS_UC|MCI_STATUS_S)
#define MCI_UC_SAR (MCI_STATUS_UC|MCI_STATUS_S|MCI_STATUS_AR)
+#define MCI_ADDR (MCI_STATUS_ADDRV|MCI_STATUS_MISCV)
#define MCACOD 0xffff
MCESEV(
@@ -102,11 +103,22 @@ static struct severity {
SER, BITCLR(MCI_STATUS_S)
),
- /* AR add known MCACODs here */
MCESEV(
PANIC, "Action required with lost events",
SER, BITSET(MCI_STATUS_OVER|MCI_UC_SAR)
),
+
+ /* known AR MCACODs: */
+ MCESEV(
+ KEEP, "HT thread notices Action required: data load error",
+ SER, MASK(MCI_STATUS_OVER|MCI_UC_SAR|MCI_ADDR|MCACOD,
MCI_UC_SAR|MCI_ADDR|0x0134),
+ MCGMASK(MCG_STATUS_EIPV, 0)
+ ),
+ MCESEV(
+ AR, "Action required: data load error",
+ SER, MASK(MCI_STATUS_OVER|MCI_UC_SAR|MCI_ADDR|MCACOD,
MCI_UC_SAR|MCI_ADDR|0x0134),
+ USER
+ ),
I don't think *AR* makes sense here because the following codes have a
assumption that it means *user space* condition. If so, in the future
a new *AR* severity for kernel usage is created, we can't distinguish
which one can call "memory_failure" as below. At least, it should have
a suffix such as AR_USER/AR_KERN:
enum severity_level {
MCE_NO_SEVERITY,
MCE_KEEP_SEVERITY,
MCE_SOME_SEVERITY,
MCE_AO_SEVERITY,
MCE_UC_SEVERITY,
MCE_AR_USER_SEVERITY,
MCE_AR_KERN_SEVERITY,
MCE_PANIC_SEVERITY,
};
MCESEV(
PANIC, "Action required: unknown MCACOD",
SER, MASK(MCI_STATUS_OVER|MCI_UC_SAR, MCI_UC_SAR)
diff --git a/arch/x86/kernel/cpu/mcheck/mce.c
b/arch/x86/kernel/cpu/mcheck/mce.c
index 135e12d..2c59a34 100644
--- a/arch/x86/kernel/cpu/mcheck/mce.c
+++ b/arch/x86/kernel/cpu/mcheck/mce.c
@@ -996,12 +996,6 @@ void do_machine_check(struct pt_regs *regs, long
error_code)
continue;
}
- /*
- * Kill on action required.
- */
- if (severity == MCE_AR_SEVERITY)
- kill_it = 1;
-
mce_read_aux(&m, i);
/*
@@ -1022,6 +1016,8 @@ void do_machine_check(struct pt_regs *regs,
long error_code)
}
}
+ m = *final;
+
if (!no_way_out)
mce_clear_state(toclear);
@@ -1040,7 +1036,7 @@ void do_machine_check(struct pt_regs *regs,
long error_code)
* support MCE broadcasting or it has been disabled.
*/
if (no_way_out&& tolerant< 3)
- mce_panic("Fatal machine check on current CPU", final, msg);
+ mce_panic("Fatal machine check on current CPU",&m, msg);
/*
* If the error seems to be unrecoverable, something should be
@@ -1049,11 +1045,24 @@ void do_machine_check(struct pt_regs *regs,
long error_code)
* high, don't try to do anything at all.
*/
- if (kill_it&& tolerant< 3)
+ if (worst != MCE_AR_SEVERITY&& kill_it&& tolerant< 3)
force_sig(SIGBUS, current);
if (worst> 0)
mce_report_event(regs);
+
+ if (worst == MCE_AR_SEVERITY) {
+ unsigned long pfn = m.addr>> PAGE_SHIFT;
+
+ pr_err("Uncorrected hardware memory error in user-access at
%llx",
+ m.addr);
print in the MCE handler maybe makes a deadlock ? say, when other CPUs
are printing something, suddently they received MCE broadcast from
Monarch CPU, when Monarch CPU runs above codes, a deadlock happens ?
Please fix me if I miss something :-)
+ if (__memory_failure(pfn, MCE_VECTOR, 0)< 0) {
+ pr_err("Memory error not recovered");
+ force_sig(SIGBUS, current);
+ } else
+ pr_err("Memory error recovered");
+ }
as you mentioned in the comment, the biggest concern is that when
__memory_failure runs too long, if another MCE happens at the same
time, (assuming this MCE is happened on its sibling CPU which has the
same banks), the 2nd MCE will crash the system. Why not delaying the
process in a safer context, such as using user_return_notifer ?
besides, I somewhat suspect that calling __memory_failure()
in do_machine_check() will cause deadlock.
__memory_failure() handling calls some routines, such
as is_free_buddy_page(), which needs to acquire the spin
lock, zone->lock. How can we guarantee that other CPUs
haven't acquired the lock when receiving #mc broadcast
and entering #mc handlers ?
Moreover, there are too many printk in __memory_failure()
which can cause deadlock.
-minskey
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/