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

Groups > linux.kernel > #1346290 > unrolled thread

[PATCH 2/3] ipmi/watchdog: Use nmi_panic() when kernel panics in NMI handler

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

  [PATCH 2/3] ipmi/watchdog: Use nmi_panic() when kernel panics in  NMI handler Hidehiro Kawai <hidehiro.kawai.ez@hitachi.com> - 2016-03-01 03:00 +0100
    Re: [PATCH 2/3] ipmi/watchdog: Use nmi_panic() when kernel panics in  NMI handler Guenter Roeck <linux@roeck-us.net> - 2016-03-01 04:00 +0100
    Re: [PATCH 2/3] ipmi/watchdog: Use nmi_panic() when kernel panics in  NMI handler Corey Minyard <minyard@acm.org> - 2016-03-01 04:40 +0100
      RE: [PATCH 2/3] ipmi/watchdog: Use nmi_panic() when kernel panics  in NMI handler 河合英宏 / KAWAI，HIDEHIRO   <hidehiro.kawai.ez@hitachi.com> - 2016-03-01 05:30 +0100
        Re: [PATCH 2/3] ipmi/watchdog: Use nmi_panic() when kernel panics in  NMI handler Guenter Roeck <linux@roeck-us.net> - 2016-03-01 06:20 +0100
    Re: [PATCH 2/3] ipmi/watchdog: Use nmi_panic() when kernel panics in  NMI handler Michal Hocko <mhocko@kernel.org> - 2016-03-01 10:00 +0100

#1346290 — [PATCH 2/3] ipmi/watchdog: Use nmi_panic() when kernel panics in NMI handler

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.

ipmi_watchdog driver can call panic() from NMI handler, so replace
it with nmi_panic().

Signed-off-by: Hidehiro Kawai <hidehiro.kawai.ez@hitachi.com>
Cc: Corey Minyard <minyard@acm.org>
Cc: openipmi-developer@lists.sourceforge.net
---
 drivers/char/ipmi/ipmi_watchdog.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/char/ipmi/ipmi_watchdog.c b/drivers/char/ipmi/ipmi_watchdog.c
index 096f0ce..4facc75 100644
--- a/drivers/char/ipmi/ipmi_watchdog.c
+++ b/drivers/char/ipmi/ipmi_watchdog.c
@@ -1140,7 +1140,7 @@ ipmi_nmi(unsigned int val, struct pt_regs *regs)
 		   the timer.   So do so. */
 		pretimeout_since_last_heartbeat = 1;
 		if (atomic_inc_and_test(&preop_panic_excl))
-			panic(PFX "pre-timeout");
+			nmi_panic(regs, PFX "pre-timeout");
 	}
 
 	return NMI_HANDLED;

[toc] | [next] | [standalone]

#1346320

On 02/29/2016 05:50 PM, Hidehiro Kawai wrote:
> 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.
>
> ipmi_watchdog driver can call panic() from NMI handler, so replace
> it with nmi_panic().
>
> Signed-off-by: Hidehiro Kawai <hidehiro.kawai.ez@hitachi.com>
> Cc: Corey Minyard <minyard@acm.org>
> Cc: openipmi-developer@lists.sourceforge.net

Acked-by: Guenter Roeck <linux@roeck-us.net>

> ---
>   drivers/char/ipmi/ipmi_watchdog.c |    2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/char/ipmi/ipmi_watchdog.c b/drivers/char/ipmi/ipmi_watchdog.c
> index 096f0ce..4facc75 100644
> --- a/drivers/char/ipmi/ipmi_watchdog.c
> +++ b/drivers/char/ipmi/ipmi_watchdog.c
> @@ -1140,7 +1140,7 @@ ipmi_nmi(unsigned int val, struct pt_regs *regs)
>   		   the timer.   So do so. */
>   		pretimeout_since_last_heartbeat = 1;
>   		if (atomic_inc_and_test(&preop_panic_excl))
> -			panic(PFX "pre-timeout");
> +			nmi_panic(regs, PFX "pre-timeout");
>   	}
>
>   	return NMI_HANDLED;
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

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

#1346344

Sure, this is a good idea.

Acked-by: Corey Minyard <cminyard@mvista.com>

Note that nmi_panic() came in commit 1717f2096b5 (panic, x86: Fix
re-entrance problem due to panic on NMI) and then the regs field
was added in the commit you reference.

Do you want me to add this to the IPMI queue or do you have another
way to get this patch into the kernel?

-corey

