Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1193194 > unrolled thread

Re: [V2 PATCH 1/3] x86/panic: Fix re-entrance problem due to panic on NMI

Started byMichal Hocko <mhocko@kernel.org>
First post2015-07-27 16:40 +0200
Last post2015-08-04 14:00 +0200
Articles 13 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: [V2 PATCH 1/3] x86/panic: Fix re-entrance problem due to panic  on NMI Michal Hocko <mhocko@kernel.org> - 2015-07-27 16:40 +0200
    Re: [V2 PATCH 1/3] x86/panic: Fix re-entrance problem due to panic  on NMI Michal Hocko <mhocko@kernel.org> - 2015-07-28 10:10 +0200
    RE: Re: [V2 PATCH 1/3] x86/panic: Fix re-entrance problem due to  panic on NMI 河合英宏 / KAWAI,HIDEHIRO   <hidehiro.kawai.ez@hitachi.com> - 2015-07-29 07:50 +0200
      Re: Re: [V2 PATCH 1/3] x86/panic: Fix re-entrance problem due to  panic on NMI Michal Hocko <mhocko@kernel.org> - 2015-07-29 10:30 +0200
        RE: Re: [V2 PATCH 1/3] x86/panic: Fix re-entrance problem due to  panic on NMI 河合英宏 / KAWAI,HIDEHIRO   <hidehiro.kawai.ez@hitachi.com> - 2015-07-29 11:10 +0200
          Re: Re: [V2 PATCH 1/3] x86/panic: Fix re-entrance problem due to  panic on NMI Michal Hocko <mhocko@kernel.org> - 2015-07-29 11:30 +0200
            RE: Re: [V2 PATCH 1/3] x86/panic: Fix re-entrance problem due to  panic on NMI 河合英宏 / KAWAI,HIDEHIRO   <hidehiro.kawai.ez@hitachi.com> - 2015-07-30 03:50 +0200
              RE: Re: [V2 PATCH 1/3] x86/panic: Fix re-entrance problem due to  panic on NMI 河合英宏 / KAWAI,HIDEHIRO   <hidehiro.kawai.ez@hitachi.com> - 2015-07-30 09:40 +0200
                Re: Re: [V2 PATCH 1/3] x86/panic: Fix re-entrance problem due to  panic on NMI Michal Hocko <mhocko@kernel.org> - 2015-07-30 10:00 +0200
                  RE: Re: [V2 PATCH 1/3] x86/panic: Fix re-entrance problem due to  panic on NMI 河合英宏 / KAWAI,HIDEHIRO   <hidehiro.kawai.ez@hitachi.com> - 2015-07-30 10:10 +0200
              Re: Re: [V2 PATCH 1/3] x86/panic: Fix re-entrance problem due to  panic on NMI Michal Hocko <mhocko@kernel.org> - 2015-07-30 09:50 +0200
                Re: Re: [V2 PATCH 1/3] x86/panic: Fix re-entrance problem due to  panic on NMI Michal Hocko <mhocko@kernel.org> - 2015-08-04 11:20 +0200
                  RE: Re: [V2 PATCH 1/3] x86/panic: Fix re-entrance problem due to  panic on NMI 河合英宏 / KAWAI,HIDEHIRO   <hidehiro.kawai.ez@hitachi.com> - 2015-08-04 14:00 +0200

#1193194 — Re: [V2 PATCH 1/3] x86/panic: Fix re-entrance problem due to panic on NMI

FromMichal Hocko <mhocko@kernel.org>
Date2015-07-27 16:40 +0200
SubjectRe: [V2 PATCH 1/3] x86/panic: Fix re-entrance problem due to panic on NMI
Message-ID<pQRJn-4Ft-15@gated-at.bofh.it>
On Mon 27-07-15 10:58:50, Hidehiro Kawai wrote:
[...]
> diff --git a/arch/x86/kernel/nmi.c b/arch/x86/kernel/nmi.c
> index d05bd2e..5b32d81 100644
> --- a/arch/x86/kernel/nmi.c
> +++ b/arch/x86/kernel/nmi.c
> @@ -230,7 +230,8 @@ void unregister_nmi_handler(unsigned int type, const char *name)
>  	}
>  #endif
>  
> -	if (panic_on_unrecovered_nmi)
> +	if (panic_on_unrecovered_nmi &&
> +	    atomic_cmpxchg(&panicking_cpu, -1, raw_smp_processor_id()) == -1)
>  		panic("NMI: Not continuing");

Spreading the check to all NMI callers is quite ugly. Wouldn't it be
better to introduce nmi_panic() which wouldn't be __noreturn unlike the
regular panic. The check could be also relaxed a bit and nmi_panic would
return only if the ongoing panic is the current cpu when we really have
to return and allow the preempted panic to finish.

Something like
---
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 5582410727cb..409091c48e6c 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -253,6 +253,7 @@ static inline void might_fault(void) { }
 extern struct atomic_notifier_head panic_notifier_list;
 extern long (*panic_blink)(int state);
 __printf(1, 2)
+void nmi_panic(const char *fmt, ...) __cold;
 void panic(const char *fmt, ...)
 	__noreturn __cold;
 extern void oops_enter(void);
diff --git a/kernel/panic.c b/kernel/panic.c
index 04e91ff7560b..4c1ff7e19cdc 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -60,6 +60,8 @@ void __weak panic_smp_self_stop(void)
 		cpu_relax();
 }
 
+static atomic_t panic_cpu = ATOMIC_INIT(-1);
+
 /**
  *	panic - halt the system
  *	@fmt: The text string to print
@@ -70,11 +72,11 @@ void __weak panic_smp_self_stop(void)
  */
 void panic(const char *fmt, ...)
 {
-	static DEFINE_SPINLOCK(panic_lock);
 	static char buf[1024];
 	va_list args;
 	long i, i_next = 0;
 	int state = 0;
+	int this_cpu, old_cpu;
 
 	/*
 	 * Disable local interrupts. This will prevent panic_smp_self_stop
@@ -94,7 +96,9 @@ void panic(const char *fmt, ...)
 	 * stop themself or will wait until they are stopped by the 1st CPU
 	 * with smp_send_stop().
 	 */
-	if (!spin_trylock(&panic_lock))
+	this_cpu = raw_smp_processor_id();
+	old_cpu = atomic_cmpxchg(&panic_cpu, -1, this_cpu);
+	if (old_cpu != -1 && old_cpu != this_cpu)
 		panic_smp_self_stop();
 
 	console_verbose();
@@ -201,9 +205,20 @@ void panic(const char *fmt, ...)
 		mdelay(PANIC_TIMER_STEP);
 	}
 }
-
 EXPORT_SYMBOL(panic);
 
+void nmi_panic(const char *fmt, ...)
+{
+	/*
+	 * We have to back off if the NMI has preempted an ongoing panic and
+	 * allow it to finish
+	 */
+	if (atomic_read(&panic_cpu) == raw_smp_processor_id())
+		return;
+
+	panic();
+}
+EXPORT_SYMBOL(nmi_panic);
 
 struct tnt {
 	u8	bit;
-- 
Michal Hocko
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]


#1193850

FromMichal Hocko <mhocko@kernel.org>
Date2015-07-28 10:10 +0200
Message-ID<pR87w-3jY-15@gated-at.bofh.it>
In reply to#1193194
On Tue 28-07-15 11:02:11, Hidehiro Kawai wrote:
[...]
> > Something like
> [...]
> > +void nmi_panic(const char *fmt, ...)
> 
> Since we can't directly pass variable arguments to a subroutine,

Sure, I was just too lazy to finish this as it was just an illustration
of the idea.

> we have to use a macro or do like this:
> 
> void nmi_panic(const char *msg)
> {
> ...
> 	panic("%s", msg);
> }
> 
> If there is no objection, I'm going to use a macro.

Your other patch needs panic_cpu externally visible so the macro should
be OK.
-- 
Michal Hocko
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1194816 — RE: Re: [V2 PATCH 1/3] x86/panic: Fix re-entrance problem due to panic on NMI

From河合英宏 / KAWAI,HIDEHIRO <hidehiro.kawai.ez@hitachi.com>
Date2015-07-29 07:50 +0200
SubjectRE: Re: [V2 PATCH 1/3] x86/panic: Fix re-entrance problem due to panic on NMI
Message-ID<pRspA-7rV-11@gated-at.bofh.it>
In reply to#1193194
Hi,

