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

Groups > linux.kernel > #1348989 > unrolled thread

[v3 PATCH 0/3] Use nmi_panic() in panic on NMI case

Back to article view | Back to linux.kernel

Contents

  [v3 PATCH 0/3] Use nmi_panic() in panic on NMI case Hidehiro Kawai <hidehiro.kawai.ez@hitachi.com> - 2016-03-03 12:10 +0100
    [v3 PATCH 1/3] panic: Change nmi_panic from macro to function Hidehiro Kawai <hidehiro.kawai.ez@hitachi.com> - 2016-03-03 12:10 +0100
      Re: [v3 PATCH 1/3] panic: Change nmi_panic from macro to function Borislav Petkov <bp@alien8.de> - 2016-03-03 14:20 +0100
        RE: [v3 PATCH 1/3] panic: Change nmi_panic from macro to function 河合英宏 / KAWAI，HIDEHIRO   <hidehiro.kawai.ez@hitachi.com> - 2016-03-07 12:00 +0100
          Re: [v3 PATCH 1/3] panic: Change nmi_panic from macro to function Borislav Petkov <bp@alien8.de> - 2016-03-07 12:20 +0100
      Re: [v3 PATCH 1/3] panic: Change nmi_panic from macro to function Michal Hocko <mhocko@kernel.org> - 2016-03-04 18:50 +0100
      Re: [v3 PATCH 1/3] panic: Change nmi_panic from macro to function Michal Nazarewicz <mina86@mina86.com> - 2016-03-04 20:30 +0100

#1348989 — [v3 PATCH 0/3] Use nmi_panic() in panic on NMI case