On 02/29/2016 07:50 PM, Hidehiro Kawai wrote:
> 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.
>
> ipmi_watchdog driver can call panic() from NMI handler, so replace
> it with nmi_panic().
>
> Signed-off-by: Hidehiro Kawai <hidehiro.kawai.ez@hitachi.com>
> Cc: Corey Minyard <minyard@acm.org>
> Cc: openipmi-developer@lists.sourceforge.net
> ---
>   drivers/char/ipmi/ipmi_watchdog.c |    2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/char/ipmi/ipmi_watchdog.c b/drivers/char/ipmi/ipmi_watchdog.c
> index 096f0ce..4facc75 100644
> --- a/drivers/char/ipmi/ipmi_watchdog.c
> +++ b/drivers/char/ipmi/ipmi_watchdog.c
> @@ -1140,7 +1140,7 @@ ipmi_nmi(unsigned int val, struct pt_regs *regs)
>   		   the timer.   So do so. */
>   		pretimeout_since_last_heartbeat = 1;
>   		if (atomic_inc_and_test(&preop_panic_excl))
> -			panic(PFX "pre-timeout");
> +			nmi_panic(regs, PFX "pre-timeout");
>   	}
>   
>   	return NMI_HANDLED;
>
>

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

#1346355 — RE: [PATCH 2/3] ipmi/watchdog: Use nmi_panic() when kernel panics in NMI handler

Hi Corey,

Thanks for the review.

> Sure, this is a good idea.
> 
> Acked-by: Corey Minyard <cminyard@mvista.com>
> 
> Note that nmi_panic() came in commit 1717f2096b5 (panic, x86: Fix
> re-entrance problem due to panic on NMI) and then the regs field
> was added in the commit you reference.

Yes. So, I'll change the description to more proper one.

> Do you want me to add this to the IPMI queue or do you have another
> way to get this patch into the kernel?

I don't have another way, and I don't know how cross-subsystem
patch set should be handled.

I think it would be better this patch set is managed by one person
because both PATCH 2/3 and 3/3 depend on 1/3.

Thanks,

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

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

#1346372

On 02/29/2016 08:23 PM, 河合英宏 / KAWAI，HIDEHIRO wrote:
> Hi Corey,
>
> Thanks for the review.
>
>> Sure, this is a good idea.
>>
>> Acked-by: Corey Minyard <cminyard@mvista.com>
>>
>> Note that nmi_panic() came in commit 1717f2096b5 (panic, x86: Fix
>> re-entrance problem due to panic on NMI) and then the regs field
>> was added in the commit you reference.
>
> Yes. So, I'll change the description to more proper one.
>
>> Do you want me to add this to the IPMI queue or do you have another
>> way to get this patch into the kernel?
>
> I don't have another way, and I don't know how cross-subsystem
> patch set should be handled.
>
> I think it would be better this patch set is managed by one person
> because both PATCH 2/3 and 3/3 depend on 1/3.
>

We'll need an Ack from someone with authority over kernel/panic.c.
If someone sends an Ack, the series can go through IPMI (or watchdog).
It might be easier though if Andrew would take the entire series.

Guenter

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

#1346458

On Tue 01-03-16 10:50:39, Hidehiro Kawai wrote:
> 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.
> 
> ipmi_watchdog driver can call panic() from NMI handler, so replace
> it with nmi_panic().
> 
> Signed-off-by: Hidehiro Kawai <hidehiro.kawai.ez@hitachi.com>
> Cc: Corey Minyard <minyard@acm.org>
> Cc: openipmi-developer@lists.sourceforge.net

Reviewed-by: Michal Hocko <mhocko@suse.com>

> ---
>  drivers/char/ipmi/ipmi_watchdog.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/char/ipmi/ipmi_watchdog.c b/drivers/char/ipmi/ipmi_watchdog.c
> index 096f0ce..4facc75 100644
> --- a/drivers/char/ipmi/ipmi_watchdog.c
> +++ b/drivers/char/ipmi/ipmi_watchdog.c
> @@ -1140,7 +1140,7 @@ ipmi_nmi(unsigned int val, struct pt_regs *regs)
>  		   the timer.   So do so. */
>  		pretimeout_since_last_heartbeat = 1;
>  		if (atomic_inc_and_test(&preop_panic_excl))
> -			panic(PFX "pre-timeout");
> +			nmi_panic(regs, PFX "pre-timeout");
>  	}
>  
>  	return NMI_HANDLED;
> 

-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web