> From: linux-kernel-owner@vger.kernel.org [mailto:linux-kernel-owner@vger.kernel.org] On Behalf Of Hidehiro Kawai
> (2015/07/27 23:34), Michal Hocko wrote:
> > On Mon 27-07-15 10:58:50, Hidehiro Kawai wrote:
[...]
> > The check could be also relaxed a bit and nmi_panic would
> > return only if the ongoing panic is the current cpu when we really have
> > to return and allow the preempted panic to finish.
> 
> It's reasonable.  I'll do that in the next version.

I noticed atomic_read() is insufficient.  Please consider the following
scenario.

CPU 1: call panic() in the normal context
CPU 0: call nmi_panic(), check the value of panic_cpu, then call panic()
CPU 1: set 1 to panic_cpu
CPU 0: fail to set 0 to panic_cpu, then do an infinite loop
CPU 1: call crash_kexec(), then call kdump_nmi_shootdown_cpus()

At this point, since CPU 0 loops in NMI context, it never executes
the NMI handler registered by kdump_nmi_shootdown_cpus().  This means
that no register states are saved and no cleanups for VMX/SVM are
performed.  So, we should still use atomic_cmpxchg() in nmi_panic() to
prevent other cpus from running panic routines.

> > +void nmi_panic(const char *fmt, ...)
> > +{
> > +	/*
> > +	 * We have to back off if the NMI has preempted an ongoing panic and
> > +	 * allow it to finish
> > +	 */
> > +	if (atomic_read(&panic_cpu) == raw_smp_processor_id())
> > +		return;
> > +
> > +	panic();
> > +}
> > +EXPORT_SYMBOL(nmi_panic);

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1194940 — Re: Re: [V2 PATCH 1/3] x86/panic: Fix re-entrance problem due to panic on NMI

FromMichal Hocko <mhocko@kernel.org>
Date2015-07-29 10:30 +0200
SubjectRe: Re: [V2 PATCH 1/3] x86/panic: Fix re-entrance problem due to panic on NMI
Message-ID<pRuUr-2EV-31@gated-at.bofh.it>
In reply to#1194816
On Wed 29-07-15 05:48:47, 河合英宏 / KAWAI,HIDEHIRO wrote:
> Hi,
> 
> > From: linux-kernel-owner@vger.kernel.org [mailto:linux-kernel-owner@vger.kernel.org] On Behalf Of Hidehiro Kawai
> > (2015/07/27 23:34), Michal Hocko wrote:
> > > On Mon 27-07-15 10:58:50, Hidehiro Kawai wrote:
> [...]
> > > The check could be also relaxed a bit and nmi_panic would
> > > return only if the ongoing panic is the current cpu when we really have
> > > to return and allow the preempted panic to finish.
> > 
> > It's reasonable.  I'll do that in the next version.
> 
> I noticed atomic_read() is insufficient.  Please consider the following
> scenario.
> 
> CPU 1: call panic() in the normal context
> CPU 0: call nmi_panic(), check the value of panic_cpu, then call panic()
> CPU 1: set 1 to panic_cpu
> CPU 0: fail to set 0 to panic_cpu, then do an infinite loop
> CPU 1: call crash_kexec(), then call kdump_nmi_shootdown_cpus()
> 
> At this point, since CPU 0 loops in NMI context, it never executes
> the NMI handler registered by kdump_nmi_shootdown_cpus().  This means
> that no register states are saved and no cleanups for VMX/SVM are
> performed.

Yes this is true but it is no different from the current state, isn't
it? So if you want to handle that then it deserves a separate patch.
It is certainly not harmful wrt. panic behavior.

> So, we should still use atomic_cmpxchg() in nmi_panic() to
> prevent other cpus from running panic routines.

Not sure what you mean by that.

-- 
Michal Hocko
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1194977 — RE: Re: [V2 PATCH 1/3] x86/panic: Fix re-entrance problem due to panic on NMI

