reworked machine check recovery patches

From: Tony Luck <[email protected]>

monarch clears all of the per cpu "mces_seen", so we must keep a copy to use after mce_end()

Signed-off-by Tony Luck <[email protected]>

---
 arch/x86/kernel/cpu/mcheck/mce.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

arch/x86/kernel/cpu/mcheck/mce.c +3 -0

index 3385ea2..ed1542a 100644
@@ -1046,6 +1046,9 @@ void do_machine_check(struct pt_regs *regs, long error_code)
 		}
 	}
 
+	/* Save our worst error locally, monarch will clear mces_seen */
+	m = *final;
+
 	if (!no_way_out)
 		mce_clear_state(toclear);
 

Could you clarify why we have to use mces_seen after mce_end(), please?

From: Andi Kleen <[email protected]>

Used for next patch.

Signed-off-by Andi Kleen <[email protected]>

Signed-off-by Tony Luck <[email protected]>

---
 arch/x86/kernel/cpu/mcheck/mce.c |   21 +++++++++++++--------
 1 files changed, 13 insertions(+), 8 deletions(-)

arch/x86/kernel/cpu/mcheck/mce.c +0 -0

index cbd4b0f..0349e87 100644

arch/x86/kernel/cpu/mcheck/mce.c +9 -8

+{
+	if (m->status & MCI_STATUS_MISCV)
+		m->misc = mce_rdmsrl(MSR_IA32_MCx_MISC(i));
+	if (m->status & MCI_STATUS_ADDRV)
+		m->addr = mce_rdmsrl(MSR_IA32_MCx_ADDR(i));
+}
+
 DEFINE_PER_CPU(unsigned, mce_poll_count);
 
 /*

@@ -582,10 +593,7 @@ void machine_check_poll(enum mcp_flags flags, mce_banks_t *b)
 		    (m.status & (mce_ser ? MCI_STATUS_S : MCI_STATUS_UC)))
 			continue;
 
-		if (m.status & MCI_STATUS_MISCV)
-			m.misc = mce_rdmsrl(MSR_IA32_MCx_MISC(i));
-		if (m.status & MCI_STATUS_ADDRV)
-			m.addr = mce_rdmsrl(MSR_IA32_MCx_ADDR(i));
+		mce_read_aux(&m, i);
 
 		if (!(flags & MCP_TIMESTAMP))
 			m.tsc = 0;

@@ -1027,10 +1035,7 @@ void do_machine_check(struct pt_regs *regs, long error_code)
 		if (severity == MCE_AR_SEVERITY)
 			kill_it = 1;
 
-		if (m.status & MCI_STATUS_MISCV)
-			m.misc = mce_rdmsrl(MSR_IA32_MCx_MISC(i));
-		if (m.status & MCI_STATUS_ADDRV)
-			m.addr = mce_rdmsrl(MSR_IA32_MCx_ADDR(i));
+		mce_read_aux(&m, i);
 
 		/*
 		 * Action optional error. Queue address for later processing.

Signature · 1 line

1.7.3.1

Quoted 1 line

Used for next patch.

Yeah, let's have a more general comment here like "save us some code duplication"

Quoted 3 lines

+/*
+ * Read ADDR and MISC registers.
+ */

No need for that comment, IMO. Function is small enough and the macros speak for themselves :).

Quoted 1 line

+static void mce_read_aux(struct mce *m, int i)

inline?

From: Andi Kleen <[email protected]>

SER enabled systems report the address granuality for each reported address in a machine check. But the bits below the granuality are undefined. Mask them out before logging the machine check.

Signed-off-by Andi Kleen <[email protected]>

Signed-off-by Tony Luck <[email protected]>

---
 arch/x86/kernel/cpu/mcheck/mce.c |   12 +++++++++++-
 1 files changed, 11 insertions(+), 1 deletions(-)

arch/x86/kernel/cpu/mcheck/mce.c +4 -1

index 0349e87..ffc8d11 100644
@@ -539,8 +539,18 @@ static void mce_read_aux(struct mce *m, int i)
 {
 	if (m->status & MCI_STATUS_MISCV)
 		m->misc = mce_rdmsrl(MSR_IA32_MCx_MISC(i));
-	if (m->status & MCI_STATUS_ADDRV)
+	if (m->status & MCI_STATUS_ADDRV) {
 		m->addr = mce_rdmsrl(MSR_IA32_MCx_ADDR(i));
+
+		/*
+		 * Mask the reported address by the reported granuality.

Quoted 27 lines · Tony

From: Andi Kleen <[email protected]>

SER enabled systems report the address granuality for each reported address in a machine check. But the bits below the granuality are undefined. Mask them out before logging the machine check.

Signed-off-by Andi Kleen <[email protected]>

Signed-off-by Tony Luck <[email protected]>

---
 arch/x86/kernel/cpu/mcheck/mce.c |   12 +++++++++++-
 1 files changed, 11 insertions(+), 1 deletions(-)

arch/x86/kernel/cpu/mcheck/mce.c +4 -1

index 0349e87..ffc8d11 100644
@@ -539,8 +539,18 @@ static void mce_read_aux(struct mce *m, int i)
 {
 	if (m->status & MCI_STATUS_MISCV)
 		m->misc = mce_rdmsrl(MSR_IA32_MCx_MISC(i));
-	if (m->status & MCI_STATUS_ADDRV)
+	if (m->status & MCI_STATUS_ADDRV) {
 		m->addr = mce_rdmsrl(MSR_IA32_MCx_ADDR(i));
+
+		/*
+		 * Mask the reported address by the reported granuality.

arch/x86/kernel/cpu/mcheck/mce.c static void mce_read_aux(struct mce *m, int i) · line 556

Quoted 10 lines

arch/x86/kernel/cpu/mcheck/mce.c +7 -0

+		 */
+		if (mce_ser && (m->status & MCI_STATUS_MISCV)) {
+			u8 shift = m->misc & 0x1f;
+			m->addr >>= shift;
+			m->addr <<= shift;
+		}
+	}
 }
 
 DEFINE_PER_CPU(unsigned, mce_poll_count);

Why do you have to mask it out in kernel, why not in user/logger?

One possible story is:
"... the brand-new Xeon XXXX has new MCx_***_VALID bit in **** register, if it is set the lower bits of MCx_ADDR indicates ****, otherwise the bits are undefined ..."

So I think that kernel should convey the raw value from hardware to userland. Even if it contains some noise on it, user can determine whether it is useful or not. And more, since this is an error record, there will be no second chance to retrieve the data afterward.

Why do you have to mask it out in kernel, why not in user/logger?

