Messages in this thread | | | Date | Tue, 24 Nov 2015 10:05:10 -0500 | From | Steven Rostedt <> | Subject | Re: [V5 PATCH 1/4] panic/x86: Fix re-entrance problem due to panic on NMI |
| |
On Fri, Nov 20, 2015 at 06:36:44PM +0900, Hidehiro Kawai wrote: > diff --git a/include/linux/kernel.h b/include/linux/kernel.h > index 350dfb0..480a4fd 100644 > --- a/include/linux/kernel.h > +++ b/include/linux/kernel.h > @@ -445,6 +445,19 @@ extern int sysctl_panic_on_stackoverflow; > > extern bool crash_kexec_post_notifiers; > > +extern atomic_t panic_cpu; > + > +/* > + * A variant of panic() called from NMI context. > + * If we've already panicked on this cpu, return from here. > + */ > +#define nmi_panic(fmt, ...) \ > + do { \ > + int this_cpu = raw_smp_processor_id(); \ > + if (atomic_cmpxchg(&panic_cpu, -1, this_cpu) != this_cpu) \ > + panic(fmt, ##__VA_ARGS__); \
Hmm,
What happens if:
CPU 0: CPU 1: ------ ------ nmi_panic();
nmi_panic(); <external nmi> nmi_panic();
?
cmpxchg(&panic_cpu, -1, 0) != 0
returns -1 for cpu 0, thus 0 != 0, and sets panic_cpu to 0
cmpxchg(&panic_cpu, -1, 1) != 1
returns 0, and then it too panics, but does not set panic_cpu to 1
Now you have your external NMI triggering on CPU 1
cmpxchg(&panic_cpu, -1, 1) != 1
returns 0 again, and you call panic again within the panic of CPU 1.
Is this OK?
Perhaps you want a per cpu bitmask, and do a test_and_set() on the CPU. That would prevent any CPU from rerunning a panic() twice on any CPU.
-- Steve
> + } while (0) > + > /* > * Only to be used by arch init code. If the user over-wrote the default > * CONFIG_PANIC_TIMEOUT, honor it. > diff --git a/kernel/panic.c b/kernel/panic.c > index 4579dbb..24ee2ea 100644
| |