Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1273951 > unrolled thread
| Started by | Hidehiro Kawai <hidehiro.kawai.ez@hitachi.com> |
|---|---|
| First post | 2015-11-20 11:30 +0100 |
| Last post | 2015-11-25 11:00 +0100 |
| Articles | 6 — 5 participants |
Back to article view | Back to linux.kernel
[V5 PATCH 0/4] Fix race issues among panic, NMI and crash_kexec Hidehiro Kawai <hidehiro.kawai.ez@hitachi.com> - 2015-11-20 11:30 +0100
[V5 PATCH 3/4] kexec: Fix race between panic() and crash_kexec() called directly Hidehiro Kawai <hidehiro.kawai.ez@hitachi.com> - 2015-11-20 11:30 +0100
Re: [V5 PATCH 3/4] kexec: Fix race between panic() and crash_kexec() called directly Michal Hocko <mhocko@kernel.org> - 2015-11-24 14:10 +0100
Re: [V5 PATCH 3/4] kexec: Fix race between panic() and crash_kexec() called directly Steven Rostedt <rostedt@goodmis.org> - 2015-11-24 21:40 +0100
RE: [V5 PATCH 3/4] kexec: Fix race between panic() and crash_kexec() called directly 河合英宏 / KAWAI，HIDEHIRO <hidehiro.kawai.ez@hitachi.com> - 2015-11-25 07:30 +0100
Re: [V5 PATCH 3/4] kexec: Fix race between panic() and crash_kexec() called directly Borislav Petkov <bp@alien8.de> - 2015-11-25 11:00 +0100
| From | Hidehiro Kawai <hidehiro.kawai.ez@hitachi.com> |
|---|---|
| Date | 2015-11-20 11:30 +0100 |
| Subject | [V5 PATCH 0/4] Fix race issues among panic, NMI and crash_kexec |
| Message-ID | <qwQum-5rq-19@gated-at.bofh.it> |
When an HA clustering software or administrator detects unresponsiveness
of a host, they issue an NMI to the host to completely stop current
works and take a crash dump. If the kernel has already panicked
or is capturing a crash dump at that time, further NMI can cause
a crash dump failure.
Also, crash_kexec() called from oops context and panic() can
cause race conditions.
To solve these issues, this patch set does following things:
- Don't call panic() on NMI if the kernel has already panicked
- Extend exclusion control currently done by panic_lock to crash_kexec
- Introduce "apic_extnmi=none" boot option which masks external NMI
NMI at the boot time
Additionally, "apic_extnmi=all" is provieded. This option unmasks
external NMI for all CPUs. This would help cause kernel panic even if
CPU 0 can't handle an external NMI due to hang-up in NMI context
or being handled by other NMI handlers.
This patch set can be applied to current -tip tree.
V5:
- Use WRITE_ONCE() for crash_ipi_done to keep the instruction order
(PATCH 2/4)
- Address concurrent unknown/external NMI case, too (PATCH 2/4)
- Fix build errors (PATCH 3/4)
- Rename "noextnmi" boot option to "apic_extnmi" and expand its
feature (PATCH 4/4)
V4: https://lkml.org/lkml/2015/9/25/193
- Improve comments and descriptions (PATCH 1/4 to 3/4)
- Use new __crash_kexec(), no exclusion check version of crash_kexec(),
instead of checking if panic_cpu is the current cpu or not
(PATCH 3/4)
V3: https://lkml.org/lkml/2015/8/6/39
- Introduce nmi_panic() macro to reduce code duplication
- In the case of panic on NMI, don't return from NMI handlers
if another cpu already panicked
V2: https://lkml.org/lkml/2015/7/27/31
- Use atomic_cmpxchg() instead of current spin_trylock() to exclude
concurrent accesses to panic() and crash_kexec()
- Don't introduce no-lock version of panic() and crash_kexec()
V1: https://lkml.org/lkml/2015/7/22/81
---
Hidehiro Kawai (4):
panic/x86: Fix re-entrance problem due to panic on NMI
panic/x86: Allow cpus to save registers even if they are looping in NMI context
kexec: Fix race between panic() and crash_kexec() called directly
x86/apic: Introduce apic_extnmi boot option
Documentation/kernel-parameters.txt | 9 +++++++++
arch/x86/include/asm/apic.h | 5 +++++
arch/x86/include/asm/reboot.h | 1 +
arch/x86/kernel/apic/apic.c | 31 ++++++++++++++++++++++++++++++-
arch/x86/kernel/nmi.c | 27 ++++++++++++++++++++++-----
arch/x86/kernel/reboot.c | 28 ++++++++++++++++++++++++++++
include/linux/kernel.h | 21 +++++++++++++++++++++
include/linux/kexec.h | 2 ++
kernel/kexec_core.c | 26 +++++++++++++++++++++++++-
kernel/panic.c | 29 ++++++++++++++++++++++++-----
kernel/watchdog.c | 2 +-
11 files changed, 168 insertions(+), 13 deletions(-)
--
Hidehiro Kawai
Hitachi, Ltd. Research & Development Group
--
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]
| From | Hidehiro Kawai <hidehiro.kawai.ez@hitachi.com> |
|---|---|
| Date | 2015-11-20 11:30 +0100 |
| Subject | [V5 PATCH 3/4] kexec: Fix race between panic() and crash_kexec() called directly |
| Message-ID | <qwR75-5WC-41@gated-at.bofh.it> |
| In reply to | #1273951 |
Currently, panic() and crash_kexec() can be called at the same time.
For example (x86 case):
CPU 0:
oops_end()
crash_kexec()
mutex_trylock() // acquired
nmi_shootdown_cpus() // stop other cpus
CPU 1:
panic()
crash_kexec()
mutex_trylock() // failed to acquire
smp_send_stop() // stop other cpus
infinite loop
If CPU 1 calls smp_send_stop() before nmi_shootdown_cpus(), kdump
fails.
In another case:
CPU 0:
oops_end()
crash_kexec()
mutex_trylock() // acquired
<NMI>
io_check_error()
panic()
crash_kexec()
mutex_trylock() // failed to acquire
infinite loop
Clearly, this is an undesirable result.
To fix this problem, this patch changes crash_kexec() to exclude
others by using atomic_t panic_cpu.
V5:
- Add missing dummy __crash_kexec() for !CONFIG_KEXEC_CORE case
- Replace atomic_xchg() with atomic_set() in crash_kexec() because
it is used as a release operation and there is no need of memory
barrier effect. This change also removes an unused value warning
V4:
- Use new __crash_kexec(), no exclusion check version of crash_kexec(),
instead of checking if panic_cpu is the current cpu or not
V2:
- Use atomic_cmpxchg() instead of spin_trylock() on panic_lock
to exclude concurrent accesses
- Don't introduce no-lock version of crash_kexec()
Signed-off-by: Hidehiro Kawai <hidehiro.kawai.ez@hitachi.com>
Cc: Eric Biederman <ebiederm@xmission.com>
Cc: Vivek Goyal <vgoyal@redhat.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Michal Hocko <mhocko@kernel.org>
---
include/linux/kexec.h | 2 ++
kernel/kexec_core.c | 26 +++++++++++++++++++++++++-
kernel/panic.c | 4 ++--
3 files changed, 29 insertions(+), 3 deletions(-)
diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index d140b1e..7b68d27 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -237,6 +237,7 @@ extern int kexec_purgatory_get_set_symbol(struct kimage *image,
unsigned int size, bool get_value);
extern void *kexec_purgatory_get_symbol_addr(struct kimage *image,
const char *name);
+extern void __crash_kexec(struct pt_regs *);
extern void crash_kexec(struct pt_regs *);
int kexec_should_crash(struct task_struct *);
void crash_save_cpu(struct pt_regs *regs, int cpu);
@@ -332,6 +333,7 @@ int __weak arch_kexec_apply_relocations(const Elf_Ehdr *ehdr, Elf_Shdr *sechdrs,
#else /* !CONFIG_KEXEC_CORE */
struct pt_regs;
struct task_struct;
+static inline void __crash_kexec(struct pt_regs *regs) { }
static inline void crash_kexec(struct pt_regs *regs) { }
static inline int kexec_should_crash(struct task_struct *p) { return 0; }
#define kexec_in_progress false
diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
index 11b64a6..9d097f5 100644
--- a/kernel/kexec_core.c
+++ b/kernel/kexec_core.c
@@ -853,7 +853,8 @@ struct kimage *kexec_image;
struct kimage *kexec_crash_image;
int kexec_load_disabled;
-void crash_kexec(struct pt_regs *regs)
+/* No panic_cpu check version of crash_kexec */
+void __crash_kexec(struct pt_regs *regs)
{
/* Take the kexec_mutex here to prevent sys_kexec_load
* running on one cpu from replacing the crash kernel
@@ -876,6 +877,29 @@ void crash_kexec(struct pt_regs *regs)
}
}
+void crash_kexec(struct pt_regs *regs)
+{
+ int old_cpu, this_cpu;
+
+ /*
+ * Only one CPU is allowed to execute the crash_kexec() code as with
+ * panic(). Otherwise parallel calls of panic() and crash_kexec()
+ * may stop each other. To exclude them, we use panic_cpu here too.
+ */
+ this_cpu = raw_smp_processor_id();
+ old_cpu = atomic_cmpxchg(&panic_cpu, -1, this_cpu);
+ if (old_cpu == -1) {
+ /* This is the 1st CPU which comes here, so go ahead. */
+ __crash_kexec(regs);
+
+ /*
+ * Reset panic_cpu to allow another panic()/crash_kexec()
+ * call.
+ */
+ atomic_set(&panic_cpu, -1);
+ }
+}
+
size_t crash_get_memory_size(void)
{
size_t size = 0;
diff --git a/kernel/panic.c b/kernel/panic.c
index 4fce2be..5d0b807 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -138,7 +138,7 @@ void panic(const char *fmt, ...)
* the "crash_kexec_post_notifiers" option to the kernel.
*/
if (!crash_kexec_post_notifiers)
- crash_kexec(NULL);
+ __crash_kexec(NULL);
/*
* Note smp_send_stop is the usual smp shutdown function, which
@@ -163,7 +163,7 @@ void panic(const char *fmt, ...)
* more unstable, it can increase risks of the kdump failure too.
*/
if (crash_kexec_post_notifiers)
- crash_kexec(NULL);
+ __crash_kexec(NULL);
bust_spinlocks(0);
--
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]
| From | Michal Hocko <mhocko@kernel.org> |
|---|---|
| Date | 2015-11-24 14:10 +0100 |
| Subject | Re: [V5 PATCH 3/4] kexec: Fix race between panic() and crash_kexec() called directly |
| Message-ID | <qylw6-5A-27@gated-at.bofh.it> |
| In reply to | #1273952 |
On Fri 20-11-15 18:36:48, Hidehiro Kawai wrote:
> Currently, panic() and crash_kexec() can be called at the same time.
> For example (x86 case):
>
> CPU 0:
> oops_end()
> crash_kexec()
> mutex_trylock() // acquired
> nmi_shootdown_cpus() // stop other cpus
>
> CPU 1:
> panic()
> crash_kexec()
> mutex_trylock() // failed to acquire
> smp_send_stop() // stop other cpus
> infinite loop
>
> If CPU 1 calls smp_send_stop() before nmi_shootdown_cpus(), kdump
> fails.
>
> In another case:
>
> CPU 0:
> oops_end()
> crash_kexec()
> mutex_trylock() // acquired
> <NMI>
> io_check_error()
> panic()
> crash_kexec()
> mutex_trylock() // failed to acquire
> infinite loop
>
> Clearly, this is an undesirable result.
>
> To fix this problem, this patch changes crash_kexec() to exclude
> others by using atomic_t panic_cpu.
>
> V5:
> - Add missing dummy __crash_kexec() for !CONFIG_KEXEC_CORE case
> - Replace atomic_xchg() with atomic_set() in crash_kexec() because
> it is used as a release operation and there is no need of memory
> barrier effect. This change also removes an unused value warning
>
> V4:
> - Use new __crash_kexec(), no exclusion check version of crash_kexec(),
> instead of checking if panic_cpu is the current cpu or not
>
> V2:
> - Use atomic_cmpxchg() instead of spin_trylock() on panic_lock
> to exclude concurrent accesses
> - Don't introduce no-lock version of crash_kexec()
>
> Signed-off-by: Hidehiro Kawai <hidehiro.kawai.ez@hitachi.com>
> Cc: Eric Biederman <ebiederm@xmission.com>
> Cc: Vivek Goyal <vgoyal@redhat.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Michal Hocko <mhocko@kernel.org>
Looks good to me as well
Acked-by: Michal Hocko <mhocko@suse.com>
[...]
> +void crash_kexec(struct pt_regs *regs)
> +{
> + int old_cpu, this_cpu;
> +
> + /*
> + * Only one CPU is allowed to execute the crash_kexec() code as with
> + * panic(). Otherwise parallel calls of panic() and crash_kexec()
> + * may stop each other. To exclude them, we use panic_cpu here too.
> + */
> + this_cpu = raw_smp_processor_id();
> + old_cpu = atomic_cmpxchg(&panic_cpu, -1, this_cpu);
> + if (old_cpu == -1) {
> + /* This is the 1st CPU which comes here, so go ahead. */
> + __crash_kexec(regs);
> +
> + /*
> + * Reset panic_cpu to allow another panic()/crash_kexec()
> + * call.
> + */
> + atomic_set(&panic_cpu, -1);
This was slighly more obvious in the previous version where the reset
happened after the trylock on the mutex failed, maybe the comment could
be more specific
+ /*
+ * Reset panic_cpu to allow another panic()/crash_kexec()
+ * call if __crash_kexec couldn't handle the situation.
+ */
--
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]
| From | Steven Rostedt <rostedt@goodmis.org> |
|---|---|
| Date | 2015-11-24 21:40 +0100 |
| Subject | Re: [V5 PATCH 3/4] kexec: Fix race between panic() and crash_kexec() called directly |
| Message-ID | <qysxz-4yx-1@gated-at.bofh.it> |
| In reply to | #1273952 |
On Fri, Nov 20, 2015 at 06:36:48PM +0900, Hidehiro Kawai wrote:
> Currently, panic() and crash_kexec() can be called at the same time.
> For example (x86 case):
>
> CPU 0:
> oops_end()
> crash_kexec()
> mutex_trylock() // acquired
> nmi_shootdown_cpus() // stop other cpus
>
> CPU 1:
> panic()
> crash_kexec()
> mutex_trylock() // failed to acquire
> smp_send_stop() // stop other cpus
> infinite loop
>
> If CPU 1 calls smp_send_stop() before nmi_shootdown_cpus(), kdump
> fails.
So the smp_send_stop() stops CPU 0 from calling nmi_shootdown_cpus(), right?
>
> In another case:
>
> CPU 0:
> oops_end()
> crash_kexec()
> mutex_trylock() // acquired
> <NMI>
> io_check_error()
> panic()
> crash_kexec()
> mutex_trylock() // failed to acquire
> infinite loop
>
> Clearly, this is an undesirable result.
I'm trying to see how this patch fixes this case.
>
> To fix this problem, this patch changes crash_kexec() to exclude
> others by using atomic_t panic_cpu.
>
> V5:
> - Add missing dummy __crash_kexec() for !CONFIG_KEXEC_CORE case
> - Replace atomic_xchg() with atomic_set() in crash_kexec() because
> it is used as a release operation and there is no need of memory
> barrier effect. This change also removes an unused value warning
>
> V4:
> - Use new __crash_kexec(), no exclusion check version of crash_kexec(),
> instead of checking if panic_cpu is the current cpu or not
>
> V2:
> - Use atomic_cmpxchg() instead of spin_trylock() on panic_lock
> to exclude concurrent accesses
> - Don't introduce no-lock version of crash_kexec()
>
> Signed-off-by: Hidehiro Kawai <hidehiro.kawai.ez@hitachi.com>
> Cc: Eric Biederman <ebiederm@xmission.com>
> Cc: Vivek Goyal <vgoyal@redhat.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Michal Hocko <mhocko@kernel.org>
> ---
> include/linux/kexec.h | 2 ++
> kernel/kexec_core.c | 26 +++++++++++++++++++++++++-
> kernel/panic.c | 4 ++--
> 3 files changed, 29 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/kexec.h b/include/linux/kexec.h
> index d140b1e..7b68d27 100644
> --- a/include/linux/kexec.h
> +++ b/include/linux/kexec.h
> @@ -237,6 +237,7 @@ extern int kexec_purgatory_get_set_symbol(struct kimage *image,
> unsigned int size, bool get_value);
> extern void *kexec_purgatory_get_symbol_addr(struct kimage *image,
> const char *name);
> +extern void __crash_kexec(struct pt_regs *);
> extern void crash_kexec(struct pt_regs *);
> int kexec_should_crash(struct task_struct *);
> void crash_save_cpu(struct pt_regs *regs, int cpu);
> @@ -332,6 +333,7 @@ int __weak arch_kexec_apply_relocations(const Elf_Ehdr *ehdr, Elf_Shdr *sechdrs,
> #else /* !CONFIG_KEXEC_CORE */
> struct pt_regs;
> struct task_struct;
> +static inline void __crash_kexec(struct pt_regs *regs) { }
> static inline void crash_kexec(struct pt_regs *regs) { }
> static inline int kexec_should_crash(struct task_struct *p) { return 0; }
> #define kexec_in_progress false
> diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
> index 11b64a6..9d097f5 100644
> --- a/kernel/kexec_core.c
> +++ b/kernel/kexec_core.c
> @@ -853,7 +853,8 @@ struct kimage *kexec_image;
> struct kimage *kexec_crash_image;
> int kexec_load_disabled;
>
> -void crash_kexec(struct pt_regs *regs)
> +/* No panic_cpu check version of crash_kexec */
> +void __crash_kexec(struct pt_regs *regs)
> {
> /* Take the kexec_mutex here to prevent sys_kexec_load
> * running on one cpu from replacing the crash kernel
> @@ -876,6 +877,29 @@ void crash_kexec(struct pt_regs *regs)
> }
> }
>
> +void crash_kexec(struct pt_regs *regs)
> +{
> + int old_cpu, this_cpu;
> +
> + /*
> + * Only one CPU is allowed to execute the crash_kexec() code as with
> + * panic(). Otherwise parallel calls of panic() and crash_kexec()
> + * may stop each other. To exclude them, we use panic_cpu here too.
> + */
> + this_cpu = raw_smp_processor_id();
> + old_cpu = atomic_cmpxchg(&panic_cpu, -1, this_cpu);
> + if (old_cpu == -1) {
> + /* This is the 1st CPU which comes here, so go ahead. */
> + __crash_kexec(regs);
> +
> + /*
> + * Reset panic_cpu to allow another panic()/crash_kexec()
> + * call.
> + */
> + atomic_set(&panic_cpu, -1);
> + }
> +}
> +
> size_t crash_get_memory_size(void)
> {
> size_t size = 0;
> diff --git a/kernel/panic.c b/kernel/panic.c
> index 4fce2be..5d0b807 100644
> --- a/kernel/panic.c
> +++ b/kernel/panic.c
> @@ -138,7 +138,7 @@ void panic(const char *fmt, ...)
> * the "crash_kexec_post_notifiers" option to the kernel.
> */
> if (!crash_kexec_post_notifiers)
> - crash_kexec(NULL);
> + __crash_kexec(NULL);
Why call the __crash_kexec() version and not just crash_kexec() here.
This needs to be documented.
>
> /*
> * Note smp_send_stop is the usual smp shutdown function, which
> @@ -163,7 +163,7 @@ void panic(const char *fmt, ...)
> * more unstable, it can increase risks of the kdump failure too.
> */
> if (crash_kexec_post_notifiers)
> - crash_kexec(NULL);
> + __crash_kexec(NULL);
ditto.
-- Steve
>
> bust_spinlocks(0);
>
>
--
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]
| From | 河合英宏 / KAWAI，HIDEHIRO <hidehiro.kawai.ez@hitachi.com> |
|---|---|
| Date | 2015-11-25 07:30 +0100 |
| Subject | RE: [V5 PATCH 3/4] kexec: Fix race between panic() and crash_kexec() called directly |
| Message-ID | <qyBKy-2oi-5@gated-at.bofh.it> |
| In reply to | #1276753 |
PiBPbiBGcmksIE5vdiAyMCwgMjAxNSBhdCAwNjozNjo0OFBNICswOTAwLCBIaWRlaGlybyBLYXdh aSB3cm90ZToNCj4gPiBDdXJyZW50bHksIHBhbmljKCkgYW5kIGNyYXNoX2tleGVjKCkgY2FuIGJl IGNhbGxlZCBhdCB0aGUgc2FtZSB0aW1lLg0KPiA+IEZvciBleGFtcGxlICh4ODYgY2FzZSk6DQo+ ID4NCj4gPiBDUFUgMDoNCj4gPiAgIG9vcHNfZW5kKCkNCj4gPiAgICAgY3Jhc2hfa2V4ZWMoKQ0K PiA+ICAgICAgIG11dGV4X3RyeWxvY2soKSAvLyBhY3F1aXJlZA0KPiA+ICAgICAgICAgbm1pX3No b290ZG93bl9jcHVzKCkgLy8gc3RvcCBvdGhlciBjcHVzDQo+ID4NCj4gPiBDUFUgMToNCj4gPiAg IHBhbmljKCkNCj4gPiAgICAgY3Jhc2hfa2V4ZWMoKQ0KPiA+ICAgICAgIG11dGV4X3RyeWxvY2so KSAvLyBmYWlsZWQgdG8gYWNxdWlyZQ0KPiA+ICAgICBzbXBfc2VuZF9zdG9wKCkgLy8gc3RvcCBv dGhlciBjcHVzDQo+ID4gICAgIGluZmluaXRlIGxvb3ANCj4gPg0KPiA+IElmIENQVSAxIGNhbGxz IHNtcF9zZW5kX3N0b3AoKSBiZWZvcmUgbm1pX3Nob290ZG93bl9jcHVzKCksIGtkdW1wDQo+ID4g ZmFpbHMuDQo+IA0KPiBTbyB0aGUgc21wX3NlbmRfc3RvcCgpIHN0b3BzIENQVSAwIGZyb20gY2Fs bGluZyBubWlfc2hvb3Rkb3duX2NwdXMoKSwgcmlnaHQ/DQoNClllcywgYnV0IHRoZSBpbXBvcnRh bnQgdGhpbmcgaXMgdGhhdCBDUFUgMSBzdG9wcyBDUFUgMCB3aGljaCBpcw0Kb25seSBDUFUgcHJv Y2Vzc2luZyBjcmFzaF8ga2V4ZWMgcm91dGluZXMuDQoNCj4gPg0KPiA+IEluIGFub3RoZXIgY2Fz ZToNCj4gPg0KPiA+IENQVSAwOg0KPiA+ICAgb29wc19lbmQoKQ0KPiA+ICAgICBjcmFzaF9rZXhl YygpDQo+ID4gICAgICAgbXV0ZXhfdHJ5bG9jaygpIC8vIGFjcXVpcmVkDQo+ID4gICAgICAgICA8 Tk1JPg0KPiA+ICAgICAgICAgaW9fY2hlY2tfZXJyb3IoKQ0KPiA+ICAgICAgICAgICBwYW5pYygp DQo+ID4gICAgICAgICAgICAgY3Jhc2hfa2V4ZWMoKQ0KPiA+ICAgICAgICAgICAgICAgbXV0ZXhf dHJ5bG9jaygpIC8vIGZhaWxlZCB0byBhY3F1aXJlDQo+ID4gICAgICAgICAgICAgaW5maW5pdGUg bG9vcA0KPiA+DQo+ID4gQ2xlYXJseSwgdGhpcyBpcyBhbiB1bmRlc2lyYWJsZSByZXN1bHQuDQo+ IA0KPiBJJ20gdHJ5aW5nIHRvIHNlZSBob3cgdGhpcyBwYXRjaCBmaXhlcyB0aGlzIGNhc2UuDQo+ IA0KPiA+DQo+ID4gVG8gZml4IHRoaXMgcHJvYmxlbSwgdGhpcyBwYXRjaCBjaGFuZ2VzIGNyYXNo X2tleGVjKCkgdG8gZXhjbHVkZQ0KPiA+IG90aGVycyBieSB1c2luZyBhdG9taWNfdCBwYW5pY19j cHUuDQo+ID4NCj4gPiBWNToNCj4gPiAtIEFkZCBtaXNzaW5nIGR1bW15IF9fY3Jhc2hfa2V4ZWMo KSBmb3IgIUNPTkZJR19LRVhFQ19DT1JFIGNhc2UNCj4gPiAtIFJlcGxhY2UgYXRvbWljX3hjaGco KSB3aXRoIGF0b21pY19zZXQoKSBpbiBjcmFzaF9rZXhlYygpIGJlY2F1c2UNCj4gPiAgIGl0IGlz IHVzZWQgYXMgYSByZWxlYXNlIG9wZXJhdGlvbiBhbmQgdGhlcmUgaXMgbm8gbmVlZCBvZiBtZW1v cnkNCj4gPiAgIGJhcnJpZXIgZWZmZWN0LiAgVGhpcyBjaGFuZ2UgYWxzbyByZW1vdmVzIGFuIHVu dXNlZCB2YWx1ZSB3YXJuaW5nDQo+ID4NCj4gPiBWNDoNCj4gPiAtIFVzZSBuZXcgX19jcmFzaF9r ZXhlYygpLCBubyBleGNsdXNpb24gY2hlY2sgdmVyc2lvbiBvZiBjcmFzaF9rZXhlYygpLA0KPiA+ ICAgaW5zdGVhZCBvZiBjaGVja2luZyBpZiBwYW5pY19jcHUgaXMgdGhlIGN1cnJlbnQgY3B1IG9y IG5vdA0KPiA+DQo+ID4gVjI6DQo+ID4gLSBVc2UgYXRvbWljX2NtcHhjaGcoKSBpbnN0ZWFkIG9m IHNwaW5fdHJ5bG9jaygpIG9uIHBhbmljX2xvY2sNCj4gPiAgIHRvIGV4Y2x1ZGUgY29uY3VycmVu dCBhY2Nlc3Nlcw0KPiA+IC0gRG9uJ3QgaW50cm9kdWNlIG5vLWxvY2sgdmVyc2lvbiBvZiBjcmFz aF9rZXhlYygpDQo+ID4NCj4gPiBTaWduZWQtb2ZmLWJ5OiBIaWRlaGlybyBLYXdhaSA8aGlkZWhp cm8ua2F3YWkuZXpAaGl0YWNoaS5jb20+DQo+ID4gQ2M6IEVyaWMgQmllZGVybWFuIDxlYmllZGVy bUB4bWlzc2lvbi5jb20+DQo+ID4gQ2M6IFZpdmVrIEdveWFsIDx2Z295YWxAcmVkaGF0LmNvbT4N Cj4gPiBDYzogQW5kcmV3IE1vcnRvbiA8YWtwbUBsaW51eC1mb3VuZGF0aW9uLm9yZz4NCj4gPiBD YzogTWljaGFsIEhvY2tvIDxtaG9ja29Aa2VybmVsLm9yZz4NCj4gPiAtLS0NCj4gPiAgaW5jbHVk ZS9saW51eC9rZXhlYy5oIHwgICAgMiArKw0KPiA+ICBrZXJuZWwva2V4ZWNfY29yZS5jICAgfCAg IDI2ICsrKysrKysrKysrKysrKysrKysrKysrKystDQo+ID4gIGtlcm5lbC9wYW5pYy5jICAgICAg ICB8ICAgIDQgKystLQ0KPiA+ICAzIGZpbGVzIGNoYW5nZWQsIDI5IGluc2VydGlvbnMoKyksIDMg ZGVsZXRpb25zKC0pDQo+ID4NCj4gPiBkaWZmIC0tZ2l0IGEvaW5jbHVkZS9saW51eC9rZXhlYy5o IGIvaW5jbHVkZS9saW51eC9rZXhlYy5oDQo+ID4gaW5kZXggZDE0MGIxZS4uN2I2OGQyNyAxMDA2 NDQNCj4gPiAtLS0gYS9pbmNsdWRlL2xpbnV4L2tleGVjLmgNCj4gPiArKysgYi9pbmNsdWRlL2xp bnV4L2tleGVjLmgNCj4gPiBAQCAtMjM3LDYgKzIzNyw3IEBAIGV4dGVybiBpbnQga2V4ZWNfcHVy Z2F0b3J5X2dldF9zZXRfc3ltYm9sKHN0cnVjdCBraW1hZ2UgKmltYWdlLA0KPiA+ICAJCQkJCSAg dW5zaWduZWQgaW50IHNpemUsIGJvb2wgZ2V0X3ZhbHVlKTsNCj4gPiAgZXh0ZXJuIHZvaWQgKmtl eGVjX3B1cmdhdG9yeV9nZXRfc3ltYm9sX2FkZHIoc3RydWN0IGtpbWFnZSAqaW1hZ2UsDQo+ID4g IAkJCQkJICAgICBjb25zdCBjaGFyICpuYW1lKTsNCj4gPiArZXh0ZXJuIHZvaWQgX19jcmFzaF9r ZXhlYyhzdHJ1Y3QgcHRfcmVncyAqKTsNCj4gPiAgZXh0ZXJuIHZvaWQgY3Jhc2hfa2V4ZWMoc3Ry dWN0IHB0X3JlZ3MgKik7DQo+ID4gIGludCBrZXhlY19zaG91bGRfY3Jhc2goc3RydWN0IHRhc2tf c3RydWN0ICopOw0KPiA+ICB2b2lkIGNyYXNoX3NhdmVfY3B1KHN0cnVjdCBwdF9yZWdzICpyZWdz LCBpbnQgY3B1KTsNCj4gPiBAQCAtMzMyLDYgKzMzMyw3IEBAIGludCBfX3dlYWsgYXJjaF9rZXhl Y19hcHBseV9yZWxvY2F0aW9ucyhjb25zdCBFbGZfRWhkciAqZWhkciwgRWxmX1NoZHIgKnNlY2hk cnMsDQo+ID4gICNlbHNlIC8qICFDT05GSUdfS0VYRUNfQ09SRSAqLw0KPiA+ICBzdHJ1Y3QgcHRf cmVnczsNCj4gPiAgc3RydWN0IHRhc2tfc3RydWN0Ow0KPiA+ICtzdGF0aWMgaW5saW5lIHZvaWQg X19jcmFzaF9rZXhlYyhzdHJ1Y3QgcHRfcmVncyAqcmVncykgeyB9DQo+ID4gIHN0YXRpYyBpbmxp bmUgdm9pZCBjcmFzaF9rZXhlYyhzdHJ1Y3QgcHRfcmVncyAqcmVncykgeyB9DQo+ID4gIHN0YXRp YyBpbmxpbmUgaW50IGtleGVjX3Nob3VsZF9jcmFzaChzdHJ1Y3QgdGFza19zdHJ1Y3QgKnApIHsg cmV0dXJuIDA7IH0NCj4gPiAgI2RlZmluZSBrZXhlY19pbl9wcm9ncmVzcyBmYWxzZQ0KPiA+IGRp ZmYgLS1naXQgYS9rZXJuZWwva2V4ZWNfY29yZS5jIGIva2VybmVsL2tleGVjX2NvcmUuYw0KPiA+ IGluZGV4IDExYjY0YTYuLjlkMDk3ZjUgMTAwNjQ0DQo+ID4gLS0tIGEva2VybmVsL2tleGVjX2Nv cmUuYw0KPiA+ICsrKyBiL2tlcm5lbC9rZXhlY19jb3JlLmMNCj4gPiBAQCAtODUzLDcgKzg1Myw4 IEBAIHN0cnVjdCBraW1hZ2UgKmtleGVjX2ltYWdlOw0KPiA+ICBzdHJ1Y3Qga2ltYWdlICprZXhl Y19jcmFzaF9pbWFnZTsNCj4gPiAgaW50IGtleGVjX2xvYWRfZGlzYWJsZWQ7DQo+ID4NCj4gPiAt dm9pZCBjcmFzaF9rZXhlYyhzdHJ1Y3QgcHRfcmVncyAqcmVncykNCj4gPiArLyogTm8gcGFuaWNf Y3B1IGNoZWNrIHZlcnNpb24gb2YgY3Jhc2hfa2V4ZWMgKi8NCj4gPiArdm9pZCBfX2NyYXNoX2tl eGVjKHN0cnVjdCBwdF9yZWdzICpyZWdzKQ0KPiA+ICB7DQo+ID4gIAkvKiBUYWtlIHRoZSBrZXhl Y19tdXRleCBoZXJlIHRvIHByZXZlbnQgc3lzX2tleGVjX2xvYWQNCj4gPiAgCSAqIHJ1bm5pbmcg b24gb25lIGNwdSBmcm9tIHJlcGxhY2luZyB0aGUgY3Jhc2gga2VybmVsDQo+ID4gQEAgLTg3Niw2 ICs4NzcsMjkgQEAgdm9pZCBjcmFzaF9rZXhlYyhzdHJ1Y3QgcHRfcmVncyAqcmVncykNCj4gPiAg CX0NCj4gPiAgfQ0KPiA+DQo+ID4gK3ZvaWQgY3Jhc2hfa2V4ZWMoc3RydWN0IHB0X3JlZ3MgKnJl Z3MpDQo+ID4gK3sNCj4gPiArCWludCBvbGRfY3B1LCB0aGlzX2NwdTsNCj4gPiArDQo+ID4gKwkv Kg0KPiA+ICsJICogT25seSBvbmUgQ1BVIGlzIGFsbG93ZWQgdG8gZXhlY3V0ZSB0aGUgY3Jhc2hf a2V4ZWMoKSBjb2RlIGFzIHdpdGgNCj4gPiArCSAqIHBhbmljKCkuICBPdGhlcndpc2UgcGFyYWxs ZWwgY2FsbHMgb2YgcGFuaWMoKSBhbmQgY3Jhc2hfa2V4ZWMoKQ0KPiA+ICsJICogbWF5IHN0b3Ag ZWFjaCBvdGhlci4gIFRvIGV4Y2x1ZGUgdGhlbSwgd2UgdXNlIHBhbmljX2NwdSBoZXJlIHRvby4N Cj4gPiArCSAqLw0KPiA+ICsJdGhpc19jcHUgPSByYXdfc21wX3Byb2Nlc3Nvcl9pZCgpOw0KPiA+ ICsJb2xkX2NwdSA9IGF0b21pY19jbXB4Y2hnKCZwYW5pY19jcHUsIC0xLCB0aGlzX2NwdSk7DQo+ ID4gKwlpZiAob2xkX2NwdSA9PSAtMSkgew0KPiA+ICsJCS8qIFRoaXMgaXMgdGhlIDFzdCBDUFUg d2hpY2ggY29tZXMgaGVyZSwgc28gZ28gYWhlYWQuICovDQo+ID4gKwkJX19jcmFzaF9rZXhlYyhy ZWdzKTsNCj4gPiArDQo+ID4gKwkJLyoNCj4gPiArCQkgKiBSZXNldCBwYW5pY19jcHUgdG8gYWxs b3cgYW5vdGhlciBwYW5pYygpL2NyYXNoX2tleGVjKCkNCj4gPiArCQkgKiBjYWxsLg0KPiA+ICsJ CSAqLw0KPiA+ICsJCWF0b21pY19zZXQoJnBhbmljX2NwdSwgLTEpOw0KPiA+ICsJfQ0KPiA+ICt9 DQo+ID4gKw0KPiA+ICBzaXplX3QgY3Jhc2hfZ2V0X21lbW9yeV9zaXplKHZvaWQpDQo+ID4gIHsN Cj4gPiAgCXNpemVfdCBzaXplID0gMDsNCj4gPiBkaWZmIC0tZ2l0IGEva2VybmVsL3BhbmljLmMg Yi9rZXJuZWwvcGFuaWMuYw0KPiA+IGluZGV4IDRmY2UyYmUuLjVkMGI4MDcgMTAwNjQ0DQo+ID4g LS0tIGEva2VybmVsL3BhbmljLmMNCj4gPiArKysgYi9rZXJuZWwvcGFuaWMuYw0KPiA+IEBAIC0x MzgsNyArMTM4LDcgQEAgdm9pZCBwYW5pYyhjb25zdCBjaGFyICpmbXQsIC4uLikNCj4gPiAgCSAq IHRoZSAiY3Jhc2hfa2V4ZWNfcG9zdF9ub3RpZmllcnMiIG9wdGlvbiB0byB0aGUga2VybmVsLg0K PiA+ICAJICovDQo+ID4gIAlpZiAoIWNyYXNoX2tleGVjX3Bvc3Rfbm90aWZpZXJzKQ0KPiA+IC0J CWNyYXNoX2tleGVjKE5VTEwpOw0KPiA+ICsJCV9fY3Jhc2hfa2V4ZWMoTlVMTCk7DQo+IA0KPiBX aHkgY2FsbCB0aGUgX19jcmFzaF9rZXhlYygpIHZlcnNpb24gYW5kIG5vdCBqdXN0IGNyYXNoX2tl eGVjKCkgaGVyZS4NCj4gVGhpcyBuZWVkcyB0byBiZSBkb2N1bWVudGVkLg0KDQpJbiB0aGlzIHBh dGNoLCBhbiBleGNsdXNpdmUgZXhlY3V0aW9uIGNvbnRyb2wgd2l0aCBwYW5pY19jcHUNCmlzIGFk ZGVkIHRvIGNyYXNoX2tleGVjKCkuIFdoZW4gY3Jhc2hfa2V4ZWMoKSBpcyBjYWxsZWQgZnJvbQ0K cGFuaWMoKSwgd2UgZG9uJ3QgbmVlZCB0byBjaGVjayBwYW5pY19jcHUgYmVjYXVzZSB3ZSBoYXZl IGFscmVhZHkNCmhlbGQgdGhlIGV4Y2x1c2l2ZSBjb250cm9sLiBTbywgX19jcmFzaF9rZXhlYygp IGlzIHVzZWQgaGVyZQ0KdG8gYnlwYXNzIGl0Lg0KDQpPZiBjb3Vyc2UsIHdlIGNhbiBjYWxsIGNy YXNoX2tleGVjKCkgaGVyZSwgYW5kIGNyYXNoX2tleGVjKCkNCmNoZWNrcyBpZiBwYW5pY19jcHUg aXMgZXF1YWwgdG8gdGhlIGN1cnJlbnQgQ1BVIG51bWJlciwgYW5kDQppZiBzbywgY29udGludWVz IHRvIHByb2Nlc3MgY3Jhc2hfa2V4ZWMoKSByb3V0aW5lcy4NClRoaXMgd2FzIGRvbmUgaW4gb2xk ZXIgdmVyc2lvbiBvZiB0aGlzIHBhdGNoIHNlcmllcywgYnV0DQpQZXRlciByZWNlaXZlZCBhIHdy b25nIGltcHJlc3Npb24gYWJvdXQgY2hlY2tpbmcgaWYgcGFuaWNfY3B1DQppcyBlcXVhbCB0byB0 aGUgY3VycmVudCBDUFUgbnVtYmVyOyBpdCBpbXBsaWVzIHRoYXQgaXQgcGVybWl0cw0KcmVjdXJz aXZlIGNhbGwgb2YgY3Jhc2hfa2V4ZWMoKSAoYWN0dWFsbHkgcmVjdXJzaXZlIGNhbGwgb2YNCmNy YXNoX2tleGVjKCkgY2FuJ3QgaGFwcGVuKS4NCg0KQW55d2F5LCBJJ2xsIGFkZCBzb21lIGNvbW1l bnRzLg0KIA0KUmVnYXJkcywNCg0KLS0NCkhpZGVoaXJvIEthd2FpDQpIaXRhY2hpLCBMdGQuIFJl c2VhcmNoICYgRGV2ZWxvcG1lbnQgR3JvdXANCg0K -- 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]
| From | Borislav Petkov <bp@alien8.de> |
|---|---|
| Date | 2015-11-25 11:00 +0100 |
| Subject | Re: [V5 PATCH 3/4] kexec: Fix race between panic() and crash_kexec() called directly |
| Message-ID | <qyF1M-4qs-7@gated-at.bofh.it> |
| In reply to | #1273952 |
On Fri, Nov 20, 2015 at 06:36:48PM +0900, Hidehiro Kawai wrote:
> Currently, panic() and crash_kexec() can be called at the same time.
> For example (x86 case):
>
> CPU 0:
> oops_end()
> crash_kexec()
> mutex_trylock() // acquired
> nmi_shootdown_cpus() // stop other cpus
>
> CPU 1:
> panic()
> crash_kexec()
> mutex_trylock() // failed to acquire
> smp_send_stop() // stop other cpus
> infinite loop
>
> If CPU 1 calls smp_send_stop() before nmi_shootdown_cpus(), kdump
> fails.
>
> In another case:
>
> CPU 0:
> oops_end()
> crash_kexec()
> mutex_trylock() // acquired
> <NMI>
> io_check_error()
> panic()
> crash_kexec()
> mutex_trylock() // failed to acquire
> infinite loop
>
> Clearly, this is an undesirable result.
>
> To fix this problem, this patch changes crash_kexec() to exclude
> others by using atomic_t panic_cpu.
>
> V5:
> - Add missing dummy __crash_kexec() for !CONFIG_KEXEC_CORE case
> - Replace atomic_xchg() with atomic_set() in crash_kexec() because
> it is used as a release operation and there is no need of memory
> barrier effect. This change also removes an unused value warning
>
> V4:
> - Use new __crash_kexec(), no exclusion check version of crash_kexec(),
> instead of checking if panic_cpu is the current cpu or not
>
> V2:
> - Use atomic_cmpxchg() instead of spin_trylock() on panic_lock
> to exclude concurrent accesses
> - Don't introduce no-lock version of crash_kexec()
>
> Signed-off-by: Hidehiro Kawai <hidehiro.kawai.ez@hitachi.com>
> Cc: Eric Biederman <ebiederm@xmission.com>
> Cc: Vivek Goyal <vgoyal@redhat.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Michal Hocko <mhocko@kernel.org>
> ---
> include/linux/kexec.h | 2 ++
> kernel/kexec_core.c | 26 +++++++++++++++++++++++++-
> kernel/panic.c | 4 ++--
> 3 files changed, 29 insertions(+), 3 deletions(-)
...
> +void crash_kexec(struct pt_regs *regs)
> +{
> + int old_cpu, this_cpu;
> +
> + /*
> + * Only one CPU is allowed to execute the crash_kexec() code as with
> + * panic(). Otherwise parallel calls of panic() and crash_kexec()
> + * may stop each other. To exclude them, we use panic_cpu here too.
> + */
> + this_cpu = raw_smp_processor_id();
> + old_cpu = atomic_cmpxchg(&panic_cpu, -1, this_cpu);
> + if (old_cpu == -1) {
> + /* This is the 1st CPU which comes here, so go ahead. */
> + __crash_kexec(regs);
> +
> + /*
> + * Reset panic_cpu to allow another panic()/crash_kexec()
> + * call.
So can we make __crash_kexec() return error values?
* failed to grab kexec_mutex -> reset panic_cpu
* no kexec_crash_image -> no need to reset it, all future crash_kexec()
calls won't work so no need to run into that path anymore. However, this could
be problematic if we want the other CPUs to panic. Do we care?
* machine_kexec successful -> doesn't matter
Thanks.
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--
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