commit 1717f2096b54 ("panic, x86: Fix re-entrance problem due to
panic on NMI") and commit 58c5661f2144 ("panic, x86: Allow CPUs to
save registers even if looping in NMI context") introduced nmi_panic()
which prevents concurrent/recursive execution of panic().  It also
saves registers for the crash dump on x86.

However, there are some cases where NMI handlers still use panic().
This patch set partially replaces them with nmi_panic() in those
cases.

Changes since v2 (https://lkml.org/lkml/2016/3/2/173):
- Make nmi_panic receive a single string instead of printf style args
  (PATCH 1/3)

Changes since v1 (https://lkml.org/lkml/2016/2/29/858):
- Replace nmi_panic() macro with a function version instead of
  exporting symbols referred by the macro (PATCH 1/3)
- Improve the patch descriptions (PATCH 2/3 and 3/3)
- Do small cleanups (PATCH 3/3)

---
Even if applying this patch set, some NMI or similar handlers (e.g.
MCE handler) remains to use panic().  This is because I can't test
them well and actual problems won't happen.  For example, the
possibility that normal panic and panic on MCE happen simultaneously
is very low.

Hidehiro Kawai (3):
      panic: Change nmi_panic from macro to function
      ipmi/watchdog: Use nmi_panic() when kernel panics in NMI handler
      hpwdt: Use nmi_panic() when kernel panics in NMI handler


 drivers/char/ipmi/ipmi_watchdog.c |    2 +-
 drivers/watchdog/hpwdt.c          |   11 +++++------
 include/linux/kernel.h            |   21 +--------------------
 kernel/panic.c                    |   20 ++++++++++++++++++++
 4 files changed, 27 insertions(+), 27 deletions(-)


-- 
Hidehiro Kawai
Hitachi, Ltd. Research & Development Group

[toc] | [next] | [standalone]

#1348993 — [v3 PATCH 1/3] panic: Change nmi_panic from macro to function

Change nmi_panic() macro to a normal function for the portability.
Also, export it for modules.

Changes since v2:
- Make nmi_panic receive a single string instead of printf style args

Signed-off-by: Hidehiro Kawai <hidehiro.kawai.ez@hitachi.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Borislav Petkov <bp@suse.de>
Cc: Michal Nazarewicz <mina86@mina86.com>
Cc: Michal Hocko <mhocko@suse.com>
Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Cc: Nicolas Iooss <nicolas.iooss_linux@m4x.org>
Cc: Javi Merino <javi.merino@arm.com>
Cc: Gobinda Charan Maji <gobinda.cemk07@gmail.com>
Cc: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Vitaly Kuznetsov <vkuznets@redhat.com>
Cc: HATAYAMA Daisuke <d.hatayama@jp.fujitsu.com>
Cc: Tejun Heo <tj@kernel.org>
---
 include/linux/kernel.h |   21 +--------------------
 kernel/panic.c         |   20 ++++++++++++++++++++
 2 files changed, 21 insertions(+), 20 deletions(-)

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index f31638c..cbe7d70 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -255,7 +255,7 @@ extern long (*panic_blink)(int state);
 __printf(1, 2)
 void panic(const char *fmt, ...)
 	__noreturn __cold;
-void nmi_panic_self_stop(struct pt_regs *);
+void nmi_panic(struct pt_regs *regs, const char *msg);
 extern void oops_enter(void);
 extern void oops_exit(void);
 void print_oops_end_marker(void);
@@ -455,25 +455,6 @@ extern atomic_t panic_cpu;
 #define PANIC_CPU_INVALID	-1
 
 /*
- * A variant of panic() called from NMI context. We return if we've already
- * panicked on this CPU. If another CPU already panicked, loop in
- * nmi_panic_self_stop() which can provide architecture dependent code such
- * as saving register state for crash dump.
- */
-#define nmi_panic(regs, fmt, ...)					\
-do {									\
-	int old_cpu, cpu;						\
-									\
-	cpu = raw_smp_processor_id();					\
-	old_cpu = atomic_cmpxchg(&panic_cpu, PANIC_CPU_INVALID, cpu);	\
-									\
-	if (old_cpu == PANIC_CPU_INVALID)				\
-		panic(fmt, ##__VA_ARGS__);				\
-	else if (old_cpu != cpu)					\
-		nmi_panic_self_stop(regs);				\
-} 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 d96469d..8abfc30 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -72,6 +72,26 @@ void __weak nmi_panic_self_stop(struct pt_regs *regs)
 
 atomic_t panic_cpu = ATOMIC_INIT(PANIC_CPU_INVALID);
 
+/*
+ * A variant of panic() called from NMI context. We return if we've already
+ * panicked on this CPU. If another CPU already panicked, loop in
+ * nmi_panic_self_stop() which can provide architecture dependent code such
+ * as saving register state for crash dump.
+ */
+void nmi_panic(struct pt_regs *regs, const char *msg)
+{
+	int old_cpu, cpu;
+
+	cpu = raw_smp_processor_id();
+	old_cpu = atomic_cmpxchg(&panic_cpu, PANIC_CPU_INVALID, cpu);
+
+	if (old_cpu == PANIC_CPU_INVALID)
+		panic("%s", msg);
+	else if (old_cpu != cpu)
+		nmi_panic_self_stop(regs);
+}
+EXPORT_SYMBOL(nmi_panic);
+
 /**
  *	panic - halt the system
  *	@fmt: The text string to print

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

#1349149 — Re: [v3 PATCH 1/3] panic: Change nmi_panic from macro to function

On Thu, Mar 03, 2016 at 07:57:44PM +0900, Hidehiro Kawai wrote:
> Change nmi_panic() macro to a normal function for the portability.

portability?

> Also, export it for modules.
> 
> Changes since v2:
> - Make nmi_panic receive a single string instead of printf style args
> 
> Signed-off-by: Hidehiro Kawai <hidehiro.kawai.ez@hitachi.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Borislav Petkov <bp@suse.de>
> Cc: Michal Nazarewicz <mina86@mina86.com>
> Cc: Michal Hocko <mhocko@suse.com>
> Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> Cc: Nicolas Iooss <nicolas.iooss_linux@m4x.org>
> Cc: Javi Merino <javi.merino@arm.com>
> Cc: Gobinda Charan Maji <gobinda.cemk07@gmail.com>
> Cc: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Vitaly Kuznetsov <vkuznets@redhat.com>
> Cc: HATAYAMA Daisuke <d.hatayama@jp.fujitsu.com>
> Cc: Tejun Heo <tj@kernel.org>
> ---
>  include/linux/kernel.h |   21 +--------------------
>  kernel/panic.c         |   20 ++++++++++++++++++++
>  2 files changed, 21 insertions(+), 20 deletions(-)

Acked-by: Borislav Petkov <bp@suse.de>

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.

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

#1351529 — RE: [v3 PATCH 1/3] panic: Change nmi_panic from macro to function

> From: Borislav Petkov [mailto:bp@alien8.de]
> On Thu, Mar 03, 2016 at 07:57:44PM +0900, Hidehiro Kawai wrote:
> > Change nmi_panic() macro to a normal function for the portability.
> 
> portability?

I wanted to say encapsulating things into a function makes modules
only have to know about the function. Modules don't need to know
all things in the macro.  But I thought again, and `portability'
was not appropriate.

However, this patch set has been queued into -mm now.  So I'll
leave this if not necessary.

Regards,
Hidehiro Kawai

> > Also, export it for modules.
> >
> > Changes since v2:
> > - Make nmi_panic receive a single string instead of printf style args
> >
> > Signed-off-by: Hidehiro Kawai <hidehiro.kawai.ez@hitachi.com>
> > Cc: Andrew Morton <akpm@linux-foundation.org>
> > Cc: Borislav Petkov <bp@suse.de>
> > Cc: Michal Nazarewicz <mina86@mina86.com>
> > Cc: Michal Hocko <mhocko@suse.com>
> > Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> > Cc: Nicolas Iooss <nicolas.iooss_linux@m4x.org>
> > Cc: Javi Merino <javi.merino@arm.com>
> > Cc: Gobinda Charan Maji <gobinda.cemk07@gmail.com>
> > Cc: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>
> > Cc: Thomas Gleixner <tglx@linutronix.de>
> > Cc: Vitaly Kuznetsov <vkuznets@redhat.com>
> > Cc: HATAYAMA Daisuke <d.hatayama@jp.fujitsu.com>
> > Cc: Tejun Heo <tj@kernel.org>
> > ---
> >  include/linux/kernel.h |   21 +--------------------
> >  kernel/panic.c         |   20 ++++++++++++++++++++
> >  2 files changed, 21 insertions(+), 20 deletions(-)
> 
> Acked-by: Borislav Petkov <bp@suse.de>
> 
> --
> Regards/Gruss,
>     Boris.
> 
> ECO tip #101: Trim your mails when you reply.

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

#1351542 — Re: [v3 PATCH 1/3] panic: Change nmi_panic from macro to function

On Mon, Mar 07, 2016 at 10:53:51AM +0000, 河合英宏 / KAWAI，HIDEHIRO wrote:
> > From: Borislav Petkov [mailto:bp@alien8.de]
> > On Thu, Mar 03, 2016 at 07:57:44PM +0900, Hidehiro Kawai wrote:
> > > Change nmi_panic() macro to a normal function for the portability.
> > 
> > portability?
> 
> I wanted to say encapsulating things into a function makes modules
> only have to know about the function. Modules don't need to know
> all things in the macro.  But I thought again, and `portability'
> was not appropriate.
> 
> However, this patch set has been queued into -mm now.  So I'll
> leave this if not necessary.

Unless Andrew is willing to do a quick "quilt header -e" on your patch
and paste in a corrected commit message text which you give him...

:-)

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.

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

#1350472 — Re: [v3 PATCH 1/3] panic: Change nmi_panic from macro to function

On Thu 03-03-16 19:57:44, Hidehiro Kawai wrote:
> Change nmi_panic() macro to a normal function for the portability.
> Also, export it for modules.

I guess you wanted to say
"
Change nmi_panic() from macro to a normal function so that it can be
exported to modules. At least ipmi and hpwdt watchdogs can be compiled
as a module and need to panic from the NMI context.
"
> 
> Changes since v2:
> - Make nmi_panic receive a single string instead of printf style args
> 
> Signed-off-by: Hidehiro Kawai <hidehiro.kawai.ez@hitachi.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Borislav Petkov <bp@suse.de>
> Cc: Michal Nazarewicz <mina86@mina86.com>
> Cc: Michal Hocko <mhocko@suse.com>
> Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> Cc: Nicolas Iooss <nicolas.iooss_linux@m4x.org>
> Cc: Javi Merino <javi.merino@arm.com>
> Cc: Gobinda Charan Maji <gobinda.cemk07@gmail.com>
> Cc: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Vitaly Kuznetsov <vkuznets@redhat.com>
> Cc: HATAYAMA Daisuke <d.hatayama@jp.fujitsu.com>
> Cc: Tejun Heo <tj@kernel.org>

For the change itself
Acked-by: Michal Hocko <mhocko@suse.com>

> ---
>  include/linux/kernel.h |   21 +--------------------
>  kernel/panic.c         |   20 ++++++++++++++++++++
>  2 files changed, 21 insertions(+), 20 deletions(-)
> 
> diff --git a/include/linux/kernel.h b/include/linux/kernel.h
> index f31638c..cbe7d70 100644
> --- a/include/linux/kernel.h
> +++ b/include/linux/kernel.h
> @@ -255,7 +255,7 @@ extern long (*panic_blink)(int state);
>  __printf(1, 2)
>  void panic(const char *fmt, ...)
>  	__noreturn __cold;
> -void nmi_panic_self_stop(struct pt_regs *);
> +void nmi_panic(struct pt_regs *regs, const char *msg);
>  extern void oops_enter(void);
>  extern void oops_exit(void);
>  void print_oops_end_marker(void);
> @@ -455,25 +455,6 @@ extern atomic_t panic_cpu;
>  #define PANIC_CPU_INVALID	-1
>  
>  /*
> - * A variant of panic() called from NMI context. We return if we've already
> - * panicked on this CPU. If another CPU already panicked, loop in
> - * nmi_panic_self_stop() which can provide architecture dependent code such
> - * as saving register state for crash dump.
> - */
> -#define nmi_panic(regs, fmt, ...)					\
> -do {									\
> -	int old_cpu, cpu;						\
> -									\
> -	cpu = raw_smp_processor_id();					\
> -	old_cpu = atomic_cmpxchg(&panic_cpu, PANIC_CPU_INVALID, cpu);	\
> -									\
> -	if (old_cpu == PANIC_CPU_INVALID)				\
> -		panic(fmt, ##__VA_ARGS__);				\
> -	else if (old_cpu != cpu)					\
> -		nmi_panic_self_stop(regs);				\
> -} 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 d96469d..8abfc30 100644
> --- a/kernel/panic.c
> +++ b/kernel/panic.c
> @@ -72,6 +72,26 @@ void __weak nmi_panic_self_stop(struct pt_regs *regs)
>  
>  atomic_t panic_cpu = ATOMIC_INIT(PANIC_CPU_INVALID);
>  
> +/*
> + * A variant of panic() called from NMI context. We return if we've already
> + * panicked on this CPU. If another CPU already panicked, loop in
> + * nmi_panic_self_stop() which can provide architecture dependent code such
> + * as saving register state for crash dump.
> + */
> +void nmi_panic(struct pt_regs *regs, const char *msg)
> +{
> +	int old_cpu, cpu;
> +
> +	cpu = raw_smp_processor_id();
> +	old_cpu = atomic_cmpxchg(&panic_cpu, PANIC_CPU_INVALID, cpu);
> +
> +	if (old_cpu == PANIC_CPU_INVALID)
> +		panic("%s", msg);
> +	else if (old_cpu != cpu)
> +		nmi_panic_self_stop(regs);
> +}
> +EXPORT_SYMBOL(nmi_panic);
> +
>  /**
>   *	panic - halt the system
>   *	@fmt: The text string to print
> 
> 

-- 
Michal Hocko
SUSE Labs

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

#1350537 — Re: [v3 PATCH 1/3] panic: Change nmi_panic from macro to function

On Thu, Mar 03 2016, Hidehiro Kawai wrote:
> Change nmi_panic() macro to a normal function for the portability.
> Also, export it for modules.
>
> Changes since v2:
> - Make nmi_panic receive a single string instead of printf style args
>
> Signed-off-by: Hidehiro Kawai <hidehiro.kawai.ez@hitachi.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Borislav Petkov <bp@suse.de>
> Cc: Michal Nazarewicz <mina86@mina86.com>

Code does what the commit advertises so

Acked-by: Michal Nazarewicz <mina86@mina86.com>

> Cc: Michal Hocko <mhocko@suse.com>
> Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> Cc: Nicolas Iooss <nicolas.iooss_linux@m4x.org>
> Cc: Javi Merino <javi.merino@arm.com>
> Cc: Gobinda Charan Maji <gobinda.cemk07@gmail.com>
> Cc: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Vitaly Kuznetsov <vkuznets@redhat.com>
> Cc: HATAYAMA Daisuke <d.hatayama@jp.fujitsu.com>
> Cc: Tejun Heo <tj@kernel.org>
> ---
>  include/linux/kernel.h |   21 +-------------------
>  kernel/panic.c         |   20 ++++++++++++++++++++
>  2 files changed, 21 insertions(+), 20 deletions(-)
>
> diff --git a/include/linux/kernel.h b/include/linux/kernel.h
> index f31638c..cbe7d70 100644
> --- a/include/linux/kernel.h
> +++ b/include/linux/kernel.h
> @@ -255,7 +255,7 @@ extern long (*panic_blink)(int state);
>  __printf(1, 2)
>  void panic(const char *fmt, ...)
>  	__noreturn __cold;
> -void nmi_panic_self_stop(struct pt_regs *);
> +void nmi_panic(struct pt_regs *regs, const char *msg);
>  extern void oops_enter(void);
>  extern void oops_exit(void);
>  void print_oops_end_marker(void);
> @@ -455,25 +455,6 @@ extern atomic_t panic_cpu;
>  #define PANIC_CPU_INVALID	-1
>  
>  /*
> - * A variant of panic() called from NMI context. We return if we've already
> - * panicked on this CPU. If another CPU already panicked, loop in
> - * nmi_panic_self_stop() which can provide architecture dependent code such
> - * as saving register state for crash dump.
> - */
> -#define nmi_panic(regs, fmt, ...)					\
> -do {									\
> -	int old_cpu, cpu;						\
> -									\
> -	cpu = raw_smp_processor_id();					\
> -	old_cpu = atomic_cmpxchg(&panic_cpu, PANIC_CPU_INVALID, cpu);	\
> -									\
> -	if (old_cpu == PANIC_CPU_INVALID)				\
> -		panic(fmt, ##__VA_ARGS__);				\
> -	else if (old_cpu != cpu)					\
> -		nmi_panic_self_stop(regs);				\
> -} 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 d96469d..8abfc30 100644
> --- a/kernel/panic.c
> +++ b/kernel/panic.c
> @@ -72,6 +72,26 @@ void __weak nmi_panic_self_stop(struct pt_regs *regs)
>  
>  atomic_t panic_cpu = ATOMIC_INIT(PANIC_CPU_INVALID);
>  
> +/*
> + * A variant of panic() called from NMI context. We return if we've already
> + * panicked on this CPU. If another CPU already panicked, loop in
> + * nmi_panic_self_stop() which can provide architecture dependent code such
> + * as saving register state for crash dump.
> + */
> +void nmi_panic(struct pt_regs *regs, const char *msg)
> +{
> +	int old_cpu, cpu;
> +
> +	cpu = raw_smp_processor_id();
> +	old_cpu = atomic_cmpxchg(&panic_cpu, PANIC_CPU_INVALID, cpu);
> +
> +	if (old_cpu == PANIC_CPU_INVALID)
> +		panic("%s", msg);
> +	else if (old_cpu != cpu)
> +		nmi_panic_self_stop(regs);
> +}
> +EXPORT_SYMBOL(nmi_panic);
> +
>  /**
>   *	panic - halt the system
>   *	@fmt: The text string to print
>
>

-- 
Best regards
ミハウ “𝓶𝓲𝓷𝓪86” ナザレヴイツ
«If at first you don’t succeed, give up skydiving»

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web