From河合英宏 / KAWAI,HIDEHIRO <hidehiro.kawai.ez@hitachi.com>
Date2015-07-29 11:10 +0200
SubjectRE: Re: [V2 PATCH 1/3] x86/panic: Fix re-entrance problem due to panic on NMI
Message-ID<pRvx9-3Dq-29@gated-at.bofh.it>
In reply to#1194940
PiBGcm9tOiBNaWNoYWwgSG9ja28gW21haWx0bzptaG9ja29Aa2VybmVsLm9yZ10NCj4gT24gV2Vk
IDI5LTA3LTE1IDA1OjQ4OjQ3LCDmsrPlkIjoi7Hlro8gLyBLQVdBSe+8jEhJREVISVJPIHdyb3Rl
Og0KPiA+IEhpLA0KPiA+DQo+ID4gPiBGcm9tOiBsaW51eC1rZXJuZWwtb3duZXJAdmdlci5rZXJu
ZWwub3JnIFttYWlsdG86bGludXgta2VybmVsLW93bmVyQHZnZXIua2VybmVsLm9yZ10gT24gQmVo
YWxmIE9mIEhpZGVoaXJvIEthd2FpDQo+ID4gPiAoMjAxNS8wNy8yNyAyMzozNCksIE1pY2hhbCBI
b2NrbyB3cm90ZToNCj4gPiA+ID4gT24gTW9uIDI3LTA3LTE1IDEwOjU4OjUwLCBIaWRlaGlybyBL
YXdhaSB3cm90ZToNCj4gPiBbLi4uXQ0KPiA+ID4gPiBUaGUgY2hlY2sgY291bGQgYmUgYWxzbyBy
ZWxheGVkIGEgYml0IGFuZCBubWlfcGFuaWMgd291bGQNCj4gPiA+ID4gcmV0dXJuIG9ubHkgaWYg
dGhlIG9uZ29pbmcgcGFuaWMgaXMgdGhlIGN1cnJlbnQgY3B1IHdoZW4gd2UgcmVhbGx5IGhhdmUN
Cj4gPiA+ID4gdG8gcmV0dXJuIGFuZCBhbGxvdyB0aGUgcHJlZW1wdGVkIHBhbmljIHRvIGZpbmlz
aC4NCj4gPiA+DQo+ID4gPiBJdCdzIHJlYXNvbmFibGUuICBJJ2xsIGRvIHRoYXQgaW4gdGhlIG5l
eHQgdmVyc2lvbi4NCj4gPg0KPiA+IEkgbm90aWNlZCBhdG9taWNfcmVhZCgpIGlzIGluc3VmZmlj
aWVudC4gIFBsZWFzZSBjb25zaWRlciB0aGUgZm9sbG93aW5nDQo+ID4gc2NlbmFyaW8uDQo+ID4N
Cj4gPiBDUFUgMTogY2FsbCBwYW5pYygpIGluIHRoZSBub3JtYWwgY29udGV4dA0KPiA+IENQVSAw
OiBjYWxsIG5taV9wYW5pYygpLCBjaGVjayB0aGUgdmFsdWUgb2YgcGFuaWNfY3B1LCB0aGVuIGNh
bGwgcGFuaWMoKQ0KPiA+IENQVSAxOiBzZXQgMSB0byBwYW5pY19jcHUNCj4gPiBDUFUgMDogZmFp
bCB0byBzZXQgMCB0byBwYW5pY19jcHUsIHRoZW4gZG8gYW4gaW5maW5pdGUgbG9vcA0KPiA+IENQ
VSAxOiBjYWxsIGNyYXNoX2tleGVjKCksIHRoZW4gY2FsbCBrZHVtcF9ubWlfc2hvb3Rkb3duX2Nw
dXMoKQ0KPiA+DQo+ID4gQXQgdGhpcyBwb2ludCwgc2luY2UgQ1BVIDAgbG9vcHMgaW4gTk1JIGNv
bnRleHQsIGl0IG5ldmVyIGV4ZWN1dGVzDQo+ID4gdGhlIE5NSSBoYW5kbGVyIHJlZ2lzdGVyZWQg
Ynkga2R1bXBfbm1pX3Nob290ZG93bl9jcHVzKCkuICBUaGlzIG1lYW5zDQo+ID4gdGhhdCBubyBy
ZWdpc3RlciBzdGF0ZXMgYXJlIHNhdmVkIGFuZCBubyBjbGVhbnVwcyBmb3IgVk1YL1NWTSBhcmUN
Cj4gPiBwZXJmb3JtZWQuDQo+IA0KPiBZZXMgdGhpcyBpcyB0cnVlIGJ1dCBpdCBpcyBubyBkaWZm
ZXJlbnQgZnJvbSB0aGUgY3VycmVudCBzdGF0ZSwgaXNuJ3QNCj4gaXQ/IFNvIGlmIHlvdSB3YW50
IHRvIGhhbmRsZSB0aGF0IHRoZW4gaXQgZGVzZXJ2ZXMgYSBzZXBhcmF0ZSBwYXRjaC4NCj4gSXQg
aXMgY2VydGFpbmx5IG5vdCBoYXJtZnVsIHdydC4gcGFuaWMgYmVoYXZpb3IuDQo+IA0KPiA+IFNv
LCB3ZSBzaG91bGQgc3RpbGwgdXNlIGF0b21pY19jbXB4Y2hnKCkgaW4gbm1pX3BhbmljKCkgdG8N
Cj4gPiBwcmV2ZW50IG90aGVyIGNwdXMgZnJvbSBydW5uaW5nIHBhbmljIHJvdXRpbmVzLg0KPiAN
Cj4gTm90IHN1cmUgd2hhdCB5b3UgbWVhbiBieSB0aGF0Lg0KDQpJIG1lYW4gdGhhdCB3ZSBzaG91
bGQgdXNlIHRoZSBzYW1lIGxvZ2ljIGFzIG15IFYyIHBhdGNoIGxpa2UgdGhpczoNCg0KI2RlZmlu
ZSBubWlfcGFuaWMoZm10LCAuLi4pICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg
ICAgICAgICBcDQogICAgICAgZG8geyAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg
ICAgICAgICAgICAgICAgICAgICAgICAgIFwNCiAgICAgICAgICAgICAgIGlmIChhdG9taWNfY21w
eGNoZygmcGFuaWNfY3B1LCAtMSwgcmF3X3NtcF9wcm9jZXNzb3JfaWQoKSkgXA0KICAgICAgICAg
ICAgICAgICAgID09IC0xKSAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg
ICAgICBcDQogICAgICAgICAgICAgICAgICAgICAgIHBhbmljKGZtdCwgIyNfX1ZBX0FSR1NfXyk7
ICAgICAgICAgICAgICAgICAgICAgIFwNCiAgICAgICB9IHdoaWxlICgwKQ0KDQpCeSB1c2luZyBh
dG9taWNfY21weGNoZyBoZXJlLCB3ZSBjYW4gZW5zdXJlIHRoYXQgb25seSB0aGlzIGNwdQ0KcnVu
cyBwYW5pYyByb3V0aW5lcy4gIEl0IGlzIGltcG9ydGFudCB0byBwcmV2ZW50IGEgTk1JLWNvbnRl
eHQgY3B1DQpmcm9tIGNhbGxpbmcgcGFuaWNfc21wX3NlbGZfc3RvcCgpLiANCg0Kdm9pZCBwYW5p
Yyhjb25zdCBjaGFyICpmbXQsIC4uLikNCnsNCi4uLg0KICAgICAgICAqIGBvbGRfY3B1ID09IC0x
JyBtZWFucyB3ZSBhcmUgdGhlIGZpcnN0IGNvbWVyLg0KICAgICAgICAqIGBvbGRfY3B1ID09IHRo
aXNfY3B1JyBtZWFucyB3ZSBjYW1lIGhlcmUgZHVlIHRvIHBhbmljIG9uIE5NSS4NCiAgICAgICAg
Ki8NCiAgICAgICB0aGlzX2NwdSA9IHJhd19zbXBfcHJvY2Vzc29yX2lkKCk7DQogICAgICAgb2xk
X2NwdSA9IGF0b21pY19jbXB4Y2hnKCZwYW5pY19jcHUsIC0xLCB0aGlzX2NwdSk7DQogICAgICAg
aWYgKG9sZF9jcHUgIT0gLTEgJiYgb2xkX2NwdSAhPSB0aGlzX2NwdSkNCiAgICAgICAgICAgICAg
ICBwYW5pY19zbXBfc2VsZl9zdG9wKCk7DQoNClBsZWFzZSBhc3N1bWUgdGhhdCBDUFUgMCBjYWxs
cyBubWlfcGFuaWMoKSBpbiBOTUkgY29udGV4dA0KYW5kIENQVSAxIGNhbGxzIHBhbmljKCkgaW4g
bm9ybWFsIGNvbnRleHQgYXQgdGhhIHNhbWUgdGltZS4NCg0KSWYgQ1BVIDEgc2V0IHBhbmljX2Nw
dSBiZWZvcmUgQ1BVIDAgZG9lcywgQ1BVIDEgcnVucyBwYW5pYyByb3V0aW5lcw0KYW5kIENQVSAw
IHJldHVybiBmcm9tIHRoZSBubWkgaGFuZGxlci4gIEV2ZW50dWFsbHkgQ1BVIDAgaXMgc3RvcHBl
ZA0KYnkgbm1pX3Nob290ZG93bl9jcHVzKCkuDQoNCklmIENQVSAwIHNldCBwYW5pY19jcHUgYmVm
b3JlIENQVSAxIGRvZXMsIENQVSAwIHJ1bnMgcGFuaWMgcm91dGluZXMuDQpDUFUgMSBjYWxscyBw
YW5pY19zbXBfc2VsZl9zdG9wKCksIGFuZCB3YWl0IGZvciBOTUkgYnkNCm5taV9zaG9vdGRvd25f
Y3B1cygpLg0KDQpBbnl3YXksIEkgdGVzdGVkIG15IGFwcHJvYWNoIGFuZCBpdCB3b3JrZWQgZmlu
ZS4NCg0KUmVnYXJkcywNCkthd2FpDQoNCg==
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1194990 — Re: Re: [V2 PATCH 1/3] x86/panic: Fix re-entrance problem due to panic on NMI

