Re: [PATCH] x86_64: mcelog tolerant level cleanup
On 5/18/07, Andi Kleen <[EMAIL PROTECTED]> wrote: > * If RIPV is set it is not safe to restart, so set the 'no way out' >flag rather than the 'kill it' flag. Why? It is not PCC. We cannot return of course, but killing isn't returning. My understanding is that the absence of RIPV indicates that it is not safe to restart, period. Not that the running *task* is not safe* but that the IP on the stack is not valid to restart at all. > * Don't panic() on correctable MCEs. The idea behind this was that if you get an exception it is always a bit risky because there are a few potential deadlocks that cannot be avoided. And normally non UC is just polled which will never cause a panic. So I don't quite see the value of this change. It will still always panic when tolerant == 0, and of course you're right correctable errors would skip over the panic() path anyway. I can roll back the "<0" part, though I don't see the difference now :) > This patch also calls nonseekable_open() in mce_open (as suggested by akpm). That should be a separate patch Andrew already sucked it into -mm - do you want me to break it out, and re-submit? > + 0: always panic on uncorrected errors, log corrected errors > + 1: panic or SIGBUS on uncorrected errors, log corrected errors > + 2: SIGBUS or log uncorrected errors, log corrected errors Just saying SIGBUS is misleading because it isn't a catchable signal. should I change that to "kill" ? Why did you remove the idle special case? Because once the other tolerant rules are clarified, it's redundant for tolerant < 2, and I think it's a bad special case for tolerant == 2, and it's definately wrong for tolerant == 3. Shall I re-roll? - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86_64: mcelog tolerant level cleanup
On Wednesday 16 May 2007 22:29, Tim Hockin wrote:
> From: Tim Hockin <[EMAIL PROTECTED]>
>
> Background:
> The MCE handler has several paths that it can take, depending on various
> conditions of the MCE status and the value of the 'tolerant' knob. The
> exact semantics are not well defined and the code is a bit twisty.
>
> Description:
> This patch makes the MCE handler's behavior more clear by documenting the
> behavior for various 'tolerant' levels. It also fixes or enhances
> several small things in the handler. Specifically:
> * If RIPV is set it is not safe to restart, so set the 'no way out'
>flag rather than the 'kill it' flag.
Why? It is not PCC. We cannot return of course, but killing isn't returning.
> * Don't panic() on correctable MCEs.
The idea behind this was that if you get an exception it is always a bit risky
because there are a few potential deadlocks that cannot be avoided.
And normally non UC is just polled which will never cause a panic.
So I don't quite see the value of this change.
> This patch also calls nonseekable_open() in mce_open (as suggested by akpm).
That should be a separate patch
> + 0: always panic on uncorrected errors, log corrected errors
> + 1: panic or SIGBUS on uncorrected errors, log corrected errors
> + 2: SIGBUS or log uncorrected errors, log corrected errors
Just saying SIGBUS is misleading because it isn't a catchable
signal.
> +
> + /*
> + * 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
> + * high, don't try to do anything at all.
> + */
> + if (kill_it && tolerant < 3) {
> int user_space = 0;
>
> - if (m.mcgstatus & MCG_STATUS_RIPV)
> + /*
> + * If the EIPV bit is set, it means the saved IP is the
> + * instruction which caused the MCE.
> + */
> + if (m.mcgstatus & MCG_STATUS_EIPV)
> user_space = panicm.rip && (panicm.cs & 3);
> -
> - /* When the machine was in user space and the CPU didn't get
> -confused it's normally not necessary to panic, unless you
> -are paranoid (tolerant == 0)
> -
> -RED-PEN could be more tolerant for MCEs in idle,
> -but most likely they occur at boot anyways, where
> -it is best to just halt the machine. */
> - if ((!user_space && (panic_on_oops || tolerant < 2)) ||
> - (unsigned)current->pid <= 1)
> - mce_panic("Uncorrected machine check", &panicm,
> mcestart);
> -
> - /* do_exit takes an awful lot of locks and has as
> -slight risk of deadlocking. If you don't want that
> -don't set tolerant >= 2 */
> - if (tolerant < 3)
> +
> + /*
> + * If we know that the error was in user space, send a
> + * SIGBUS. Otherwise, panic if tolerance is low.
> + *
> + * do_exit() takes an awful lot of locks and has a slight
> + * risk of deadlocking.
> + */
> + if (user_space) {
> do_exit(SIGBUS);
> + } else if (panic_on_oops || tolerant < 2) {
> + mce_panic("Uncorrected machine check",
> + &panicm, mcestart);
> + }
Why did you remove the idle special case?
-Andi
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] x86_64: mcelog tolerant level cleanup
From: Tim Hockin <[EMAIL PROTECTED]> Background: The MCE handler has several paths that it can take, depending on various conditions of the MCE status and the value of the 'tolerant' knob. The exact semantics are not well defined and the code is a bit twisty. Description: This patch makes the MCE handler's behavior more clear by documenting the behavior for various 'tolerant' levels. It also fixes or enhances several small things in the handler. Specifically: * If RIPV is set it is not safe to restart, so set the 'no way out' flag rather than the 'kill it' flag. * Don't panic() on correctable MCEs. * If the _OVER bit is set *and* the _UC bit is set (meaning possibly dropped uncorrected errors), set the 'no way out' flag. * Use EIPV for testing whether an app can be killed (SIGBUS) rather than RIPV. According to docs, EIPV indicates that the error is related to the IP, while RIPV simply means the IP is valid to restart from. * Don't clear the MCi_STATUS registers until after the panic() path. This leaves the status bits set after the panic() so clever BIOSes can find them (and dumb BIOSes can do nothing). This patch also calls nonseekable_open() in mce_open (as suggested by akpm). Result: Tolerant levels behave almost identically to how they always have, but not it's well defined. There's a slightly higher chance of panic()ing when multiple errors happen (a good thing, IMHO). If you take an MBE and panic(), the error status bits are not cleared. Alternatives: None. Testing: I used software to inject correctable and uncorrectable errors. With tolerant = 3, the system usually survives. With tolerant = 2, the system usually panic()s (PCC) but not always. With tolerant = 1, the system always panic()s. When the system panic()s, the BIOS is able to detect that the cause of death was an MC4. I was not able to reproduce the case of a non-PCC error in userspace, with EIPV, with (tolerant < 3). That will be rare at best. Patch: This patch is against 2.6.21-mm. Signed-off-by: Tim Hockin <[EMAIL PROTECTED]> --- This is the first version of this patch. diff -pruN linux-2.6.21+01+02+03/Documentation/x86_64/boot-options.txt linux-2.6.21+04/Documentation/x86_64/boot-options.txt --- linux-2.6.21+01+02+03/Documentation/x86_64/boot-options.txt 2007-04-25 20:08:32.0 -0700 +++ linux-2.6.21+04/Documentation/x86_64/boot-options.txt 2007-05-09 23:29:01.0 -0700 @@ -14,9 +14,11 @@ Machine check mce=nobootlog Disable boot machine check logging. mce=tolerancelevel (number) - 0: always panic, 1: panic if deadlock possible, - 2: try to avoid panic, 3: never panic or exit (for testing) - default is 1 + 0: always panic on uncorrected errors, log corrected errors + 1: panic or SIGBUS on uncorrected errors, log corrected errors + 2: SIGBUS or log uncorrected errors, log corrected errors + 3: never panic or SIGBUS, log all errors (for testing only) + Default is 1 Can be also set using sysfs which is preferable. nomce (for compatibility with i386): same as mce=off diff -pruN linux-2.6.21+01+02+03/Documentation/x86_64/machinecheck linux-2.6.21+04/Documentation/x86_64/machinecheck --- linux-2.6.21+01+02+03/Documentation/x86_64/machinecheck 2007-05-07 12:08:26.0 -0700 +++ linux-2.6.21+04/Documentation/x86_64/machinecheck 2007-05-09 23:29:16.0 -0700 @@ -49,12 +49,14 @@ tolerant Since machine check exceptions can happen any time it is sometimes risky for the kernel to kill a process because it defies normal kernel locking rules. The tolerance level configures - how hard the kernel tries to recover even at some risk of deadlock. - - 0: always panic, - 1: panic if deadlock possible, - 2: try to avoid panic, - 3: never panic or exit (for testing only) + how hard the kernel tries to recover even at some risk of + deadlock. Higher tolerant values trade potentially better uptime + with the risk of a crash or even corruption (for tolerant >= 3). + + 0: always panic on uncorrected errors, log corrected errors + 1: panic or SIGBUS on uncorrected errors, log corrected errors + 2: SIGBUS or log uncorrected errors, log corrected errors + 3: never panic or SIGBUS, log all errors (for testing only) Default: 1 diff -pruN linux-2.6.21+01+02+03/arch/x86_64/kernel/mce.c linux-2.6.21+04/arch/x86_64/kernel/mce.c --- linux-2.6.21+01+02+03/arch/x86_64/kernel/mce.c 2007-05-09 22:05:48.0 -0700 +++ linux-2.6.21+04/arch/x86_64/kernel/mce.c2007-05-11 21:02:12.0 -0700 @@ -37,8 +37,13 @@ atomic_t mce_entry; static int mce_dont_init; -/* 0: always panic, 1: panic if deadlock possible, 2: try to avoid panic, - 3: neve
