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

[Michal Hocko]

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@xxxxxxxxxxx>
> Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
> Cc: Borislav Petkov <bp@xxxxxxx>
> Cc: Michal Nazarewicz <mina86@xxxxxxxxxx>
> Cc: Michal Hocko <mhocko@xxxxxxxx>
> Cc: Rasmus Villemoes <linux@xxxxxxxxxxxxxxxxxx>
> Cc: Nicolas Iooss <nicolas.iooss_linux@xxxxxxx>
> Cc: Javi Merino <javi.merino@xxxxxxx>
> Cc: Gobinda Charan Maji <gobinda.cemk07@xxxxxxxxx>
> Cc: "Steven Rostedt (Red Hat)" <rostedt@xxxxxxxxxxx>
> Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> Cc: Vitaly Kuznetsov <vkuznets@xxxxxxxxxx>
> Cc: HATAYAMA Daisuke <d.hatayama@xxxxxxxxxxxxxx>
> Cc: Tejun Heo <tj@xxxxxxxxxx>

For the change itself
Acked-by: Michal Hocko <mhocko@xxxxxxxx>

> ---
>  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