FromMichal Hocko <mhocko@kernel.org>
Date2015-07-29 11:30 +0200
SubjectRe: Re: [V2 PATCH 1/3] x86/panic: Fix re-entrance problem due to panic on NMI
Message-ID<pRvQv-3ZS-17@gated-at.bofh.it>
In reply to#1194977
On Wed 29-07-15 09:09:18, 河合英宏 / KAWAI,HIDEHIRO wrote:
> > From: Michal Hocko [mailto:mhocko@kernel.org]
> > On Wed 29-07-15 05:48:47, 河合英宏 / KAWAI,HIDEHIRO wrote:
> > > Hi,
> > >
> > > > From: linux-kernel-owner@vger.kernel.org [mailto:linux-kernel-owner@vger.kernel.org] On Behalf Of Hidehiro Kawai
> > > > (2015/07/27 23:34), Michal Hocko wrote:
> > > > > On Mon 27-07-15 10:58:50, Hidehiro Kawai wrote:
> > > [...]
> > > > > The check could be also relaxed a bit and nmi_panic would
> > > > > return only if the ongoing panic is the current cpu when we really have
> > > > > to return and allow the preempted panic to finish.
> > > >
> > > > It's reasonable.  I'll do that in the next version.
> > >
> > > I noticed atomic_read() is insufficient.  Please consider the following
> > > scenario.
> > >
> > > CPU 1: call panic() in the normal context
> > > CPU 0: call nmi_panic(), check the value of panic_cpu, then call panic()
> > > CPU 1: set 1 to panic_cpu
> > > CPU 0: fail to set 0 to panic_cpu, then do an infinite loop
> > > CPU 1: call crash_kexec(), then call kdump_nmi_shootdown_cpus()
> > >
> > > At this point, since CPU 0 loops in NMI context, it never executes
> > > the NMI handler registered by kdump_nmi_shootdown_cpus().  This means
> > > that no register states are saved and no cleanups for VMX/SVM are
> > > performed.
> > 
> > Yes this is true but it is no different from the current state, isn't
> > it? So if you want to handle that then it deserves a separate patch.
> > It is certainly not harmful wrt. panic behavior.
> > 
> > > So, we should still use atomic_cmpxchg() in nmi_panic() to
> > > prevent other cpus from running panic routines.
> > 
> > Not sure what you mean by that.
> 
> I mean that we should use the same logic as my V2 patch like this:
> 
> #define nmi_panic(fmt, ...)                                            \
>        do {                                                            \
>                if (atomic_cmpxchg(&panic_cpu, -1, raw_smp_processor_id()) \
>                    == -1)                                              \
>                        panic(fmt, ##__VA_ARGS__);                      \
>        } while (0)

This would allow to return from NMI too eagerly. When I was testing my
previous approach (on 3.0 based kernel) I had basically the same thing
(one NMI to process panic) and others to return. This led to a strange
behavior when the NMI button triggered NMI on all (hundreds) CPUs. The
crash kernel booted eventually but the log contained lockups when a
CPU waited for an IPI to the CPU which was handling the NMI panic.

Anyway, I do not thing this is really necessary to solve the panic
reentrancy issue. If the missing saved state is a real problem then it
should be handled separately - maybe it can be achieved without an IPI
and directly from the panic context if we are in NMI.
-- 
Michal Hocko
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1195676 — RE: Re: [V2 PATCH 1/3] x86/panic: Fix re-entrance problem due to panic on NMI

From河合英宏 / KAWAI,HIDEHIRO <hidehiro.kawai.ez@hitachi.com>
Date2015-07-30 03:50 +0200
SubjectRE: Re: [V2 PATCH 1/3] x86/panic: Fix re-entrance problem due to panic on NMI
Message-ID<pRL8R-wR-7@gated-at.bofh.it>
In reply to#1194990
SGksDQoNCj4gRnJvbTogTWljaGFsIEhvY2tvIFttYWlsdG86bWhvY2tvQGtlcm5lbC5vcmddDQo+
IA0KPiBPbiBXZWQgMjktMDctMTUgMDk6MDk6MTgsIOays+WQiOiLseWujyAvIEtBV0FJ77yMSElE
RUhJUk8gd3JvdGU6DQo+ID4gPiBGcm9tOiBNaWNoYWwgSG9ja28gW21haWx0bzptaG9ja29Aa2Vy
bmVsLm9yZ10NCj4gPiA+IE9uIFdlZCAyOS0wNy0xNSAwNTo0ODo0Nywg5rKz5ZCI6Iux5a6PIC8g
S0FXQUnvvIxISURFSElSTyB3cm90ZToNCj4gPiA+ID4gSGksDQo+ID4gPiA+DQo+ID4gPiA+ID4g
RnJvbTogbGludXgta2VybmVsLW93bmVyQHZnZXIua2VybmVsLm9yZyBbbWFpbHRvOmxpbnV4LWtl
cm5lbC1vd25lckB2Z2VyLmtlcm5lbC5vcmddIE9uIEJlaGFsZiBPZiBIaWRlaGlybyBLYXdhaQ0K
PiA+ID4gPiA+ICgyMDE1LzA3LzI3IDIzOjM0KSwgTWljaGFsIEhvY2tvIHdyb3RlOg0KPiA+ID4g
PiA+ID4gT24gTW9uIDI3LTA3LTE1IDEwOjU4OjUwLCBIaWRlaGlybyBLYXdhaSB3cm90ZToNCj4g
PiA+ID4gWy4uLl0NCj4gPiA+ID4gPiA+IFRoZSBjaGVjayBjb3VsZCBiZSBhbHNvIHJlbGF4ZWQg
YSBiaXQgYW5kIG5taV9wYW5pYyB3b3VsZA0KPiA+ID4gPiA+ID4gcmV0dXJuIG9ubHkgaWYgdGhl
IG9uZ29pbmcgcGFuaWMgaXMgdGhlIGN1cnJlbnQgY3B1IHdoZW4gd2UgcmVhbGx5IGhhdmUNCj4g
PiA+ID4gPiA+IHRvIHJldHVybiBhbmQgYWxsb3cgdGhlIHByZWVtcHRlZCBwYW5pYyB0byBmaW5p
c2guDQo+ID4gPiA+ID4NCj4gPiA+ID4gPiBJdCdzIHJlYXNvbmFibGUuICBJJ2xsIGRvIHRoYXQg
aW4gdGhlIG5leHQgdmVyc2lvbi4NCj4gPiA+ID4NCj4gPiA+ID4gSSBub3RpY2VkIGF0b21pY19y
ZWFkKCkgaXMgaW5zdWZmaWNpZW50LiAgUGxlYXNlIGNvbnNpZGVyIHRoZSBmb2xsb3dpbmcNCj4g
PiA+ID4gc2NlbmFyaW8uDQo+ID4gPiA+DQo+ID4gPiA+IENQVSAxOiBjYWxsIHBhbmljKCkgaW4g
dGhlIG5vcm1hbCBjb250ZXh0DQo+ID4gPiA+IENQVSAwOiBjYWxsIG5taV9wYW5pYygpLCBjaGVj
ayB0aGUgdmFsdWUgb2YgcGFuaWNfY3B1LCB0aGVuIGNhbGwgcGFuaWMoKQ0KPiA+ID4gPiBDUFUg
MTogc2V0IDEgdG8gcGFuaWNfY3B1DQo+ID4gPiA+IENQVSAwOiBmYWlsIHRvIHNldCAwIHRvIHBh
bmljX2NwdSwgdGhlbiBkbyBhbiBpbmZpbml0ZSBsb29wDQo+ID4gPiA+IENQVSAxOiBjYWxsIGNy
YXNoX2tleGVjKCksIHRoZW4gY2FsbCBrZHVtcF9ubWlfc2hvb3Rkb3duX2NwdXMoKQ0KPiA+ID4g
Pg0KPiA+ID4gPiBBdCB0aGlzIHBvaW50LCBzaW5jZSBDUFUgMCBsb29wcyBpbiBOTUkgY29udGV4
dCwgaXQgbmV2ZXIgZXhlY3V0ZXMNCj4gPiA+ID4gdGhlIE5NSSBoYW5kbGVyIHJlZ2lzdGVyZWQg
Ynkga2R1bXBfbm1pX3Nob290ZG93bl9jcHVzKCkuICBUaGlzIG1lYW5zDQo+ID4gPiA+IHRoYXQg
bm8gcmVnaXN0ZXIgc3RhdGVzIGFyZSBzYXZlZCBhbmQgbm8gY2xlYW51cHMgZm9yIFZNWC9TVk0g
YXJlDQo+ID4gPiA+IHBlcmZvcm1lZC4NCj4gPiA+DQo+ID4gPiBZZXMgdGhpcyBpcyB0cnVlIGJ1
dCBpdCBpcyBubyBkaWZmZXJlbnQgZnJvbSB0aGUgY3VycmVudCBzdGF0ZSwgaXNuJ3QNCj4gPiA+
IGl0PyBTbyBpZiB5b3Ugd2FudCB0byBoYW5kbGUgdGhhdCB0aGVuIGl0IGRlc2VydmVzIGEgc2Vw
YXJhdGUgcGF0Y2guDQo+ID4gPiBJdCBpcyBjZXJ0YWlubHkgbm90IGhhcm1mdWwgd3J0LiBwYW5p
YyBiZWhhdmlvci4NCj4gPiA+DQo+ID4gPiA+IFNvLCB3ZSBzaG91bGQgc3RpbGwgdXNlIGF0b21p
Y19jbXB4Y2hnKCkgaW4gbm1pX3BhbmljKCkgdG8NCj4gPiA+ID4gcHJldmVudCBvdGhlciBjcHVz
IGZyb20gcnVubmluZyBwYW5pYyByb3V0aW5lcy4NCj4gPiA+DQo+ID4gPiBOb3Qgc3VyZSB3aGF0
IHlvdSBtZWFuIGJ5IHRoYXQuDQo+ID4NCj4gPiBJIG1lYW4gdGhhdCB3ZSBzaG91bGQgdXNlIHRo
ZSBzYW1lIGxvZ2ljIGFzIG15IFYyIHBhdGNoIGxpa2UgdGhpczoNCj4gPg0KPiA+ICNkZWZpbmUg
bm1pX3BhbmljKGZtdCwgLi4uKSAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg
ICAgICAgXA0KPiA+ICAgICAgICBkbyB7ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg
ICAgICAgICAgICAgICAgICAgICAgICAgICAgXA0KPiA+ICAgICAgICAgICAgICAgIGlmIChhdG9t
aWNfY21weGNoZygmcGFuaWNfY3B1LCAtMSwgcmF3X3NtcF9wcm9jZXNzb3JfaWQoKSkgXA0KPiA+
ICAgICAgICAgICAgICAgICAgICA9PSAtMSkgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg
ICAgICAgICAgICAgICAgXA0KPiA+ICAgICAgICAgICAgICAgICAgICAgICAgcGFuaWMoZm10LCAj
I19fVkFfQVJHU19fKTsgICAgICAgICAgICAgICAgICAgICAgXA0KPiA+ICAgICAgICB9IHdoaWxl
ICgwKQ0KPiANCj4gVGhpcyB3b3VsZCBhbGxvdyB0byByZXR1cm4gZnJvbSBOTUkgdG9vIGVhZ2Vy
bHkuDQoNClllcywgYnV0IHdoYXQncyB0aGUgcHJvYmxlbT8NClRoZSByb290IGNhdXNlIG9mIHlv
dXIgY2FzZSBoYXNuJ3QgYmVlbiBjbGFyaWZpZWQgeWV0Lg0KSSBjYW4ndCBmaXggZm9yIGFuIHVu
Y2xlYXIgaXNzdWUgYmVjYXVzZSBJIGRvbid0IGtub3cgd2hhdCdzIHRoZSByaWdodA0Kc29sdXRp
b24uDQoNCj4gV2hlbiBJIHdhcyB0ZXN0aW5nIG15DQo+IHByZXZpb3VzIGFwcHJvYWNoIChvbiAz
LjAgYmFzZWQga2VybmVsKSBJIGhhZCBiYXNpY2FsbHkgdGhlIHNhbWUgdGhpbmcNCj4gKG9uZSBO
TUkgdG8gcHJvY2VzcyBwYW5pYykgYW5kIG90aGVycyB0byByZXR1cm4uIFRoaXMgbGVkIHRvIGEg
c3RyYW5nZQ0KPiBiZWhhdmlvciB3aGVuIHRoZSBOTUkgYnV0dG9uIHRyaWdnZXJlZCBOTUkgb24g
YWxsIChodW5kcmVkcykgQ1BVcy4NCg0KSXQncyBzdHJhbmdlLiAgVXN1YWxseSwgTk1JIGNhdXNl
ZCBieSBOTUkgYnV0dG9uIGlzIHJvdXRlZCB0byBvbmx5IENQVSAwDQphcyBhbiBleHRlcm5hbCBO
TUkuICBFeHRlcm5hbCBOTUkgZm9yIENQVXMgb3RoZXIgdGhhbiBDUFUgMCBhcmUgbWFza2VkDQph
dCBib290IHRpbWUuICBEb2VzIGl0IHJlYWxseSBoYXBwZW4/ICBEb2VzIHRoZSBwcm9ibGVtIHN0
aWxsIGhhcHBlbiBvbg0KdGhlIGxhdGVzdCBrZXJuZWw/ICBXaGF0IGtpbmQgb2YgTk1JIGlzIGRl
bGl2ZXJkIHRvIGVhY2ggQ1BVPw0KDQpUcmFkaXRpb25hbGx5LCB3ZSBzaG91bGQgaGF2ZSBhc3N1
bWVkIHRoYXQgTk1JIGZvciBjcmFzaCBkdW1waW5nIGlzDQpkZWxpdmVyZWQgdG8gb25seSBvbmUg
Y3B1LiAgT3RoZXJ3aXNlLCB3ZSBzaG91bGQgb2Z0ZW4gZmFpbCB0byB0YWtlDQphIHByb3BlciBj
cmFzaCBkdW1wLiAgSXQgc2VlbXMgdGhhdCB5b3VyIGNhc2UgaXMgYW5vdGhlciBwcm9ibGVtIHRv
IGJlDQpzb2x2ZWQgc2VwYXJhdGVseS4NCg0KPiBUaGUNCj4gY3Jhc2gga2VybmVsIGJvb3RlZCBl
dmVudHVhbGx5IGJ1dCB0aGUgbG9nIGNvbnRhaW5lZCBsb2NrdXBzIHdoZW4gYQ0KPiBDUFUgd2Fp
dGVkIGZvciBhbiBJUEkgdG8gdGhlIENQVSB3aGljaCB3YXMgaGFuZGxpbmcgdGhlIE5NSSBwYW5p
Yy4NCg0KQ291bGQgeW91IGV4cGxhaW4gbW9yZSBwcmVjaXNlbHk/DQoNCj4gQW55d2F5LCBJIGRv
IG5vdCB0aGluZyB0aGlzIGlzIHJlYWxseSBuZWNlc3NhcnkgdG8gc29sdmUgdGhlIHBhbmljDQo+
IHJlZW50cmFuY3kgaXNzdWUuDQo+IElmIHRoZSBtaXNzaW5nIHNhdmVkIHN0YXRlIGlzIGEgcmVh
bCBwcm9ibGVtIHRoZW4gaXQNCj4gc2hvdWxkIGJlIGhhbmRsZWQgc2VwYXJhdGVseSAtIG1heWJl
IGl0IGNhbiBiZSBhY2hpZXZlZCB3aXRob3V0IGFuIElQSQ0KPiBhbmQgZGlyZWN0bHkgZnJvbSB0
aGUgcGFuaWMgY29udGV4dCBpZiB3ZSBhcmUgaW4gTk1JLg0KDQpXaGF0IEkgd291bGQgbGlrZSB0
byBkbyB2aWEgdGhpcyBwYXRjaHNlIGlzIHRvIHNvbHZlIHJhY2UgaXNzdWVzDQphbW9uZyBOTUks
IHBhbmljKCkgYW5kIGNyYXNoX2tleGVjKCkuICBTbywgSSBkb24ndCB0aGluayB3ZSBzaG91bGQg
Zml4DQp0aGF0IHNlcGFyYXRlbHksIGFsdGhvdWdoIEkgd291bGQgbmVlZCB0byByZXdvcmQgc29t
ZSBkZXNjcmlwdGlvbnMNCmFuZCB0aXRsZXMuDQoNCkFueXdheSwgSSdtIGdvaW5nIHRvIHNlbnQg
b3V0IG15IHJldmlzZWQgdmVyc2lvbiBvbmNlIGluIG9yZGVyIHRvDQp0aWR5IHVwLiAgSSBhbHNv
IHdvdWxkIGxpa2UgdG8gaGVhciBrZXhlYy9rZHVtcCBndXlzJyBvcGluaW9ucy4NCg0KUmVnYXJk
cywNCkthd2FpDQoNCg==
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1195762 — RE: Re: [V2 PATCH 1/3] x86/panic: Fix re-entrance problem due to panic on NMI

From河合英宏 / KAWAI,HIDEHIRO <hidehiro.kawai.ez@hitachi.com>
Date2015-07-30 09:40 +0200
SubjectRE: Re: [V2 PATCH 1/3] x86/panic: Fix re-entrance problem due to panic on NMI
Message-ID<pRQBA-5h-25@gated-at.bofh.it>
In reply to#1195676
SGkgTWljaGFsLA0KDQo+IEZyb206IOays+WQiOiLseWujyAvIEtBV0FJ77yMSElERUhJUk8gW21h
aWx0bzpoaWRlaGlyby5rYXdhaS5lekBoaXRhY2hpLmNvbV0NCj4gPiBXaGVuIEkgd2FzIHRlc3Rp
bmcgbXkNCj4gPiBwcmV2aW91cyBhcHByb2FjaCAob24gMy4wIGJhc2VkIGtlcm5lbCkgSSBoYWQg
YmFzaWNhbGx5IHRoZSBzYW1lIHRoaW5nDQo+ID4gKG9uZSBOTUkgdG8gcHJvY2VzcyBwYW5pYykg
YW5kIG90aGVycyB0byByZXR1cm4uIFRoaXMgbGVkIHRvIGEgc3RyYW5nZQ0KPiA+IGJlaGF2aW9y
IHdoZW4gdGhlIE5NSSBidXR0b24gdHJpZ2dlcmVkIE5NSSBvbiBhbGwgKGh1bmRyZWRzKSBDUFVz
Lg0KPiANCj4gSXQncyBzdHJhbmdlLiAgVXN1YWxseSwgTk1JIGNhdXNlZCBieSBOTUkgYnV0dG9u
IGlzIHJvdXRlZCB0byBvbmx5IENQVSAwDQo+IGFzIGFuIGV4dGVybmFsIE5NSS4gIEV4dGVybmFs
IE5NSSBmb3IgQ1BVcyBvdGhlciB0aGFuIENQVSAwIGFyZSBtYXNrZWQNCj4gYXQgYm9vdCB0aW1l
LiAgRG9lcyBpdCByZWFsbHkgaGFwcGVuPyAgRG9lcyB0aGUgcHJvYmxlbSBzdGlsbCBoYXBwZW4g
b24NCj4gdGhlIGxhdGVzdCBrZXJuZWw/ICBXaGF0IGtpbmQgb2YgTk1JIGlzIGRlbGl2ZXJkIHRv
IGVhY2ggQ1BVPw0KDQpBcmUgeW91IHVzaW5nIFNHSSBVVj8gIE9uIHRoYXQgcGxhdGZvcm0sIE5N
SXMgbWF5IGJlIGRlbGl2ZXJlZCB0bw0KYWxsIGNwdXMgYmVjYXVzZSBMVlQxIG9mIGFsbCBjcHVz
IGFyZSBub3QgbWFza2VkIGFzIGZvbGxvd3M6DQoNCnZvaWQgdXZfbm1pX2luaXQodm9pZCkNCnsN
CiAgICAgICAgdW5zaWduZWQgaW50IHZhbHVlOw0KDQogICAgICAgIC8qDQogICAgICAgICAqIFVu
bWFzayBOTUkgb24gYWxsIGNwdXMNCiAgICAgICAgICovDQogICAgICAgIHZhbHVlID0gYXBpY19y
ZWFkKEFQSUNfTFZUMSkgfCBBUElDX0RNX05NSTsNCiAgICAgICAgdmFsdWUgJj0gfkFQSUNfTFZU
X01BU0tFRDsNCiAgICAgICAgYXBpY193cml0ZShBUElDX0xWVDEsIHZhbHVlKTsNCn0NCg0KPiAN
Cj4gVHJhZGl0aW9uYWxseSwgd2Ugc2hvdWxkIGhhdmUgYXNzdW1lZCB0aGF0IE5NSSBmb3IgY3Jh
c2ggZHVtcGluZyBpcw0KPiBkZWxpdmVyZWQgdG8gb25seSBvbmUgY3B1LiAgT3RoZXJ3aXNlLCB3
ZSBzaG91bGQgb2Z0ZW4gZmFpbCB0byB0YWtlDQo+IGEgcHJvcGVyIGNyYXNoIGR1bXAuICBJdCBz
ZWVtcyB0aGF0IHlvdXIgY2FzZSBpcyBhbm90aGVyIHByb2JsZW0gdG8gYmUNCj4gc29sdmVkIHNl
cGFyYXRlbHkuDQo+IA0KPiA+IFRoZQ0KPiA+IGNyYXNoIGtlcm5lbCBib290ZWQgZXZlbnR1YWxs
eSBidXQgdGhlIGxvZyBjb250YWluZWQgbG9ja3VwcyB3aGVuIGENCj4gPiBDUFUgd2FpdGVkIGZv
ciBhbiBJUEkgdG8gdGhlIENQVSB3aGljaCB3YXMgaGFuZGxpbmcgdGhlIE5NSSBwYW5pYy4NCj4g
DQo+IENvdWxkIHlvdSBleHBsYWluIG1vcmUgcHJlY2lzZWx5Pw0KPiANCj4gPiBBbnl3YXksIEkg
ZG8gbm90IHRoaW5nIHRoaXMgaXMgcmVhbGx5IG5lY2Vzc2FyeSB0byBzb2x2ZSB0aGUgcGFuaWMN
Cj4gPiByZWVudHJhbmN5IGlzc3VlLg0KPiA+IElmIHRoZSBtaXNzaW5nIHNhdmVkIHN0YXRlIGlz
IGEgcmVhbCBwcm9ibGVtIHRoZW4gaXQNCj4gPiBzaG91bGQgYmUgaGFuZGxlZCBzZXBhcmF0ZWx5
IC0gbWF5YmUgaXQgY2FuIGJlIGFjaGlldmVkIHdpdGhvdXQgYW4gSVBJDQo+ID4gYW5kIGRpcmVj
dGx5IGZyb20gdGhlIHBhbmljIGNvbnRleHQgaWYgd2UgYXJlIGluIE5NSS4NCj4gDQo+IFdoYXQg
SSB3b3VsZCBsaWtlIHRvIGRvIHZpYSB0aGlzIHBhdGNoc2UgaXMgdG8gc29sdmUgcmFjZSBpc3N1
ZXMNCj4gYW1vbmcgTk1JLCBwYW5pYygpIGFuZCBjcmFzaF9rZXhlYygpLiAgU28sIEkgZG9uJ3Qg
dGhpbmsgd2Ugc2hvdWxkIGZpeA0KPiB0aGF0IHNlcGFyYXRlbHksIGFsdGhvdWdoIEkgd291bGQg
bmVlZCB0byByZXdvcmQgc29tZSBkZXNjcmlwdGlvbnMNCj4gYW5kIHRpdGxlcy4NCj4gDQo+IEFu
eXdheSwgSSdtIGdvaW5nIHRvIHNlbnQgb3V0IG15IHJldmlzZWQgdmVyc2lvbiBvbmNlIGluIG9y
ZGVyIHRvDQo+IHRpZHkgdXAuICBJIGFsc28gd291bGQgbGlrZSB0byBoZWFyIGtleGVjL2tkdW1w
IGd1eXMnIG9waW5pb25zLg0KPiANCj4gUmVnYXJkcywNCj4gS2F3YWkNCg0K
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1195780 — Re: Re: [V2 PATCH 1/3] x86/panic: Fix re-entrance problem due to panic on NMI

FromMichal Hocko <mhocko@kernel.org>
Date2015-07-30 10:00 +0200
SubjectRe: Re: [V2 PATCH 1/3] x86/panic: Fix re-entrance problem due to panic on NMI
Message-ID<pRQUW-t0-25@gated-at.bofh.it>
In reply to#1195762
On Thu 30-07-15 07:33:15, 河合英宏 / KAWAI,HIDEHIRO wrote:
[...]
> Are you using SGI UV?  On that platform, NMIs may be delivered to
> all cpus because LVT1 of all cpus are not masked as follows:

This is Compute Blade 520XB1 from Hitachi with 240 cpus.

-- 
Michal Hocko
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1195794 — RE: Re: [V2 PATCH 1/3] x86/panic: Fix re-entrance problem due to panic on NMI

From河合英宏 / KAWAI,HIDEHIRO <hidehiro.kawai.ez@hitachi.com>
Date2015-07-30 10:10 +0200
SubjectRE: Re: [V2 PATCH 1/3] x86/panic: Fix re-entrance problem due to panic on NMI
Message-ID<pRR4C-Ty-31@gated-at.bofh.it>
In reply to#1195780
SGksDQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogTWljaGFsIEhvY2tv
IFttYWlsdG86bWhvY2tvQGtlcm5lbC5vcmddDQo+IA0KPiBPbiBUaHUgMzAtMDctMTUgMDc6MzM6
MTUsIOays+WQiOiLseWujyAvIEtBV0FJ77yMSElERUhJUk8gd3JvdGU6DQo+IFsuLi5dDQo+ID4g
QXJlIHlvdSB1c2luZyBTR0kgVVY/ICBPbiB0aGF0IHBsYXRmb3JtLCBOTUlzIG1heSBiZSBkZWxp
dmVyZWQgdG8NCj4gPiBhbGwgY3B1cyBiZWNhdXNlIExWVDEgb2YgYWxsIGNwdXMgYXJlIG5vdCBt
YXNrZWQgYXMgZm9sbG93czoNCj4gDQo+IFRoaXMgaXMgQ29tcHV0ZSBCbGFkZSA1MjBYQjEgZnJv
bSBIaXRhY2hpIHdpdGggMjQwIGNwdXMuDQoNClRoYW5rcyBmb3IgdGhlIGluZm9ybWF0aW9uIQ0K
DQpJIGFza2VkIG15IGNvbGxlYWd1ZSBpbiBvdGhlciBkZXBhcnRtZW50IGFib3V0IE5NSSBidXR0
b24gYmVoYXZpb3INCm9mIG91ciBzZXJ2ZXIganVzdCBiZWZvcmUgcmVjZWl2ZSB5b3VyIG1haWws
IGFuZCBjZXJ0YWlubHkgd2hhdCB5b3UNCnNhaWQgaGFwcGVuczsgYWxsIGNwdXMgc2F5ICJVaGh1
aC4gTk1JIHJlY2VpdmVkIGZvciB1bmtub3duIHJlYXNvbiAzZA0Kb24gQ1BVIE4iIHdoZW4gTk1J
IGJ1dHRvbiBpcyBwdXNzaGVkLiAgU28sIHRoaXMgd2FzIGFsc28gb3VyIHByb2JsZW0uLi4NCg0K
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1195766 — Re: Re: [V2 PATCH 1/3] x86/panic: Fix re-entrance problem due to panic on NMI

FromMichal Hocko <mhocko@kernel.org>
Date2015-07-30 09:50 +0200
SubjectRe: Re: [V2 PATCH 1/3] x86/panic: Fix re-entrance problem due to panic on NMI
Message-ID<pRQLg-gS-19@gated-at.bofh.it>
In reply to#1195676
On Thu 30-07-15 01:45:35, 河合英宏 / KAWAI,HIDEHIRO wrote:
> Hi,
> 
> > From: Michal Hocko [mailto:mhocko@kernel.org]
> > 
> > On Wed 29-07-15 09:09:18, 河合英宏 / KAWAI,HIDEHIRO wrote:
[...]
> > > #define nmi_panic(fmt, ...)                                            \
> > >        do {                                                            \
> > >                if (atomic_cmpxchg(&panic_cpu, -1, raw_smp_processor_id()) \
> > >                    == -1)                                              \
> > >                        panic(fmt, ##__VA_ARGS__);                      \
> > >        } while (0)
> > 
> > This would allow to return from NMI too eagerly.
> 
> Yes, but what's the problem?

I believe that panic should be noreturn as much as possible and return
only when we do not have any other options. Moreover I would ask an
opposite question, what is the problem to loop in NMI on other CPUs than
the one which is performing crash_kexec? We will not save registers, so
what?

> The root cause of your case hasn't been clarified yet.
> I can't fix for an unclear issue because I don't know what's the right
> solution.
> 
> > When I was testing my
> > previous approach (on 3.0 based kernel) I had basically the same thing
> > (one NMI to process panic) and others to return. This led to a strange
> > behavior when the NMI button triggered NMI on all (hundreds) CPUs.
> 
> It's strange.  Usually, NMI caused by NMI button is routed to only CPU 0
> as an external NMI.  External NMI for CPUs other than CPU 0 are masked
> at boot time.  Does it really happen?

Could you point me to the code which does that, please? Maybe we are
missing that in our 3.0 kernel. I was quite surprised to see this
behavior as well.

> Does the problem still happen on the latest kernel?

I do not have machine accessible so I have to rely on the customer to
test and the current vanilla might be an issue.

> What kind of NMI is deliverd to each CPU?

See the log below.

> Traditionally, we should have assumed that NMI for crash dumping is
> delivered to only one cpu.  Otherwise, we should often fail to take
> a proper crash dump.

You might still get a panic on hardlockup which will happen on all CPUs
from the NMI context so we have to be able to handle panic in NMI on
many CPUs.

> It seems that your case is another problem to be solved separately.

I do not think so, quite contrary. If you want to solve the reentrancy
then other CPUs might be spinning in NMI if there is a guarantee that at
least one CPU can progress to finish crash_kexec().

> > The
> > crash kernel booted eventually but the log contained lockups when a
> > CPU waited for an IPI to the CPU which was handling the NMI panic.
> 
> Could you explain more precisely?

[  167.843761] Uhhuh. NMI received for unknown reason 3d on CPU 130.
[  167.843763] Do you have a strange power saving mode enabled?
[... Mangled output ....]
[  167.856415] Dazed and confused, but trying to continue
[  167.856428] Dazed and confused, but trying to continue
[  167.856442] Dazed and confused, but trying to continue
[...]
[  193.108440] BUG: soft lockup - CPU#0 stuck for 22s! [kworker/0:0:4]
[...]
[  193.108586] Call Trace:
[  193.108595]  [<ffffffff8109baeb>] smp_call_function_single+0x15b/0x170
[  193.108600]  [<ffffffff8109bb4e>] smp_call_function_any+0x4e/0x110
[  193.108607]  [<ffffffffa04a332c>] get_cur_val+0xbc/0x130 [acpi_cpufreq]
[  193.108630]  [<ffffffffa04a3417>] get_cur_freq_on_cpu+0x77/0xf0 [acpi_cpufreq]
[  193.108638]  [<ffffffff8137bc37>] cpufreq_update_policy+0x97/0x140
[  193.108646]  [<ffffffffa00ca04b>] acpi_processor_notify+0x4b/0x145 [processor]
[  193.108654]  [<ffffffff812d2eca>] acpi_ev_notify_dispatch+0x61/0x77
[  193.108659]  [<ffffffff812c1785>] acpi_os_execute_deferred+0x21/0x2c
[  193.108667]  [<ffffffff8107d03c>] process_one_work+0x16c/0x350
[  193.108673]  [<ffffffff8107fd6a>] worker_thread+0x17a/0x410
[  193.108679]  [<ffffffff81084136>] kthread+0x96/0xa0
[  193.108688]  [<ffffffff8146df64>] kernel_thread_helper+0x4/0x10
[...]
[  221.068390] BUG: soft lockup - CPU#0 stuck for 22s! [kworker/0:0:4]
[...]
[  227.991235] INFO: rcu_sched_state detected stalls on CPUs/tasks: { 130} (detected by 56, t=15002 jiffies)
[  227.991247] sending NMI to all CPUs:
[  227.991251] NMI backtrace for cpu 0
[  229.074091] INFO: rcu_bh_state detected stalls on CPUs/tasks: { 130} (detected by 105, t=15013 jiffies)
[    0.000000] Initializing cgroup subsys cpuset
[    0.000000] Initializing cgroup subsys cpu
[    0.000000] Linux version 3.0.101-0.47.55.9.8853.0.TEST-default (geeko@buildhost) (gcc version 4.3.4 [gcc-4_3-branch revision 152973] (SUSE Linux) ) #1 SMP Thu May 28 08:25:11 UTC 2015 (dc083ee)
[    0.000000] Command line: root=/dev/system/lvroot resume=/dev/system/lvswap intel_idle.max_cstate=0 processor.max_cstate=0 elevator=deadline nmi_watchdog=1 console=tty0 console=ttyS1,115200 elevator=deadline sysrq=yes reset_devices irqpoll maxcpus=1 disable_cpu_apicid=0 noefi acpi_rsdp=0xba7a4014  crashkernel=1024M-:512M memmap=exactmap memmap=576K@64K memmap=523684K@393216K elfcorehdr=916900K memmap=32768K#3018748K memmap=3736K#3051516K memmap=262144K$3145728K

I can provide the full log but it is quite mangled. I guess the
CPU130 was the only one allowed to proceed with the panic while others
returned from the unknown NMI handling. It took a lot of time until
CPU130 managed to boot the crash kernel with soft lockups and RCU stalls
reports. CPU0 is most probably locked up waiting for CPU130 to
acknowledge the IPI which will not happen apparently.

Maybe this is not possible in the current kernels for some reason but it
tells me that returning from panic is quite fragile so I would like to
prevent from it as much as possible.

> > Anyway, I do not thing this is really necessary to solve the panic
> > reentrancy issue.
> > If the missing saved state is a real problem then it
> > should be handled separately - maybe it can be achieved without an IPI
> > and directly from the panic context if we are in NMI.
> 
> What I would like to do via this patchse is to solve race issues
> among NMI, panic() and crash_kexec().

Yes I fully support you in this ;) I just believe that spinning in NMI
vs. saving registers is a separate issue.

> So, I don't think we should fix that separately, although I would need
> to reword some descriptions and titles.

I can have them tested.

-- 
Michal Hocko
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1199593 — Re: Re: [V2 PATCH 1/3] x86/panic: Fix re-entrance problem due to panic on NMI

FromMichal Hocko <mhocko@kernel.org>
Date2015-08-04 11:20 +0200
SubjectRe: Re: [V2 PATCH 1/3] x86/panic: Fix re-entrance problem due to panic on NMI
Message-ID<pTGy5-6co-1@gated-at.bofh.it>
In reply to#1195766
On Fri 31-07-15 11:23:00, 河合英宏 / KAWAI,HIDEHIRO wrote:
> > From: Michal Hocko [mailto:mhocko@kernel.org]
[...]
> > I am saying that watchdog_overflow_callback might trigger on more CPUs
> > and panic from NMI context as well. So this is not reduced to the NMI
> > button sends NMI to more CPUs.
> 
> I understand.  So, I have to also modify watchdog_overflow_callback
> to call nmi_panic().

yes.

[...]
> > > There is a timeout of 1000ms in nmi_shootdown_cpus(), so I don't know
> > > why CPU 130 waits so long.  I'll try to consider for a while.
> > 
> > Yes, I do not understand the timing here either and the fact that the
> > log is a complete mess in the important parts doesn't help a wee bit.
> 
> I'm interested in where "kernel panic -not syncing: " is.
> It may give us a clue.

This one is lost in the mangled text:
[  167.843771] U<0>[  167.843771] hhuh. NMI received for unkn<0><0>[  167.843765] Uh[  16NM843774I own rea reived for unknow<0 r  16n 2d 765] Uhhuh. CPU recei11. <0known reason 7. on770] Ker<[ - not rn NMI:nic - not contt sing

<0 >[ : Not con.inu437azed and confused, b] Dtryingaed annue

fu 167.8ut trying>[   to 7.<0377 167.843775] U<0>[  167.843776] ]hhu.ived for u3nknown rMason 3 re oived for [nk167.843781]  1.
<. N0>[  167.843781] Uh. NMI recen 3d on CPU 0.i< >[ nowon 3d on] Chhuh.MI
eceived[ or7.843nknoUhhuh.wn rMason e3d ceCPivUd 120.
<0nk>no 167.wn843ason 3na s p120.
o<0er savi d6 e843ab88] Do yeu have a
<trange0>[ er saving mode e nabl1d?7<4][  167 84hu94]MIuh. NceIived for unknown reas vdfor 1no3was0>[ 2d 67.84380on CI rUe 12e.
ive7d8u3800wn rveaseo f2d on CPo3.r< u>k[o 1 rea6s.o2d8 oo you hn aPve <0st>a e power 1s7.843816] Do yoauv ng moade enbslra?ng[ e 167.8438p41o]er shhuhavi.ngIroenived fbled?nknow
< reaso0> 2d on [PU1626.41]0>   Uh67.h. NM387I] receihed for .nknown reason  2Nn MC U ceived for .
[son 2d on CPU 6.
<  160>7.8467.84873] Uhhuh. 3MI received 908 o knstra
[ n167.843908] Do ygo pave westrangesa pvnv mode enableng mode ed?
n<b0ed?
-- 
Michal Hocko
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1199713 — RE: Re: [V2 PATCH 1/3] x86/panic: Fix re-entrance problem due to panic on NMI