One possible story is:
 "... the brand-new Xeon XXXX has new MCx_***_VALID bit in **** register, if it is set the lower bits of MCx_ADDR indicates ****, otherwise the bits are undefined ..."

So I think that kernel should convey the raw value from hardware to userland.  Even if it contains some noise on it, user can determine whether it is useful or not.  And more, since this is an error record, there will be no second chance to retrieve the data afterward.

2011/6/10 Hidetoshi Seto <[email protected]>:

Quoted 7 lines

Why do you have to mask it out in kernel, why not in user/logger?

One possible story is:
 "... the brand-new Xeon XXXX has new MCx_***_VALID bit in **** register, if it is set the lower bits of MCx_ADDR indicates ****, otherwise the bits are undefined ..."

Quoted 4 lines

So I think that kernel should convey the raw value from hardware to userland.  Even if it contains some noise on it, user can determine whether it is useful or not.  And more, since this is an error record, there will be no second chance to retrieve the data afterward.

This is a good point - we should log the original untouched bits someplace.

From: Andi Kleen <[email protected]>

SER enabled systems report the address granuality for each reported address in a machine check. But the bits below the granuality are undefined. Mask them out before logging the machine check.

Signed-off-by Andi Kleen <[email protected]>

Signed-off-by Tony Luck <[email protected]>

---
 arch/x86/kernel/cpu/mcheck/mce.c |   12 +++++++++++-
 1 files changed, 11 insertions(+), 1 deletions(-)

arch/x86/kernel/cpu/mcheck/mce.c +4 -1

index 0349e87..ffc8d11 100644
@@ -539,8 +539,18 @@ static void mce_read_aux(struct mce *m, int i)
 {
 	if (m->status & MCI_STATUS_MISCV)
 		m->misc = mce_rdmsrl(MSR_IA32_MCx_MISC(i));
-	if (m->status & MCI_STATUS_ADDRV)
+	if (m->status & MCI_STATUS_ADDRV) {
 		m->addr = mce_rdmsrl(MSR_IA32_MCx_ADDR(i));
+
+		/*
+		 * Mask the reported address by the reported granuality.

s/granuality/granularity/g;

I would very much like to know how you guys got 'granularity' wrong _three_ times in one small patch?!!

From: Andi Kleen <[email protected]>

When hardware poison handles the current process use a forced signal with _AR severity.

Signed-off-by Andi Kleen <[email protected]>

Signed-off-by Tony Luck <[email protected]>

---
 mm/memory-failure.c |   28 ++++++++++++++++------------
 1 files changed, 16 insertions(+), 12 deletions(-)

mm/memory-failure.c +0 -0

index 2b9a5ee..a203113 100644

mm/memory-failure.c +8 -10

@@ -194,23 +193,28 @@ static int kill_proc_ao(struct task_struct *t, unsigned long addr, int trapno,
 	int ret;
 
 	printk(KERN_ERR
-		"MCE %#lx: Killing %s:%d early due to hardware memory corruption\n",
-		pfn, t->comm, t->pid);
+		"MCE %#lx: Killing %s:%d due to hardware memory corruption\n",
+	       pfn, t->comm, t->pid);
 	si.si_signo = SIGBUS;
 	si.si_errno = 0;
-	si.si_code = BUS_MCEERR_AO;
 	si.si_addr = (void *)addr;
 #ifdef __ARCH_SI_TRAPNO
 	si.si_trapno = trapno;
 #endif
 	si.si_addr_lsb = compound_trans_order(compound_head(page)) + PAGE_SHIFT;
-	/*
-	 * Don't use force here, it's convenient if the signal
-	 * can be temporarily blocked.
-	 * This could cause a loop when the user sets SIGBUS
-	 * to SIG_IGN, but hopefully no one will do that?
-	 */
-	ret = send_sig_info(SIGBUS, &si, t);  /* synchronous? */
+	if (t == current) {
+		si.si_code = BUS_MCEERR_AR;
+		ret = force_sig_info(SIGBUS, &si, t);
+	} else {
+		/*
+		 * Don't use force here, it's convenient if the signal

It doesn't make sense that the function named "*_ao" sends _AR.

I suppose that usually SRAO is handled in worker thread scheduled after MCE, so current is unlikely one of affected threads in that case... And I also suppose that you'd like to use this function to be called from affected thread before leaving kernel in the case of SRAR...

My concern is that "t == current" is neither strong nor clear statement to switch the type of signal.  Someone might want to use this function to inject _AO to current.

It is better to have new kill_proc_ar() (separated, or one shared _common plus a couple of _ar/_ao), I think.  I believe that there is no caller who have no idea whether it should request sending _AR or _AO.

We already have a generic facility to do such things at
return-to-userspace: _TIF_USER_RETURN_NOTIFY.

From: Tony Luck <[email protected]>

Quoted 2 lines · Ingo

We already have a generic facility to do such things at
return-to-userspace: _TIF_USER_RETURN_NOTIFY.

This just a proof of concept patch ... before this can become real the user-return-notifier code would have to be made NMI safe (currently it uses hlist_add_head/hlist_del, which would need to be changed to Ying's NMI-safe single threaded lists).

Reviewed-by Avi Kivity <[email protected]>

NOT-Signed-off-by Tony Luck <[email protected]>

---
 arch/x86/Kconfig                 |    1 +
 arch/x86/kernel/cpu/mcheck/mce.c |   47 +++++++++++++++++++++++++++----------
 arch/x86/kernel/signal.c         |    6 -----
 3 files changed, 35 insertions(+), 19 deletions(-)

arch/x86/Kconfig +1 -0

index cc6c53a..054e127 100644
@@ -845,6 +845,7 @@ config X86_REROUTE_FOR_BROKEN_BOOT_IRQS
 
 config X86_MCE
 	bool "Machine Check / overheating reporting"
+	select USER_RETURN_NOTIFIER
 	---help---
 	  Machine Check support allows the processor to notify the
 	  kernel if it detects a problem (e.g. overheating, data corruption).

arch/x86/kernel/cpu/mcheck/mce.c +27 -7

index ffc8d11..28d223e 100644
@@ -38,6 +38,7 @@
 #include <linux/mm.h>
 #include <linux/debugfs.h>
 #include <linux/edac_mce.h>
+#include <linux/user-return-notifier.h>
 
 #include <asm/processor.h>
 #include <asm/hw_irq.h>

@@ -69,6 +70,15 @@ atomic_t mce_entry;
 
 DEFINE_PER_CPU(unsigned, mce_exception_count);
 
+struct mce_notify {
+	struct user_return_notifier urn;
+	bool registered;
+};
+
+static void mce_do_notify(struct user_return_notifier *urn);
+
+static DEFINE_PER_CPU(struct mce_notify, mce_notify);
+
 /*
  * Tolerant levels:
  *   0: always panic on uncorrected errors, log corrected errors

@@ -947,6 +957,7 @@ void do_machine_check(struct pt_regs *regs, long error_code)
 	int i;
 	int worst = 0;
 	int severity;
+	struct mce_notify *np;
 	/*
 	 * Establish sequential order between the CPUs entering the machine
 	 * check handler.

@@ -1099,7 +1110,12 @@ void do_machine_check(struct pt_regs *regs, long error_code)
 		force_sig(SIGBUS, current);
 
 	/* notify userspace ASAP */
-	set_thread_flag(TIF_MCE_NOTIFY);
+	np = &__get_cpu_var(mce_notify);
+	if (np->registered == 0) {
+		np->urn.on_user_return = mce_do_notify;
+		user_return_notifier_register(&np->urn);
+		np->registered = 1;
+	}
 
 	if (worst > 0)
 		mce_report_event(regs);

@@ -1116,28 +1132,35 @@ void __attribute__((weak)) memory_failure(unsigned long pfn, int vector)
 	printk(KERN_ERR "Action optional memory failure at %lx ignored\n", pfn);
 }
 
+static void mce_process_ring(void)
+{
+	unsigned long pfn;
+
+	mce_notify_irq();
+	while (mce_ring_get(&pfn))
+		memory_failure(pfn, MCE_VECTOR);
+}
+
 /*
  * Called after mce notification in process context. This code
  * is allowed to sleep. Call the high level VM handler to process
  * any corrupted pages.
  * Assume that the work queue code only calls this one at a time
  * per CPU.
- * Note we don't disable preemption, so this code might run on the wrong
- * CPU. In this case the event is picked up by the scheduled work queue.
- * This is merely a fast path to expedite processing in some common
- * cases.
  */
-void mce_notify_process(void)
+static void mce_do_notify(struct user_return_notifier *urn)
 {
-	unsigned long pfn;

arch/x86/kernel/cpu/mcheck/mce.c +1 -3

 static void mce_process_work(struct work_struct *dummy)
 {
-	mce_notify_process();
+	mce_process_ring();
 }
 
 #ifdef CONFIG_X86_MCE_INTEL

@@ -1218,8 +1241,6 @@ int mce_notify_irq(void)
 	/* Not more than two messages every minute */
 	static DEFINE_RATELIMIT_STATE(ratelimit, 60*HZ, 2);
 
-	clear_thread_flag(TIF_MCE_NOTIFY);
-
 	if (test_and_clear_bit(0, &mce_need_notify)) {
 		wake_up_interruptible(&mce_wait);
 

arch/x86/kernel/signal.c +0 -6

index 4fd173c..44efc22 100644
@@ -838,12 +838,6 @@ static void do_signal(struct pt_regs *regs)
 void
 do_notify_resume(struct pt_regs *regs, void *unused, __u32 thread_info_flags)
 {
-#ifdef CONFIG_X86_MCE
-	/* notify userspace of pending MCEs */
-	if (thread_info_flags & _TIF_MCE_NOTIFY)
-		mce_notify_process();
-#endif /* CONFIG_X86_64 && CONFIG_X86_MCE */
-
 	/* deal with pending signal delivery */
 	if (thread_info_flags & _TIF_SIGPENDING)
 		do_signal(regs);

Now I'm reconsidering the MCE event notification mechanism. One of something nervous is whether it is really required to process "_AO" memory poisoning (i.e. mce_process_ring()) here in a process context that unfortunately interrupted by MCE (or preempted after that). I'm uncertain how long walking though the task_list for unmap will takes, and not sure it is acceptable if the unlucky thread is a kind of latency sensitive...

If we can move mce_process_ring() to worker thread completely, what we have to do will be:
 1) from NMI context, request non-NMI context by irq_work()
 2) from (irq) context, wake up loggers and schedule work if required
 3) from worker thread, process "_AO" memory poisoning etc.

So now question is why user_return_notifier is needed here. Is it just an alternative of irq_work() for !LOCAL_APIC ?

2011/6/10 Hidetoshi Seto <[email protected]>:

Quoted 16 lines

Now I'm reconsidering the MCE event notification mechanism. One of something nervous is whether it is really required to process "_AO" memory poisoning (i.e. mce_process_ring()) here in a process context that unfortunately interrupted by MCE (or preempted after that). I'm uncertain how long walking though the task_list for unmap will takes, and not sure it is acceptable if the unlucky thread is a kind of latency sensitive...

If we can move mce_process_ring() to worker thread completely, what we have to do will be:
 1) from NMI context, request non-NMI context by irq_work()
 2) from (irq) context, wake up loggers and schedule work if required
 3) from worker thread, process "_AO" memory poisoning etc.