From河合英宏 / KAWAI,HIDEHIRO <hidehiro.kawai.ez@hitachi.com>
Date2015-08-04 14:00 +0200
SubjectRE: Re: [V2 PATCH 1/3] x86/panic: Fix re-entrance problem due to panic on NMI
Message-ID<pTJ2X-1c2-39@gated-at.bofh.it>
In reply to#1199593
SGksDQoNCj4gRnJvbTogTWljaGFsIEhvY2tvIFttYWlsdG86bWhvY2tvQGtlcm5lbC5vcmddDQo+
IE9uIEZyaSAzMS0wNy0xNSAxMToyMzowMCwg5rKz5ZCI6Iux5a6PIC8gS0FXQUnvvIxISURFSElS
TyB3cm90ZToNCj4gPiA+IEZyb206IE1pY2hhbCBIb2NrbyBbbWFpbHRvOm1ob2Nrb0BrZXJuZWwu
b3JnXQ0KPiA+ID4gPiBUaGVyZSBpcyBhIHRpbWVvdXQgb2YgMTAwMG1zIGluIG5taV9zaG9vdGRv
d25fY3B1cygpLCBzbyBJIGRvbid0IGtub3cNCj4gPiA+ID4gd2h5IENQVSAxMzAgd2FpdHMgc28g
bG9uZy4gIEknbGwgdHJ5IHRvIGNvbnNpZGVyIGZvciBhIHdoaWxlLg0KPiA+ID4NCj4gPiA+IFll
cywgSSBkbyBub3QgdW5kZXJzdGFuZCB0aGUgdGltaW5nIGhlcmUgZWl0aGVyIGFuZCB0aGUgZmFj
dCB0aGF0IHRoZQ0KPiA+ID4gbG9nIGlzIGEgY29tcGxldGUgbWVzcyBpbiB0aGUgaW1wb3J0YW50
IHBhcnRzIGRvZXNuJ3QgaGVscCBhIHdlZSBiaXQuDQo+ID4NCj4gPiBJJ20gaW50ZXJlc3RlZCBp
biB3aGVyZSAia2VybmVsIHBhbmljIC1ub3Qgc3luY2luZzogIiBpcy4NCj4gPiBJdCBtYXkgZ2l2
ZSB1cyBhIGNsdWUuDQo+IA0KPiBUaGlzIG9uZSBpcyBsb3N0IGluIHRoZSBtYW5nbGVkIHRleHQ6
DQo+IFsgIDE2Ny44NDM3NzFdIFU8MD5bICAxNjcuODQzNzcxXSBoaHVoLiBOTUkgcmVjZWl2ZWQg
Zm9yIHVua248MD48MD5bICAxNjcuODQzNzY1XSBVaFsgIDE2Tk04NDM3NzRJIG93biByZWEgcmVp
dmVkIGZvcg0KPiB1bmtub3c8MCByICAxNm4gMmQgNzY1XSBVaGh1aC4gQ1BVIHJlY2VpMTEuIDww
a25vd24gcmVhc29uIDcuIG9uNzcwXSBLZXI8WyAtIG5vdCBybiBOTUk6bmljIC0gbm90IGNvbnR0
IHNpbmcNCj4gDQo+IDwwID5bIDogTm90IGNvbi5pbnU0MzdhemVkIGFuZCBjb25mdXNlZCwgYl0g
RHRyeWluZ2FlZCBhbm51ZQ0KPiANCj4gZnUgMTY3Ljh1dCB0cnlpbmc+WyAgIHRvIDcuPDAzNzcg
MTY3Ljg0Mzc3NV0gVTwwPlsgIDE2Ny44NDM3NzZdIF1oaHUuaXZlZCBmb3IgdTNua25vd24gck1h
c29uIDMgcmUgb2l2ZWQgZm9yIFtuazE2Ny44NDM3ODFdDQoNClRoYW5rcyBmb3IgdGhlIGluZm9y
bWF0aW9uLg0KDQpJIGFudGljaXBhdGVkIHRoYXQgc29tZSBsb2NrIGNvbnRlbnRpb24gb24gaXNz
dWluZyBtZXNzYWdlcyAoZS5nLg0KbG9ja3MgdXNlZCBieSBuZXR3b3JrL25ldGNvbnNvbGUgZHJp
dmVyKSBkZWxheWVkIHRoZSBwYW5pYyBwcm9jZWR1cmUsDQpidXQgaXQgc2VlbXMgbm90IHRvIGJl
IHJlbGF0ZWQgYmVjYXVzZSB0aGUgcGFuaWMgbWVzc2FnZSBmaW5pc2hlZCB0bw0KYmUgaXNzdWVk
IGVhcmx5Lg0KDQpJZiBJIGNvbWUgdXAgd2l0aCBzb21ldGhpbmcsIEkgd2lsbCBwb3N0IGEgbWFp
bC4gIEkgdGhpbmsgdGhlcmUNCm1heSBiZSBwb3RlbnRpYWwgaXNzdWVzLg0KDQo+IDEuDQo+IDwu
IE4wPlsgIDE2Ny44NDM3ODFdIFVoLiBOTUkgcmVjZW4gM2Qgb24gQ1BVIDAuaTwgPlsgbm93b24g
M2Qgb25dIENoaHVoLk1JDQo+IGVjZWl2ZWRbIG9yNy44NDNua25vVWhodWgud24gck1hc29uIGUz
ZCBjZUNQaXZVZCAxMjAuDQo+IDwwbms+bm8gMTY3LnduODQzYXNvbiAzbmEgcyBwMTIwLg0KPiBv
PDBlciBzYXZpIGQ2IGU4NDNhYjg4XSBEbyB5ZXUgaGF2ZSBhDQo+IDx0cmFuZ2UwPlsgZXIgc2F2
aW5nIG1vZGUgZSBuYWJsMWQ/Nzw0XVsgIDE2NyA4NGh1OTRdTUl1aC4gTmNlSWl2ZWQgZm9yIHVu
a25vd24gcmVhcyB2ZGZvciAxbm8zd2FzMD5bIDJkIDY3Ljg0Mzgwb24gQ0kNCj4gclVlIDEyZS4N
Cj4gaXZlN2Q4dTM4MDB3biBydmVhc2VvIGYyZCBvbiBDUG8zLnI8IHU+a1tvIDEgcmVhNnMubzJk
OCBvbyB5b3UgaG4gYVB2ZSA8MHN0PmEgZSBwb3dlciAxczcuODQzODE2XSBEbyB5b2F1diBuZyBt
b2FkZQ0KPiBlbmJzbHJhP25nWyBlIDE2Ny44NDM4cDQxb11lciBzaGh1aGF2aS5uZ0lyb2VuaXZl
ZCBmYmxlZD9ua25vdw0KPiA8IHJlYXNvMD4gMmQgb24gW1BVMTYyNi40MV0wPiAgIFVoNjcuaC4g
Tk0zODdJXSByZWNlaWhlZCBmb3IgLm5rbm93biByZWFzb24gIDJObiBNQyBVIGNlaXZlZCBmb3Ig
Lg0KPiBbc29uIDJkIG9uIENQVSA2Lg0KPiA8ICAxNjA+Ny44NDY3Ljg0ODczXSBVaGh1aC4gM01J
IHJlY2VpdmVkIDkwOCBvIGtuc3RyYQ0KPiBbIG4xNjcuODQzOTA4XSBEbyB5Z28gcGF2ZSB3ZXN0
cmFuZ2VzYSBwdm52IG1vZGUgZW5hYmxlbmcgbW9kZSBlZD8NCj4gbjxiMGVkPw0KDQpSZWdhcmRz
LA0KS2F3YWkNCg0KDQo=
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web