So now question is why user_return_notifier is needed here. Is it just an alternative of irq_work() for !LOCAL_APIC ?

I switched this notification over from old TIF_MCE_NOTIFY to using user_return_notifier to preserve existing semantics, and free up that TIF bit for the task notifier for the action required case.

You are right that we should have some better method - the shot-gun approach of hitting every task on every cpu in the hope that one of them will run our code soon sounds like overkill. Using some NMI-saf method to wake a single worker thread sounds a good idea for a subsequent clean up.

You are right that we should have some better method - the shot-gun approach of hitting every task on every cpu in the hope that one of them will run our code soon sounds like overkill. Using some NMI-saf method to wake a single worker thread sounds a good idea for a subsequent clean up.

From: Tony Luck<[email protected]>

Quoted 2 lines · Ingo

We already have a generic facility to do such things at
return-to-userspace: _TIF_USER_RETURN_NOTIFY.

This just a proof of concept patch ... before this can become real the user-return-notifier code would have to be made NMI safe (currently it uses hlist_add_head/hlist_del, which would need to be changed to Ying's NMI-safe single threaded lists).

Quoted 6 lines · Tony

From: Tony Luck<[email protected]>

Quoted 2 lines · Ingo

We already have a generic facility to do such things at
return-to-userspace: _TIF_USER_RETURN_NOTIFY.

Quoted 4 lines

This just a proof of concept patch ... before this can become real the user-return-notifier code would have to be made NMI safe (currently it uses hlist_add_head/hlist_del, which would need to be changed to Ying's NMI-safe single threaded lists).

You could use irq_work_queue() to push this into an irq context, which is user-return-notifier safe.

Quoted 7 lines · Avi Kivity

Quoted 6 lines · Tony

From: Tony Luck<[email protected]>

Quoted 2 lines · Ingo

We already have a generic facility to do such things at
return-to-userspace: _TIF_USER_RETURN_NOTIFY.

Quoted 7 lines

Quoted 4 lines

This just a proof of concept patch ... before this can become real the user-return-notifier code would have to be made NMI safe (currently it uses hlist_add_head/hlist_del, which would need to be changed to Ying's NMI-safe single threaded lists).

You could use irq_work_queue() to push this into an irq context, which is user-return-notifier safe.

Maybe I'm missing something but it looks like irq_work_queue() queues work which is run in irq_work_run() with IRQs disabled. However, user return notifiers are run after IRQs get enabled in entry_64.S. And we want to run memory_failure() with IRQs enabled.

More importantly, we want to be able to do the following:

* run #MC handler which queues work

* when returning to userspace, preempt and schedule that previously queued work _before_ the process that caused the MCE gets to execute.

Imagine this scenario:

Your userspace process causes a data cache read error due to either alpha particles or maybe because the DRAM device containing the process page is faulty and generates ECC errors which the ECC code cannot correct, i.e. an uncorrectable error we definitely want to handle; IOW Action Required MCE.

Now, if you get lucky and this page is mapped only by the process that caused the MCE, you could unmap it, mark it PageReserved and cause the process to refault. But in order to do that, you want to execute the memory_failure() handler _before_ you schedule the process again.

In the instruction cache read error case, you don't have processor context to return to (or you're being too conservative and don't want to risk it) so you kill the process, which is pretty easy to do.

Does that make a bit more sense? Tony?

You're missing the flow. The MCE handler calls irq_work_queue(), which schedules a user return notifier, which does any needed processing in task context.

From: Tony Luck <[email protected]>

Existing user return notifier mechanism is designed to catch a specific cpu just as it returns to run any task in user mode. We also need a machanism to catch a specific task.

Quoted 5 lines · Tony

From: Tony Luck <[email protected]>

Existing user return notifier mechanism is designed to catch a specific cpu just as it returns to run any task in user mode.  We also need a mechanism to catch a specific task.

Why do we need that? I mean, in the remaining patches we end up either running memory_failure() or sending signals to a task. Can't we do it all in the user return notifier and not have a different notifier for each policy?

Quoted 11 lines · Borislav Petkov

Quoted 5 lines · Tony

From: Tony Luck<[email protected]>

Existing user return notifier mechanism is designed to catch a specific cpu just as it returns to run any task in user mode. We also need a mechanism to catch a specific task.

Why do we need that? I mean, in the remaining patches we end up either running memory_failure() or sending signals to a task. Can't we do it all in the user return notifier and not have a different notifier for each policy?

Unless I'm mis-reading the user-return-notifier code, it is possible that we'll context switch before we get to the notifier. At that point the user-return-notifier TIF bit is passed on from our task to the newly run-able task. But our task is still viable, so another cpu could grab it and start running it ... then we have a race ... will the new task that inherited the notifier unmap the page fast enough, or will there be a loud BANG as the original task runs right into the machine check again.

Quoted 13 lines · Tony Luck

Quoted 11 lines · Borislav Petkov

Quoted 5 lines · Tony

From: Tony Luck<[email protected]>

Existing user return notifier mechanism is designed to catch a specific cpu just as it returns to run any task in user mode. We also need a mechanism to catch a specific task.

Why do we need that? I mean, in the remaining patches we end up either running memory_failure() or sending signals to a task. Can't we do it all in the user return notifier and not have a different notifier for each policy?

Quoted 8 lines

Unless I'm mis-reading the user-return-notifier code, it is possible that we'll context switch before we get to the notifier. At that point the user-return-notifier TIF bit is passed on from our task to the newly run-able task. But our task is still viable, so another cpu could grab it and start running it ... then we have a race ... will the new task that inherited the notifier unmap the page fast enough, or will there be a loud BANG as the original task runs right into the machine check again.

Right. user-return-notifiers are really a per-cpu notifier, unrelated to any specific task. The use of per-task flags was an optimization.

If running into the MCE again is really bad, then you need something more, since other threads (or other processes) could run into the same page as well.

If not, do we care? Let it hit the MCE again, as long as we'll catch it eventually.

Quoted 4 lines

If running into the MCE again is really bad, then you need something more, since other threads (or other processes) could run into the same page as well.

Well, the #MC handler runs on all CPUs on Intel so what we could do is set the current task to TASK_STOPPED or _UNINTERRUPTIBLE or something that doesn't make it viable for scheduling anymore.

Then we can take our time running the notifier since the "problematic" task won't get scheduled until we're done. Then, when we finish analyzing the MCE, we either kill it so it has to handle SIGKILL the next time it gets scheduled or we unmap its page with error in it so that it #PFs on the next run.

But no, I don't think we can catch all possible situations where a page is mapped by multiple tasks ...

Quoted 2 lines

If not, do we care? Let it hit the MCE again, as long as we'll catch it eventually.

... and in that case we are going to have to let it hit again. Or is there a way to get to the tasklist of all the tasks mapping a page in atomic context, stop them from scheduling and run the notifier work in process context?

Hmmm..

Quoted 9 lines · Borislav Petkov

Quoted 4 lines

If running into the MCE again is really bad, then you need something more, since other threads (or other processes) could run into the same page as well.

Well, the #MC handler runs on all CPUs on Intel so what we could do is set the current task to TASK_STOPPED or _UNINTERRUPTIBLE or something that doesn't make it viable for scheduling anymore.

Quoted 5 lines

Then we can take our time running the notifier since the "problematic" task won't get scheduled until we're done. Then, when we finish analyzing the MCE, we either kill it so it has to handle SIGKILL the next time it gets scheduled or we unmap its page with error in it so that it #PFs on the next run.

If all cpus catch it, do we even know which task it is?

On the other hand, it makes user return notifiers attractive, since they are per-cpu, and combined with MCE broadcast that turns them into a global event.

Quoted 11 lines

But no, I don't think we can catch all possible situations where a page is mapped by multiple tasks ...

Quoted 2 lines

If not, do we care? Let it hit the MCE again, as long as we'll catch it eventually.

... and in that case we are going to have to let it hit again. Or is there a way to get to the tasklist of all the tasks mapping a page in atomic context, stop them from scheduling and run the notifier work in process context?

Quoted 1 line

Hmmm..

Surely not in atomic context, but you can use rmap to find all mappers of a given page.

So: MCE uses irq_work_queue() -> wake up a realtime task -> process the mce, unmap the page, go back to sleep.

Quoted 3 lines

So: MCE uses irq_work_queue() -> wake up a realtime task -> process the mce, unmap the page, go back to sleep.

Yes, this is basically it. However, the other cores cannot schedule a task which maps the compromized page until we haven't finished finding and 'fixing' all the mappers.

So we either hold off the cores from executing userspace - in that case no need to mark a task as unsuitable to run - or use the task return notifiers in patch 10/10.

HOWEVER, AFAICT, if the page is mapped multiple times, killing/recovering the current task doesn't help from another core touching it and causing a follow-up MCE. So holding off all the cores from scheduling userspace in some manner might be the superior solution. Especially if you don't execute the #MC handler on all CPUs as is the case on AMD.

Quoted 7 lines

HOWEVER, AFAICT, if the page is mapped multiple times, killing/recovering the current task doesn't help from another core touching it and causing a follow-up MCE. So holding off all the cores from scheduling userspace in some manner might be the superior solution. Especially if you don't execute the #MC handler on all CPUs as is the case on AMD.

That's basically impossible, since the other cores may be in fact executing userspace, with the next instruction accessing the bad page. In fact the access may have been started simultaneously with the one that triggered the #MC.

The best you can do is IPI everyone as soon as you've caught the #MC, but you have to be prepared for multiple #MC for the same page. Once you have that, global synchronization is not so important anymore.

Quoted 3 lines

The best you can do is IPI everyone as soon as you've caught the #MC, but you have to be prepared for multiple #MC for the same page. Once you have that, global synchronization is not so important anymore.

Yeah, in the multiple #MC case the memory_failure() thing should probably be made reentrant-safe (if it is not yet). And in that case, we'll be starting a worker thread on each CPU that caused an MCE from accessing that page. The thread that manages to clear all the mappings of our page simply does so while the others should be able to 'see' that there's no work to be done anymore (PFN is not mapped in the pagetables anymore) and exit without doing anything. Yeah, sounds doable with the irq_work_queue -> user_return_notifier flow.

I don't think a user_return_notifier is needed here.  You don't just want to do things before a userspace return, you also want to do them soon.  A user return notifier might take a very long time to run, if a context switch occurs to a thread that spends a lot of time in the kernel (perhaps a realtime thread).

So I think the best choice here is MCE -> irq_work -> realtime kernel thread (or work queue)

Quoted 8 lines · Avi Kivity

I don't think a user_return_notifier is needed here. You don't just want to do things before a userspace return, you also want to do them soon. A user return notifier might take a very long time to run, if a context switch occurs to a thread that spends a lot of time in the kernel (perhaps a realtime thread).

So I think the best choice here is MCE -> irq_work -> realtime kernel thread (or work queue)

In the AO (action optional case (e.g. patrol scrubber) - there isn't much rush. We'd like to process things "soon" (before someone hits the corrupt location) but we don't need to take extraordinary efforts to make "soon" happen.

In the AR (action required - instruction or data fetch from a corrupted memory location) our main priority is making sure that we don't continue the task that hit the error - because we don't want to hit it again (as Boris said, on Intel cpus this is very disruptive to the system as every cpu is sent the machine check signal - and the code has to read a large number of slow "msr" registers to figure out what happened. If we can guarantee that we won't run this task - then the time pressure is greatly reduced.

So if we can do:

MCE -> irq_work -> make-task-not-runnable -> thread-or-work-queue

in a reliable way, then that would meet the needs. PeterZ didn't like the idea of setting TASK_STOPPED or _UNINTERRUPTIBLE in NMI context in the MC handler - but I think he was okay with it inside the irq_work handler.

-Tony

+ * Called in process context that interrupted by MCE and marked with
+ * TIF_MCE_NOTFY, just before returning to errorneous userland.
+ * This code is allowed to sleep.
+ * Attempt possible recovery such as calling the high level VM handler to
+ * process any corrupted pages, and kill/signal current process if required. */

 void mce_notify_process(void)
 {
-       mce_notify_irq();
-       mce_memory_failure_process();
+       clear_thread_flag(TIF_MCE_NOTIFY);
+
+       /* TBD: do recovery for action required event */ }

need (failing address) to take some action. That was put into the ring

Quoted 2 lines · Tony Luck

need (failing address) to take some action. That was put into the ring

Correction - we only put the pfn into the ring for AO (action optional) errors [in the current state of the patch series].

But the need for a path to get the pfn from the MC handler to the mce_notify_process() function remains the same.

Or ... is it possible to push siginfo w/ addr and pop here?

Quoted 1 line · Hidetoshi Seto

Or ... is it possible to push siginfo w/ addr and pop here?

I chatted to Peter Anvin about this over lunch ... his suggestion was that since we know (for now) that the recovery case is always from user mode. We can let all the non-involved cpus return from do_machine_check() .. but catch the cpu with the problem and do a sideways stack jump from the machine check stack to the normal trap stack. At this point we'll be executing in a context that is effectively the same as a page fault - so we have plenty of safe options on functions we can call, locks we can take etc.

So perhaps we can change "void do_machine_check()" to "unsigned long do_machine_check()" and have the bystander cpus "return 0;" and the cpu that hit the error "return m.addr;" ... and then do the necessary magic in entry_64.S to leap from stack to stack in one mighty leap (and then onto a "handle_action_required(regs, addr)" function.

-Tony

I've made small patches to clear things. Could you take a look?

These are based on my cleanup patch set:
https://lkml.org/lkml/2011/6/7/677

Quoted 8 lines · Avi Kivity

I don't think a user_return_notifier is needed here. You don't just want to do things before a userspace return, you also want to do them soon. A user return notifier might take a very long time to run, if a context switch occurs to a thread that spends a lot of time in the kernel (perhaps a realtime thread).

So I think the best choice here is MCE -> irq_work -> realtime kernel thread (or work queue)

In the AO (action optional case (e.g. patrol scrubber) - there isn't much rush. We'd like to process things "soon" (before someone hits the corrupt location) but we don't need to take extraordinary efforts to make "soon" happen.

In the AR (action required - instruction or data fetch from a corrupted memory location) our main priority is making sure that we don't continue the task that hit the error - because we don't want to hit it again (as Boris said, on Intel cpus this is very disruptive to the system as every cpu is sent the machine check signal - and the code has to read a large number of slow "msr" registers to figure out what happened. If we can guarantee that we won't run this task - then the time pressure is greatly reduced.

So if we can do:

   MCE ->  irq_work ->  make-task-not-runnable ->  thread-or-work-queue

in a reliable way, then that would meet the needs. PeterZ didn't like the idea of setting TASK_STOPPED or _UNINTERRUPTIBLE in NMI context in the MC handler - but I think he was okay with it inside the irq_work handler.

Quoted 7 lines

Quoted 2 lines

So I think the best choice here is MCE -> irq_work -> realtime kernel thread (or work queue)

In the AO (action optional case (e.g. patrol scrubber) - there isn't much rush. We'd like to process things "soon" (before someone hits the corrupt location) but we don't need to take extraordinary efforts to make "soon" happen.

Quoted 7 lines

In the AR (action required - instruction or data fetch from a corrupted memory location) our main priority is making sure that we don't continue the task that hit the error - because we don't want to hit it again (as Boris said, on Intel cpus this is very disruptive to the system as every cpu is sent the machine check signal - and the code has to read a large number of slow "msr" registers to figure out what happened. If we can guarantee that we won't run this task - then the time pressure is greatly reduced.

Aren't these events extraordinarily rare? I think we can afford a little inefficiency there.

Even with mce -> irq_work -> rt thread, we're unlikely to return to the task as the rt thread will displace the task. It may be migrated to an idle cpu, but even then we may be able to drop the page before it gets back to userspace.

Quoted 5 lines

Even with mce -> irq_work -> rt thread, we're unlikely to return to the task as the rt thread will displace the task. It may be migrated to an idle cpu, but even then we may be able to drop the page before it gets back to userspace.

This doesn't give you the guarantee that the realtime task manages to unmap the page from all pagetables before another process running on another core accesses it.

I think your previous suggestion of making the memory failure handling code reentrant would cover all holes.

Even marking all processes mapping a faulty page STOPPED or UNINTERRUPTIBLE won't work in all cases since you have to go out and find which those processes are. And this is what the rt thread will do.

Since we can't have nested #MC (due to the IST mechanism resetting %rsp and clobbering the previous invocation's stack), we have to clear MCIP outside the #MC handler. And that means irq_work_queue()

(note that this changes the behaviour from memory corruption to shutdown state; both suck, but one more than the other).

Quoted 6 lines

Since we can't have nested #MC (due to the IST mechanism resetting %rsp and clobbering the previous invocation's stack), we have to clear MCIP outside the #MC handler. And that means irq_work_queue()

(note that this changes the behaviour from memory corruption to shutdown state; both suck, but one more than the other).

Hmm. We currently clear MCIP inside the handler (at the very end, but still inside). Deferring clearing would make sense - but we'd have to be sure to do so a.s.a.p on all processors. It also would change how we look at the possibility of a process being able to run and re-execute an instruction that causes a machine check. If we defer clearing MCIP this won't just be annoying, it will be fatal (if any cpu hasn't yet got to the work queue to clear MCIP).

Aren't these events extraordinarily rare? I think we can afford a little inefficiency there.

I don't think that doing anything to the task is correct, though, as the problem is with the page, not the task itself. In fact if the task is executing a vgather instruction, or if another thread munmap()s the page, it may not hit the same page again when re-executed.

Quoted 4 lines

I don't think that doing anything to the task is correct, though, as the problem is with the page, not the task itself. In fact if the task is executing a vgather instruction, or if another thread munmap()s the page, it may not hit the same page again when re-executed.

True the memory is the source of the problem - but the task is intimately affected. Time for a car analogy :-) ...

You are driving along the road when you notice a giant hole. You hit the brakes and stop on the very edge. The problem is with the road, not with your car. But I don't think you want to start driving again (at least not in the forward direction!)

Well, in the ActionRequired case, the error is obviously reported through a #MC exception meaning that the core definitely generates the MCE before we've made a context switch (CR3 change etc) so in that case 'current' will point to the task at fault.

The problem is finding which 'current' it is, from all the tasks running on all cores when the #MC is raised. Tony, can you tell from the hw which core actually caused the MCE? Is it the monarch, so to speak?

I'm thinking that in cases were we have a page shared by multiple processes, we still would want to run a 'main' user return notifier on one core which does the rmap lookup _but_, _also_ very importantly, the other cores still hold off from executing userspace until that main notifier hasn't finished finding out how big the fallout is,i.e. how many other processes would run into the same page.

From: Tony Luck <[email protected]>

Instead of letting cpus run through the MC bank scanning code in the order that they turned up in the handler, we arrange to deal with those that have more severe problems (mcgstatus.ripv=0) first. This will make life simpler in the case that banks are shared between processors, since the cpu with the problem will see it and clear it, leaving the other cpu(s) that share the bank with nothing to do.

Signed-off-by Tony Luck <[email protected]>

---

arch/x86/kernel/cpu/mcheck/mce.c +6 -0

+/*
+ * next cpu choosing first from cant_return, and then from can_return
+ */
+int mce_nextcpu(int this)
+{
+	int next;

Well, I agree about the point that reordering is required to handle shared banks.

I think it is better to put this change in early of the series.

I don't like to have multiple cpumasks here, notably one is just an inversion of another...

How about using severity-leveling?
Pick cpus with PANIC level first, then AR, AO ...

Or how about checking rip in each mces_seen?

Quoted 1 line

Or how about checking rip in each mces_seen?

This is equivalent to what I did - but I think the code will be cleaner. I'll give it a try.

Quoted 4 lines

Quoted 1 line

Or how about checking rip in each mces_seen?

This is equivalent to what I did - but I think the code will be cleaner. I'll give it a try.

Here's a patch on top of my previous series that just looks at mces_seen to choose the order. Obviously I'd fold this into the other patch for a final version - but this one lets you see what the "mce_nextcpu()" function would look like (and how removing the bitmaps cleans up the other parts of the code). It does look better to me.

Seto-san: Does this fit with what you were thinking?

Compile tested only.

(quoted diff) +0 -2

-Tony
 
---

arch/x86/kernel/cpu/mcheck/mce.c +0 -8

index a7a8c53..6b4176b 100644
@@ -791,31 +791,47 @@ static void mce_reign(void)
 
 static atomic_t global_nwo;
 
-/*
- * Keep separate bitmaps for cpus that have the option return from
- * machine check handler (MCG_STATUS.RIPV == 1) and those for that
- * cannot.
- */
-static cpumask_t	can_return;
-static cpumask_t	cant_return;
-
 static int	monarch;
 
 /*

arch/x86/kernel/cpu/mcheck/mce.c +0 -4

-	if (this == -1 || cpumask_test_cpu(this, &cant_return)) {
-		next = cpumask_next(this, &cant_return);
-		if (next >= nr_cpu_ids)
-			next = cpumask_next(-1, &can_return);

arch/x86/kernel/cpu/mcheck/mce.c +15 -8

+		if (cpu >= nr_cpu_ids) {
+			if (mask & MCG_STATUS_RIPV)
+				return cpu;
+			mask |= MCG_STATUS_RIPV;
+			cpu = -1;
+			continue;
+		}
+
+		m = &per_cpu(mces_seen, cpu);
+		if ((m->mcgstatus & (MCG_STATUS_MCIP|MCG_STATUS_RIPV)) == mask)
+			break;
+	}
+
+	return cpu;
 }
 
 /*

@@ -825,7 +841,7 @@ int mce_nextcpu(int this)
  * one at a time.
  * TBD double check parallel CPU hotunplug
  */
-static int mce_start(int *no_way_out, int noreturn)
+static int mce_start(int *no_way_out)
 {
 	int order;
 	int cpus = num_online_cpus();

@@ -841,11 +857,6 @@ static int mce_start(int *no_way_out, int noreturn)
 	smp_wmb();
 	order = atomic_inc_return(&mce_callin);
 
-	if (noreturn)
-		cpumask_set_cpu(smp_processor_id(), &cant_return);
-	else
-		cpumask_set_cpu(smp_processor_id(), &can_return);
-
 	/*
 	 * Wait for everyone.
 	 */

@@ -951,8 +962,6 @@ static int mce_end(int order)
 reset:
 	atomic_set(&global_nwo, 0);
 	atomic_set(&mce_callin, 0);
-	cpumask_clear(&can_return);
-	cpumask_clear(&cant_return);
 	barrier();
 
 	/*

BTW in case of "no_way_out" events, we don't clear banks because they could be carried over to the next boot (expecting logged as bootlog). So we may need to have some trick for some known cases; e.g. ignore observed AR by bystanders, anyway.

Quoted 1 line

+     u64     mask = MCG_STATUS_MCIP;

Why do you check the MCG_STATUS_MCIP too here?
What happens if there is a problematic cpu that could not read MCG register properly so indicates "PANIC with !MCIP"?

Quoted 1 line

+             cpu = cpumask_next(cpu, cpu_possible_mask);

possible? online?

Ah, I guess you assumed that all cpus checked in should have mces_seen with MCIP while offline cpus have cleaned mces_seen.

Though we know there might be races with cpu hotplug, now we already use num_online_cpus() in this rendezvous mechanism, it is OK to use cpu_online_mask here at the moment, I think.

Or we should invent new, better rendezvous mechanism...

From: Tony Luck <[email protected]>

Implement core MCA recovery. This is used for errors that happen in the current execution context.

The kernel has to first pass the error information to a function running on the current process stack. This is done using task_return_notifier_register().

Just handle errors in user mode for now. Later we may be able to handle some kernel cases (e.g. when kernel is in copy_*_user())

Based on some original code by Andi Kleen.

Signed-off-by Tony Luck <[email protected]>

---
 arch/x86/kernel/cpu/mcheck/mce-severity.c |   35 +++++++-
 arch/x86/kernel/cpu/mcheck/mce.c          |  118 +++++++++++++++++++++++++++--
 2 files changed, 142 insertions(+), 11 deletions(-)

arch/x86/kernel/cpu/mcheck/mce-severity.c +15 -3

index 352d16a..fe8a28c 100644
@@ -13,6 +13,7 @@
 #include <linux/seq_file.h>
 #include <linux/init.h>
 #include <linux/debugfs.h>
+#include <linux/module.h>
 #include <asm/mce.h>
 
 #include "mce-internal.h"

@@ -54,6 +55,9 @@ static struct severity {
 	{ .mcgmask = x, .mcgres = res, SEV(s), .msg = m, ## r }
 #define MASK(x, y, s, m, r...) \
 	{ .mask = x, .result = y, SEV(s), .msg = m, ## r }
+#define ARMASK(x, y, s, m, r...) \
+	{ .mcgmask = MCG_STATUS_RIPV, .mcgres = 0, \
+	  .mask = x, .result = y, SEV(s), .msg = m, ## r }
 #define MCI_UC_S (MCI_STATUS_UC|MCI_STATUS_S)
 #define MCI_UC_SAR (MCI_STATUS_UC|MCI_STATUS_S|MCI_STATUS_AR)
 #define MCACOD 0xffff

@@ -67,7 +71,7 @@ static struct severity {
 	MCGMASK(MCG_STATUS_RIPV|MCG_STATUS_EIPV, 0, PANIC,
 		"Neither restart nor error IP"),
 	MCGMASK(MCG_STATUS_RIPV, 0, PANIC, "In kernel and no restart IP",
-		KERNEL),
+		KERNEL, NOSER),
 	BITCLR(MCI_STATUS_UC, KEEP, "Corrected error", NOSER),
 
 	/* ignore OVER for UCNA */

@@ -77,10 +81,16 @@ static struct severity {
 	     "Illegal combination (UCNA with AR=1)", SER),
 	MASK(MCI_STATUS_S, 0, KEEP, "Non signalled machine check", SER),
 
-	/* AR add known MCACODs here */
 	MASK(MCI_STATUS_OVER|MCI_UC_SAR, MCI_STATUS_OVER|MCI_UC_SAR, PANIC,
 	     "Action required with lost events", SER),
-	MASK(MCI_STATUS_OVER|MCI_UC_SAR, MCI_UC_SAR, PANIC,
+
+	/* known AR MCACODs: */
+	ARMASK(MCI_UC_SAR|MCI_STATUS_OVER|0xffff, MCI_UC_SAR|0x134, AR,
+	     "Action required: data load error", SER),
+	ARMASK(MCI_UC_SAR|MCI_STATUS_OVER|0xffff, MCI_UC_SAR|0x150, AR,
+	     "Action required: instruction fetch error", SER),
+
+	ARMASK(MCI_STATUS_OVER|MCI_UC_SAR, MCI_UC_SAR, PANIC,
 	     "Action required; unknown MCACOD", SER),
 
 	/* known AO MCACODs: */

@@ -89,6 +99,7 @@ static struct severity {
 	MASK(MCI_UC_SAR|MCI_STATUS_OVER|MCACOD, MCI_UC_S|0x17a, AO,
 	     "Action optional: last level cache writeback error", SER),
 
+
 	MASK(MCI_STATUS_OVER|MCI_UC_SAR, MCI_UC_S, SOME,
 	     "Action optional unknown MCACOD", SER),
 	MASK(MCI_STATUS_OVER|MCI_UC_SAR, MCI_UC_S|MCI_STATUS_OVER, SOME,

@@ -110,6 +121,17 @@ static int error_context(struct mce *m)
 	return IN_KERNEL;
 }
 
+static int kernel_ar_recoverable(struct mce *m, int tolerant)

arch/x86/kernel/cpu/mcheck/mce-severity.c +5 -2

 int mce_severity(struct mce *a, int tolerant, char **msg)
 {
 	enum context ctx = error_context(a);

@@ -129,9 +151,12 @@ int mce_severity(struct mce *a, int tolerant, char **msg)
 		if (msg)
 			*msg = s->msg;
 		s->covered = 1;
-		if (s->sev >= MCE_UC_SEVERITY && ctx == IN_KERNEL) {
-			if (panic_on_oops || tolerant < 1)
+		if (ctx == IN_KERNEL) {
+			if (s->sev >= MCE_UC_SEVERITY &&
+				(panic_on_oops || tolerant < 1))
 				return MCE_PANIC_SEVERITY;
+			if (s->sev == MCE_AR_SEVERITY)
+				return kernel_ar_recoverable(a, tolerant);
 		}
 		return s->sev;
 	}

arch/x86/kernel/cpu/mcheck/mce.c +24 -0

index 9c72245..a7a8c53 100644
@@ -80,6 +80,20 @@ static void mce_do_notify(struct user_return_notifier *urn);
 static DEFINE_PER_CPU(struct mce_notify, mce_notify);
 
 /*
+ * Task return notifiers are used for "action required"
+ * recovery of tasks - i.e. we prevent return to the task
+ * that encountered the machine check, but we ensure that
+ * we process the error in task context.
+ */
+struct task_notify {
+	struct user_return_notifier urn;
+	unsigned	long pfn;
+	atomic_t	inuse;
+};
+static struct task_notify task_notifier[NR_CPUS];
+static void mce_do_task(struct user_return_notifier *urn);
+
+/*
  * Tolerant levels:
  *   0: always panic on uncorrected errors, log corrected errors
  *   1: panic or SIGBUS on uncorrected errors, log corrected errors

@@ -975,6 +989,84 @@ static void mce_clear_state(unsigned long *toclear)
 	}
 }
 
+/* Stub when hwpoison is not compiled in */
+int __attribute__((weak)) __memory_failure(unsigned long pfn, int vector,
+					   int precount)
+{
+	return -1;
+}
+
+/*
+ * Uncorrected error for current process.
+ */

arch/x86/kernel/cpu/mcheck/mce.c +14 -0

+
+	for (i = 0; i < NR_CPUS; i++)
+		if (!atomic_cmpxchg(&task_notifier[i].inuse, 0, 1))
+			break;
+	if (i == NR_CPUS)
+		mce_panic("Too many concurrent errors", m, msg);
+
+	task_notifier[i].urn.on_user_return = mce_do_task;
+	task_notifier[i].pfn = m->addr >> PAGE_SHIFT;
+	task_return_notifier_register(&task_notifier[i].urn);
+}
+
+#undef pr_fmt
+#define pr_fmt(x) "MCE: %s:%d " x "\n", current->comm, current->pid

arch/x86/kernel/cpu/mcheck/mce.c +34 -6

+
+	/*
+	 * For some reason hwpoison wasn't able to send a proper
+	 * SIGBUS.  Send a fallback signal. Unfortunately we don't
+	 * know the virtual address here, so can't tell the program
+	 * details.
+	 */
+	force_sig(SIGBUS, me);
+	pr_err("Killed due to action-required memory corruption");
+}
+
+/*
+ * Handle action-required on the process stack.  hwpoison does the
+ * bulk of the work and with some luck might even be able to fix the
+ * problem.
+ *
+ * Logic changes here should be reflected in kernel_ar_recoverable().
+ */
+static void handle_action_required(unsigned long pfn)
+{
+	struct task_struct *me = current;
+
+	pr_err("Uncorrected hardware memory error in user-access at %llx",
+	       PADDR(pfn));
+	if (__memory_failure(pfn, MCE_VECTOR, 0) < 0) {
+		pr_err("Memory error not recovered");
+		ar_fallback(me, pfn);
+	} else
+		pr_err("Memory error recovered");
+}
+
+#undef pr_fmt
+#define pr_fmt(x) x
+
 /*
  * The actual machine check handler. This only handles real
  * exceptions when something got corrupted coming in through int 18.

@@ -1086,12 +1178,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);
 
 		/*

arch/x86/kernel/cpu/mcheck/mce.c +15 -0

+		kill_it = 0;
+	}
+
+	/*
 	 * If the error seems to be unrecoverable, something should be
 	 * done.  Try to kill as little as possible.  If we can kill just
 	 * one task, do that.  If the user has set the tolerance very

@@ -1194,6 +1289,17 @@ static void mce_do_notify(struct user_return_notifier *urn)
 	mce_process_ring();
 }
 
+static void mce_do_task(struct user_return_notifier *urn)
+{
+	struct task_notify *np = container_of(urn, struct task_notify, urn);
+	unsigned long pfn = np->pfn;
+
+	task_return_notifier_unregister(urn);
+	atomic_set(&np->inuse, 0);
+
+	handle_action_required(pfn);
+}
+
 static void mce_process_work(struct work_struct *dummy)
 {
 	mce_process_ring();

Quoted 5 lines

arch/x86/kernel/cpu/mcheck/mce-severity.c +5 -0

+static int kernel_ar_recoverable(struct mce *m, int tolerant)
+{
+     if (tolerant >= 2)
+             return MCE_AR_SEVERITY;
+     if (!(m->mcgstatus & MCG_STATUS_EIPV) || !m->ip)

Review

Quoted 6 lines

+             return MCE_PANIC_SEVERITY;
+     if (search_exception_tables(m->ip))
+             return MCE_AR_SEVERITY;
+     return MCE_PANIC_SEVERITY;

(quoted diff) +2 -0

+}
+

You said "Just handle errors in user mode for now." but ...?

Quoted 6 lines

+     if (!mce_usable_address(m))
+             mce_panic("No address for Action-Required Machine Check",
+                       m, msg);
+     if (!(m->mcgstatus & MCG_STATUS_EIPV))
+             mce_panic("No EIPV for Action-Required Machine Check",
+                       m, msg);

When can this happen?

Why not create new severity {PANIC, "Action Required: but No EIPV", ...} in severity table?

arch/x86/kernel/cpu/mcheck/mce.c static void mce_clear_state(unsigned long *toclear) · line 1035

Quoted 4 lines

arch/x86/kernel/cpu/mcheck/mce.c +4 -0

+static void ar_fallback(struct task_struct *me, unsigned long pfn)
+{
+     if (signal_pending(me) && sigismember(&me->pending.signal, SIGBUS))
+             return;

Is it safe for _AR if SIGBUS is pending but blocked? I think force_sig() is reasonable for such situation.

Quoted 2 lines

+     if (worst == MCE_AR_SEVERITY) {
+             mce_action_required(&m, msg, regs);

Comprehensible name would be appreciated, e.g.:
               mce_request_dpc_for_action_required(pfn);

And if we cannot request context for recovery, it is better to suppress trailing attempts, e.g. before mce_end():

       if (!no_way_out && severity == MCE_AR_SEVERITY) { err = mce_request_dpc_for_action_required(pfn); if (err) {
                       atomic_inc(&global_nwo);
                       severity = MCE_PANIC_SEVERITY;  /* escalated */ }
       }