[PATCH] kernel/panic: Add "late_kdump" option for kdump in unstable condition

[PATCH] kernel/panic: Add "late_kdump" option for kdump in unstable condition

[Masami Hiramatsu]

Add a "late_kdump" option to run kdump after running panic
notifiers and dump kmsg. This can help rare situations which
kdump drops in failure because of unstable crashed kernel
or hardware failure (memory corruption on critical data/code),
or the 2nd kernel is broken by the 1st kernel (it's a broken
behavior, but who can guarantee that the "crashed" kernel
works correctly?).

Usage: add "late_kdump" to kernel boot option. That's all.

Note that this actually increases risks of the failure of
kdump. This option should be set only if you worry about
the rare case of kdump failure rather than increasing the
chance of success.

Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Eric Biederman <ebiederm@xmission.com>
Cc: Vivek Goyal <vgoyal@redhat.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Yoshihiro YUNOMAE <yoshihiro.yunomae.ez@hitachi.com>
Cc: Satoru MORIYA <satoru.moriya.br@hitachi.com>
Cc: Motohiro Kosaki <Motohiro.Kosaki@us.fujitsu.com>
Cc: Takenori Nagano <t-nagano@ah.jp.nec.com>
---
 Documentation/kernel-parameters.txt |    7 +++++++
 kernel/panic.c                      |   24 ++++++++++++++++++++++--
 2 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 03e50b4..1ba58da 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -2339,6 +2339,13 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 			timeout < 0: reboot immediately
 			Format: <timeout>
 
+	late_kdump	Run kdump after running panic-notifiers and dumping
+			kmsg. This only for the users who doubt kdump always
+			succeeds in any situation.
+			Note that this also increases risks of kdump failure,
+			because some panic notifiers can make the crashed
+			kernel more unstable.
+
 	parkbd.port=	[HW] Parallel port number the keyboard adapter is
 			connected to, default is 0.
 			Format: <parport#>
diff --git a/kernel/panic.c b/kernel/panic.c
index d02fa9f..bba42b5 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -32,6 +32,7 @@ static unsigned long tainted_mask;
 static int pause_on_oops;
 static int pause_on_oops_flag;
 static DEFINE_SPINLOCK(pause_on_oops_lock);
+static bool late_kdump;
 
 int panic_timeout = CONFIG_PANIC_TIMEOUT;
 EXPORT_SYMBOL_GPL(panic_timeout);
@@ -112,9 +113,14 @@ void panic(const char *fmt, ...)
 	/*
 	 * If we have crashed and we have a crash kernel loaded let it handle
 	 * everything else.
-	 * Do we want to call this before we try to display a message?
+	 * If we want to call this after we try to display a message, pass
+	 * the "late_kdump" option to the kernel.
 	 */
-	crash_kexec(NULL);
+	if (!late_kdump)
+		crash_kexec(NULL);
+	else
+		pr_emerg("Warning: late_kdump option is set. Please DO NOT "
+			"report bugs about kdump failure with this option.\n");
 
 	/*
 	 * Note smp_send_stop is the usual smp shutdown function, which
@@ -131,6 +137,13 @@ void panic(const char *fmt, ...)
 
 	kmsg_dump(KMSG_DUMP_PANIC);
 
+	/*
+	 * If you doubt kdump always works perfectly in any situation,
+	 * "late_kdump" offers you to try kdump after running panic_notifier
+	 * and dumping kmsg.
+	 */
+	crash_kexec(NULL);
+
 	bust_spinlocks(0);
 
 	if (!panic_blink)
@@ -472,6 +485,13 @@ EXPORT_SYMBOL(__stack_chk_fail);
 core_param(panic, panic_timeout, int, 0644);
 core_param(pause_on_oops, pause_on_oops, int, 0644);
 
+static int __init setup_late_kdump(char *s)
+{
+	late_kdump = true;
+	return 0;
+}
+early_param("late_kdump", setup_late_kdump);
+
 static int __init oops_setup(char *s)
 {
 	if (!s)

Re: [PATCH] kernel/panic: Add "late_kdump" option for kdump in unstable condition

[Eric W. Biederman]

Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> writes:

> Add a "late_kdump" option to run kdump after running panic
> notifiers and dump kmsg. This can help rare situations which
> kdump drops in failure because of unstable crashed kernel
> or hardware failure (memory corruption on critical data/code),
> or the 2nd kernel is broken by the 1st kernel (it's a broken
> behavior, but who can guarantee that the "crashed" kernel
> works correctly?).
>
> Usage: add "late_kdump" to kernel boot option. That's all.
>
> Note that this actually increases risks of the failure of
> kdump. This option should be set only if you worry about
> the rare case of kdump failure rather than increasing the
> chance of success.

This is better than some others, but every time I have seen a request
to do this it is because someone wants to do something horrible that
makes kdump more brittle and generally unsupportable.

You seem to in general understand that.

But how can we support an option to make the kernel flakier?

I suspect it would be more productive to work on the lkcd (spelling?)
test module and show that crash dump actually works in the situation
people are worried about.

Just thinking about this send shivers up my spine. Ick.

Eric

> Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> Cc: Eric Biederman <ebiederm@xmission.com>
> Cc: Vivek Goyal <vgoyal@redhat.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Yoshihiro YUNOMAE <yoshihiro.yunomae.ez@hitachi.com>
> Cc: Satoru MORIYA <satoru.moriya.br@hitachi.com>
> Cc: Motohiro Kosaki <Motohiro.Kosaki@us.fujitsu.com>
> Cc: Takenori Nagano <t-nagano@ah.jp.nec.com>
> ---
>  Documentation/kernel-parameters.txt |    7 +++++++
>  kernel/panic.c                      |   24 ++++++++++++++++++++++--
>  2 files changed, 29 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> index 03e50b4..1ba58da 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -2339,6 +2339,13 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
>  			timeout < 0: reboot immediately
>  			Format: <timeout>
>  
> +	late_kdump	Run kdump after running panic-notifiers and dumping
> +			kmsg. This only for the users who doubt kdump always
> +			succeeds in any situation.
> +			Note that this also increases risks of kdump failure,
> +			because some panic notifiers can make the crashed
> +			kernel more unstable.
> +
>  	parkbd.port=	[HW] Parallel port number the keyboard adapter is
>  			connected to, default is 0.
>  			Format: <parport#>
> diff --git a/kernel/panic.c b/kernel/panic.c
> index d02fa9f..bba42b5 100644
> --- a/kernel/panic.c
> +++ b/kernel/panic.c
> @@ -32,6 +32,7 @@ static unsigned long tainted_mask;
>  static int pause_on_oops;
>  static int pause_on_oops_flag;
>  static DEFINE_SPINLOCK(pause_on_oops_lock);
> +static bool late_kdump;
>  
>  int panic_timeout = CONFIG_PANIC_TIMEOUT;
>  EXPORT_SYMBOL_GPL(panic_timeout);
> @@ -112,9 +113,14 @@ void panic(const char *fmt, ...)
>  	/*
>  	 * If we have crashed and we have a crash kernel loaded let it handle
>  	 * everything else.
> -	 * Do we want to call this before we try to display a message?
> +	 * If we want to call this after we try to display a message, pass
> +	 * the "late_kdump" option to the kernel.
>  	 */
> -	crash_kexec(NULL);
> +	if (!late_kdump)
> +		crash_kexec(NULL);
> +	else
> +		pr_emerg("Warning: late_kdump option is set. Please DO NOT "
> +			"report bugs about kdump failure with this option.\n");
>  
>  	/*
>  	 * Note smp_send_stop is the usual smp shutdown function, which
> @@ -131,6 +137,13 @@ void panic(const char *fmt, ...)
>  
>  	kmsg_dump(KMSG_DUMP_PANIC);
>  
> +	/*
> +	 * If you doubt kdump always works perfectly in any situation,
> +	 * "late_kdump" offers you to try kdump after running panic_notifier
> +	 * and dumping kmsg.
> +	 */
> +	crash_kexec(NULL);
> +
>  	bust_spinlocks(0);
>  
>  	if (!panic_blink)
> @@ -472,6 +485,13 @@ EXPORT_SYMBOL(__stack_chk_fail);
>  core_param(panic, panic_timeout, int, 0644);
>  core_param(pause_on_oops, pause_on_oops, int, 0644);
>  
> +static int __init setup_late_kdump(char *s)
> +{
> +	late_kdump = true;
> +	return 0;
> +}
> +early_param("late_kdump", setup_late_kdump);
> +
>  static int __init oops_setup(char *s)
>  {
>  	if (!s)

Re: [PATCH] kernel/panic: Add "late_kdump" option for kdump in unstable condition

[Masami Hiramatsu]

(2014/04/14 14:14), Eric W. Biederman wrote:
> Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> writes:
> 
>> Add a "late_kdump" option to run kdump after running panic
>> notifiers and dump kmsg. This can help rare situations which
>> kdump drops in failure because of unstable crashed kernel
>> or hardware failure (memory corruption on critical data/code),
>> or the 2nd kernel is broken by the 1st kernel (it's a broken
>> behavior, but who can guarantee that the "crashed" kernel
>> works correctly?).
>>
>> Usage: add "late_kdump" to kernel boot option. That's all.
>>
>> Note that this actually increases risks of the failure of
>> kdump. This option should be set only if you worry about
>> the rare case of kdump failure rather than increasing the
>> chance of success.
> 
> This is better than some others, but every time I have seen a request
> to do this it is because someone wants to do something horrible that
> makes kdump more brittle and generally unsupportable.

I see, with such horrible handlers, kdump should be unreliable. However,
that is not introduced by this option.

> You seem to in general understand that.
> 
> But how can we support an option to make the kernel flakier?

I think it's user's own risk. We do not need to (individual) support it
but just give them a chance. And distros decide to provide support on such
option or not.

For example, some users may choose enabling this without any kdump guarantee
from distro, or will pay more for them to extra support. OTOH, other users who
don't enable this, will not pay much for extra support :)

> I suspect it would be more productive to work on the lkcd (spelling?)
> test module and show that crash dump actually works in the situation
> people are worried about.

No, I think there are some situation which the kdump/lkcd actually doesn't
work (e.g. software/hardware memory corruptions involving 2nd kernel image).
And in those cases, to leave the minimum information on serial console,
NVM or somewhere safe, this option may give a chance.

Note that those output action can make the situation worse, because some of
such actions are not well tested and may be immature. Thus I added a warning
message on the path of "late_kdump" :)


> Just thinking about this send shivers up my spine. Ick.

I hope everyone correctly understand the risk of this option, and not sending
bug reports to you.

Thank you,

> 
> Eric
> 
>> Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
>> Cc: Eric Biederman <ebiederm@xmission.com>
>> Cc: Vivek Goyal <vgoyal@redhat.com>
>> Cc: Andrew Morton <akpm@linux-foundation.org>
>> Cc: Yoshihiro YUNOMAE <yoshihiro.yunomae.ez@hitachi.com>
>> Cc: Satoru MORIYA <satoru.moriya.br@hitachi.com>
>> Cc: Motohiro Kosaki <Motohiro.Kosaki@us.fujitsu.com>
>> Cc: Takenori Nagano <t-nagano@ah.jp.nec.com>
>> ---
>>  Documentation/kernel-parameters.txt |    7 +++++++
>>  kernel/panic.c                      |   24 ++++++++++++++++++++++--
>>  2 files changed, 29 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
>> index 03e50b4..1ba58da 100644
>> --- a/Documentation/kernel-parameters.txt
>> +++ b/Documentation/kernel-parameters.txt
>> @@ -2339,6 +2339,13 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
>>  			timeout < 0: reboot immediately
>>  			Format: <timeout>
>>  
>> +	late_kdump	Run kdump after running panic-notifiers and dumping
>> +			kmsg. This only for the users who doubt kdump always
>> +			succeeds in any situation.
>> +			Note that this also increases risks of kdump failure,
>> +			because some panic notifiers can make the crashed
>> +			kernel more unstable.
>> +
>>  	parkbd.port=	[HW] Parallel port number the keyboard adapter is
>>  			connected to, default is 0.
>>  			Format: <parport#>
>> diff --git a/kernel/panic.c b/kernel/panic.c
>> index d02fa9f..bba42b5 100644
>> --- a/kernel/panic.c
>> +++ b/kernel/panic.c
>> @@ -32,6 +32,7 @@ static unsigned long tainted_mask;
>>  static int pause_on_oops;
>>  static int pause_on_oops_flag;
>>  static DEFINE_SPINLOCK(pause_on_oops_lock);
>> +static bool late_kdump;
>>  
>>  int panic_timeout = CONFIG_PANIC_TIMEOUT;
>>  EXPORT_SYMBOL_GPL(panic_timeout);
>> @@ -112,9 +113,14 @@ void panic(const char *fmt, ...)
>>  	/*
>>  	 * If we have crashed and we have a crash kernel loaded let it handle
>>  	 * everything else.
>> -	 * Do we want to call this before we try to display a message?
>> +	 * If we want to call this after we try to display a message, pass
>> +	 * the "late_kdump" option to the kernel.
>>  	 */
>> -	crash_kexec(NULL);
>> +	if (!late_kdump)
>> +		crash_kexec(NULL);
>> +	else
>> +		pr_emerg("Warning: late_kdump option is set. Please DO NOT "
>> +			"report bugs about kdump failure with this option.\n");
>>  
>>  	/*
>>  	 * Note smp_send_stop is the usual smp shutdown function, which
>> @@ -131,6 +137,13 @@ void panic(const char *fmt, ...)
>>  
>>  	kmsg_dump(KMSG_DUMP_PANIC);
>>  
>> +	/*
>> +	 * If you doubt kdump always works perfectly in any situation,
>> +	 * "late_kdump" offers you to try kdump after running panic_notifier
>> +	 * and dumping kmsg.
>> +	 */
>> +	crash_kexec(NULL);
>> +
>>  	bust_spinlocks(0);
>>  
>>  	if (!panic_blink)
>> @@ -472,6 +485,13 @@ EXPORT_SYMBOL(__stack_chk_fail);
>>  core_param(panic, panic_timeout, int, 0644);
>>  core_param(pause_on_oops, pause_on_oops, int, 0644);
>>  
>> +static int __init setup_late_kdump(char *s)
>> +{
>> +	late_kdump = true;
>> +	return 0;
>> +}
>> +early_param("late_kdump", setup_late_kdump);
>> +
>>  static int __init oops_setup(char *s)
>>  {
>>  	if (!s)
> 


-- 
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com

Re: [PATCH] kernel/panic: Add "late_kdump" option for kdump in unstable condition

[Vivek Goyal]

On Sun, Apr 13, 2014 at 10:14:18PM -0700, Eric W. Biederman wrote:
> Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> writes:
> 
> > Add a "late_kdump" option to run kdump after running panic
> > notifiers and dump kmsg. This can help rare situations which
> > kdump drops in failure because of unstable crashed kernel
> > or hardware failure (memory corruption on critical data/code),
> > or the 2nd kernel is broken by the 1st kernel (it's a broken
> > behavior, but who can guarantee that the "crashed" kernel
> > works correctly?).
> >
> > Usage: add "late_kdump" to kernel boot option. That's all.
> >
> > Note that this actually increases risks of the failure of
> > kdump. This option should be set only if you worry about
> > the rare case of kdump failure rather than increasing the
> > chance of success.
> 
> This is better than some others, but every time I have seen a request
> to do this it is because someone wants to do something horrible that
> makes kdump more brittle and generally unsupportable.
> 
> You seem to in general understand that.
> 
> But how can we support an option to make the kernel flakier?
> 
> I suspect it would be more productive to work on the lkcd (spelling?)
> test module and show that crash dump actually works in the situation
> people are worried about.
> 
> Just thinking about this send shivers up my spine. Ick.

Eric,

This question has been raised many a times. Our argument has been that
it reduces kdump reliability. And their argument is that so be it. They
are ready to bear that cost and before we really transition into kdump
kernel they want to do something else. One such use case is trying to
save some information into NVRAM.

Their argument is that saving to NVRAM is more reliable. There are no
guarantees that kdump kernel will actually come and save full dump. So
they think that they can save kernel log buffers to NVRAM atleast before
jumping to kdump kernel.

I understand that this is antithesis to executing some code in crashed
kernel and it reduces the reliability of kdump operation. But I also think
we should atleast provide people with a choice. And it is up to them whether
how do they want to configure the system.

If somebody is willing to live with reduced reliability of kdump
operation, why should we enforce that no such option will be provided.

Thanks
Vivek

Re: [PATCH] kernel/panic: Add "late_kdump" option for kdump in unstable condition

[Vivek Goyal]

On Mon, Apr 14, 2014 at 01:51:58PM +0900, Masami Hiramatsu wrote:
> Add a "late_kdump" option to run kdump after running panic
> notifiers and dump kmsg. This can help rare situations which
> kdump drops in failure because of unstable crashed kernel
> or hardware failure (memory corruption on critical data/code),
> or the 2nd kernel is broken by the 1st kernel (it's a broken
> behavior, but who can guarantee that the "crashed" kernel
> works correctly?).
> 
> Usage: add "late_kdump" to kernel boot option. That's all.
> 
> Note that this actually increases risks of the failure of
> kdump. This option should be set only if you worry about
> the rare case of kdump failure rather than increasing the
> chance of success.
> 

Masami,

So what's the alternative to kdump which is more reliable? IOW, what
action you are planning to take through kmsg_dump() or through
panic_notifiers?

I have seen that many a times developers have tried to make the case 
to save kernel buffers to NVRAM. Does it work well? Has it been proven
to be more reliable than kdump?

Thanks
Vivek

> Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> Cc: Eric Biederman <ebiederm@xmission.com>
> Cc: Vivek Goyal <vgoyal@redhat.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Yoshihiro YUNOMAE <yoshihiro.yunomae.ez@hitachi.com>
> Cc: Satoru MORIYA <satoru.moriya.br@hitachi.com>
> Cc: Motohiro Kosaki <Motohiro.Kosaki@us.fujitsu.com>
> Cc: Takenori Nagano <t-nagano@ah.jp.nec.com>
> ---
>  Documentation/kernel-parameters.txt |    7 +++++++
>  kernel/panic.c                      |   24 ++++++++++++++++++++++--
>  2 files changed, 29 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> index 03e50b4..1ba58da 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -2339,6 +2339,13 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
>  			timeout < 0: reboot immediately
>  			Format: <timeout>
>  
> +	late_kdump	Run kdump after running panic-notifiers and dumping
> +			kmsg. This only for the users who doubt kdump always
> +			succeeds in any situation.
> +			Note that this also increases risks of kdump failure,
> +			because some panic notifiers can make the crashed
> +			kernel more unstable.
> +
>  	parkbd.port=	[HW] Parallel port number the keyboard adapter is
>  			connected to, default is 0.
>  			Format: <parport#>
> diff --git a/kernel/panic.c b/kernel/panic.c
> index d02fa9f..bba42b5 100644
> --- a/kernel/panic.c
> +++ b/kernel/panic.c
> @@ -32,6 +32,7 @@ static unsigned long tainted_mask;
>  static int pause_on_oops;
>  static int pause_on_oops_flag;
>  static DEFINE_SPINLOCK(pause_on_oops_lock);
> +static bool late_kdump;
>  
>  int panic_timeout = CONFIG_PANIC_TIMEOUT;
>  EXPORT_SYMBOL_GPL(panic_timeout);
> @@ -112,9 +113,14 @@ void panic(const char *fmt, ...)
>  	/*
>  	 * If we have crashed and we have a crash kernel loaded let it handle
>  	 * everything else.
> -	 * Do we want to call this before we try to display a message?
> +	 * If we want to call this after we try to display a message, pass
> +	 * the "late_kdump" option to the kernel.
>  	 */
> -	crash_kexec(NULL);
> +	if (!late_kdump)
> +		crash_kexec(NULL);
> +	else
> +		pr_emerg("Warning: late_kdump option is set. Please DO NOT "
> +			"report bugs about kdump failure with this option.\n");
>  
>  	/*
>  	 * Note smp_send_stop is the usual smp shutdown function, which
> @@ -131,6 +137,13 @@ void panic(const char *fmt, ...)
>  
>  	kmsg_dump(KMSG_DUMP_PANIC);
>  
> +	/*
> +	 * If you doubt kdump always works perfectly in any situation,
> +	 * "late_kdump" offers you to try kdump after running panic_notifier
> +	 * and dumping kmsg.
> +	 */
> +	crash_kexec(NULL);
> +
>  	bust_spinlocks(0);
>  
>  	if (!panic_blink)
> @@ -472,6 +485,13 @@ EXPORT_SYMBOL(__stack_chk_fail);
>  core_param(panic, panic_timeout, int, 0644);
>  core_param(pause_on_oops, pause_on_oops, int, 0644);
>  
> +static int __init setup_late_kdump(char *s)
> +{
> +	late_kdump = true;
> +	return 0;
> +}
> +early_param("late_kdump", setup_late_kdump);
> +
>  static int __init oops_setup(char *s)
>  {
>  	if (!s)
> 

Re: [PATCH] kernel/panic: Add "late_kdump" option for kdump in unstable condition

[Masami Hiramatsu]

Hi Vivek,

(2014/04/15 4:31), Vivek Goyal wrote:
> On Mon, Apr 14, 2014 at 01:51:58PM +0900, Masami Hiramatsu wrote:
>> Add a "late_kdump" option to run kdump after running panic
>> notifiers and dump kmsg. This can help rare situations which
>> kdump drops in failure because of unstable crashed kernel
>> or hardware failure (memory corruption on critical data/code),
>> or the 2nd kernel is broken by the 1st kernel (it's a broken
>> behavior, but who can guarantee that the "crashed" kernel
>> works correctly?).
>>
>> Usage: add "late_kdump" to kernel boot option. That's all.
>>
>> Note that this actually increases risks of the failure of
>> kdump. This option should be set only if you worry about
>> the rare case of kdump failure rather than increasing the
>> chance of success.
>>
> 
> Masami,
> 
> So what's the alternative to kdump which is more reliable? IOW, what
> action you are planning to take through kmsg_dump() or through
> panic_notifiers?
> 
> I have seen that many a times developers have tried to make the case 
> to save kernel buffers to NVRAM. Does it work well? Has it been proven
> to be more reliable than kdump?

Yeah, one possible option is the NVRAM, but even with the serial,
there are other reasons to kick the notifiers, e.g.
 - dump to ipmi which has a very small amount of non-volatile memory
 - ftrace_dump() to dump "flight recorder" log to serial
 - pvpanic notifies panic to the host.

Anyway, I think the most important reason for linux developers is
that we have a chance to improve such horrible notifiers to safer,
or at least to clarify what notifier or behavior makes kdump unstable. :-)

Thank you,

-- 
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com

Re: [PATCH] kernel/panic: Add "late_kdump" option for kdump in unstable condition

[Vivek Goyal]

On Tue, Apr 15, 2014 at 10:37:40AM +0900, Masami Hiramatsu wrote:

[..]
> > Masami,
> > 
> > So what's the alternative to kdump which is more reliable? IOW, what
> > action you are planning to take through kmsg_dump() or through
> > panic_notifiers?
> > 
> > I have seen that many a times developers have tried to make the case 
> > to save kernel buffers to NVRAM. Does it work well? Has it been proven
> > to be more reliable than kdump?
> 
> Yeah, one possible option is the NVRAM, but even with the serial,
> there are other reasons to kick the notifiers, e.g.
>  - dump to ipmi which has a very small amount of non-volatile memory
>  - ftrace_dump() to dump "flight recorder" log to serial

So why do we need to run them in crashed kernel? Only argument I seem
to receive that there is no guarantee that kdump kernel will successfully
boot hence we want to run these notifiers.

But what's the guarantee that these will run successfully without creating
futher issues? Is there data to prove it.

>  - pvpanic notifies panic to the host.

I think this pvpanic() notification can go in kdump kernel too? Anyway,
if one has configured kdump, then host does not have to do anything to
save dump. Host might want to know for informational purposes that panic
happend and system rebooted. So there should not be any need to send
this notification immediately after crash?

> 
> Anyway, I think the most important reason for linux developers is
> that we have a chance to improve such horrible notifiers to safer,

I think big debate here is that we should be able to do most of it
in second kernel. 

If you provide a knob to run these in first kernel, this functionality
will never migrate to second kernel. And trying to make them safe in
crashed kernel is a losing battle, I think.

So providing this knob does not help with making these notifiers better.
These notifiers can become better only if migrate the functionality
to second kernel (preferrably in user space). There we can extract all
the data from /proc/vmcore and send it whereever you want.

But for that you will have to trust kdump and keep on improving it
constantly so that it works reasonably well.

> or at least to clarify what notifier or behavior makes kdump unstable. :-)
> 

I think that's well known. We don't have to provide a knob to prove that
running notifiers will make kdump less reliable.

Thanks
Vivek

Re: [PATCH] kernel/panic: Add "late_kdump" option for kdump in unstable condition

[Masami Hiramatsu]

(2014/04/15 23:08), Vivek Goyal wrote:
> On Tue, Apr 15, 2014 at 10:37:40AM +0900, Masami Hiramatsu wrote:
> 
> [..]
>>> Masami,
>>>
>>> So what's the alternative to kdump which is more reliable? IOW, what
>>> action you are planning to take through kmsg_dump() or through
>>> panic_notifiers?
>>>
>>> I have seen that many a times developers have tried to make the case 
>>> to save kernel buffers to NVRAM. Does it work well? Has it been proven
>>> to be more reliable than kdump?
>>
>> Yeah, one possible option is the NVRAM, but even with the serial,
>> there are other reasons to kick the notifiers, e.g.
>>  - dump to ipmi which has a very small amount of non-volatile memory
>>  - ftrace_dump() to dump "flight recorder" log to serial
> 
> So why do we need to run them in crashed kernel? Only argument I seem
> to receive that there is no guarantee that kdump kernel will successfully
> boot hence we want to run these notifiers.
> 
> But what's the guarantee that these will run successfully without creating
> futher issues? Is there data to prove it.

I think there is no guarantee, but that's same as kdump is.
However, if we can try both, there is higher possibility (more cases)
to save some information.

>>  - pvpanic notifies panic to the host.
> 
> I think this pvpanic() notification can go in kdump kernel too? Anyway,
> if one has configured kdump, then host does not have to do anything to
> save dump. Host might want to know for informational purposes that panic
> happend and system rebooted. So there should not be any need to send
> this notification immediately after crash?

Note that pvpanic works before trying to kdump with late_kdump.
Even if the kdump doesn't work, pvpanic can just notify the panic.
# of course, in that case we might better dump guest from host,
# but that's user's choice.

>> Anyway, I think the most important reason for linux developers is
>> that we have a chance to improve such horrible notifiers to safer,
> 
> I think big debate here is that we should be able to do most of it
> in second kernel. 

No, that's another topic what we talk about.

What I (and others who had argued) consider that in some rare cases,
kdump might fail to boot up the second kernel, and only for who worries
in those cases, we can give a chance.

> If you provide a knob to run these in first kernel, this functionality
> will never migrate to second kernel.

No, there are many use-cases which doesn't (and can't) use kdump
because of the limitation of resources etc. For those cases, that
functionality never migrate (means move) to the second one.

> And trying to make them safe in
> crashed kernel is a losing battle, I think.

Why? the best goal what users expect is both panic-notifiers and kdump
runs safely. If one of them fails, that's a bug (except for some rare
hardware-related corruption.)

> So providing this knob does not help with making these notifiers better.
> These notifiers can become better only if migrate the functionality
> to second kernel (preferrably in user space). There we can extract all
> the data from /proc/vmcore and send it whereever you want.

I see, that is also an important work, but that is done in userspace.
In kernel space, we can do something to give them a chance.

> But for that you will have to trust kdump and keep on improving it
> constantly so that it works reasonably well.

I trust you and kdump :) but I also know that in some rare cases that
kdump can't finish booting up, at least currently. So, if you sure
kdump is improved to boot up the second in any situation, I'm happy
to withdraw from this patch.

>> or at least to clarify what notifier or behavior makes kdump unstable. :-)
>>
> 
> I think that's well known. We don't have to provide a knob to prove that
> running notifiers will make kdump less reliable.

Hmm, could you state that?

Thank you,


-- 
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com

Re: [PATCH] kernel/panic: Add "late_kdump" option for kdump in unstable condition

[Vivek Goyal]

On Wed, Apr 16, 2014 at 10:28:39AM +0900, Masami Hiramatsu wrote:
> (2014/04/15 23:08), Vivek Goyal wrote:
> > On Tue, Apr 15, 2014 at 10:37:40AM +0900, Masami Hiramatsu wrote:
> > 
> > [..]
> >>> Masami,
> >>>
> >>> So what's the alternative to kdump which is more reliable? IOW, what
> >>> action you are planning to take through kmsg_dump() or through
> >>> panic_notifiers?
> >>>
> >>> I have seen that many a times developers have tried to make the case 
> >>> to save kernel buffers to NVRAM. Does it work well? Has it been proven
> >>> to be more reliable than kdump?
> >>
> >> Yeah, one possible option is the NVRAM, but even with the serial,
> >> there are other reasons to kick the notifiers, e.g.
> >>  - dump to ipmi which has a very small amount of non-volatile memory
> >>  - ftrace_dump() to dump "flight recorder" log to serial
> > 
> > So why do we need to run them in crashed kernel? Only argument I seem
> > to receive that there is no guarantee that kdump kernel will successfully
> > boot hence we want to run these notifiers.
> > 
> > But what's the guarantee that these will run successfully without creating
> > futher issues? Is there data to prove it.
> 
> I think there is no guarantee, but that's same as kdump is.
> However, if we can try both, there is higher possibility (more cases)
> to save some information.

This is only valid if the entity which is running before kdump has 
higher probability of saving some useful information. So do kmsg_dump()
and backend drivers provide more reliable way to save kernel logs as
compared to kdump? 

[..]
> > I think big debate here is that we should be able to do most of it
> > in second kernel. 
> 
> No, that's another topic what we talk about.
> 
> What I (and others who had argued) consider that in some rare cases,
> kdump might fail to boot up the second kernel, and only for who worries
> in those cases, we can give a chance.

And *rare failure cases* don't exist in other mechanisms which are
planning to take control before kdump? You are assuming that any entity
which runs before kdump is more reliable than kdump. And I don't think
anybody has any data to prove that. People are just looking for a hook
to execute things before kdump hoping that it will provide them better
results.

> 
> > If you provide a knob to run these in first kernel, this functionality
> > will never migrate to second kernel.
> 
> No, there are many use-cases which doesn't (and can't) use kdump
> because of the limitation of resources etc. For those cases, that
> functionality never migrate (means move) to the second one.

What are those use cases? What resources you are referring too. Are you
planning to do a whole lot after kernel has crashed. That will not make
much sense.

> 
> > And trying to make them safe in
> > crashed kernel is a losing battle, I think.
> 
> Why? the best goal what users expect is both panic-notifiers and kdump
> runs safely. If one of them fails, that's a bug (except for some rare
> hardware-related corruption.)

So you think that running panic-notifiers can be made safe? How would
we do that?

What's the special action panicn notifiers are taking which can't be
done in second kernel. 

> 
> > So providing this knob does not help with making these notifiers better.
> > These notifiers can become better only if migrate the functionality
> > to second kernel (preferrably in user space). There we can extract all
> > the data from /proc/vmcore and send it whereever you want.
> 
> I see, that is also an important work, but that is done in userspace.
> In kernel space, we can do something to give them a chance.

If the goal is to send kernel buffers at some location, then it does
not matter whether a kernel driver does it or a user space application
extracts buffers and then takes help of driver to send it.

> 
> > But for that you will have to trust kdump and keep on improving it
> > constantly so that it works reasonably well.
> 
> I trust you and kdump :) but I also know that in some rare cases that
> kdump can't finish booting up, at least currently. So, if you sure
> kdump is improved to boot up the second in any situation, I'm happy
> to withdraw from this patch.

Kdump is best effort solution. I don't think anybody can guarantee that
it will work in all situations.

And same will be true for different notifiers you are trying to run
before kdump. By running those notifiers you can't be sure that they
will always work and there are no corner cases.

So to me, late_kdump will make sense only if you have an alternate
mechanism which can more reliably save kernel buffers as compared to
kdump. My feeling is that nobody knows how reliable these kmsg_dump(),
NVRAM saving hooks are. Proponets of these hooks seem to be believe
that it will provide them a safety net in case kdump fails.

Given the fact that people have been asking for this for years, I 
think creating a command line parameter to switch to that behavior
is probably not a bad idea. Distributions can probably continue to
run without specifying "late_kdump" and those specific users who wish
to run kmsg_dump() hooks before kdump, can configure their system with
"late_kdump" parameter.

Thanks
Vivek

Re: Re: [PATCH] kernel/panic: Add "late_kdump" option for kdump in unstable condition

[Masami Hiramatsu]

(2014/04/16 11:33), Vivek Goyal wrote:
> On Wed, Apr 16, 2014 at 10:28:39AM +0900, Masami Hiramatsu wrote:
>> (2014/04/15 23:08), Vivek Goyal wrote:
>>> On Tue, Apr 15, 2014 at 10:37:40AM +0900, Masami Hiramatsu wrote:
>>>
>>> [..]
>>>>> Masami,
>>>>>
>>>>> So what's the alternative to kdump which is more reliable? IOW, what
>>>>> action you are planning to take through kmsg_dump() or through
>>>>> panic_notifiers?
>>>>>
>>>>> I have seen that many a times developers have tried to make the case 
>>>>> to save kernel buffers to NVRAM. Does it work well? Has it been proven
>>>>> to be more reliable than kdump?
>>>>
>>>> Yeah, one possible option is the NVRAM, but even with the serial,
>>>> there are other reasons to kick the notifiers, e.g.
>>>>  - dump to ipmi which has a very small amount of non-volatile memory
>>>>  - ftrace_dump() to dump "flight recorder" log to serial
>>>
>>> So why do we need to run them in crashed kernel? Only argument I seem
>>> to receive that there is no guarantee that kdump kernel will successfully
>>> boot hence we want to run these notifiers.
>>>
>>> But what's the guarantee that these will run successfully without creating
>>> futher issues? Is there data to prove it.
>>
>> I think there is no guarantee, but that's same as kdump is.
>> However, if we can try both, there is higher possibility (more cases)
>> to save some information.
> 
> This is only valid if the entity which is running before kdump has 
> higher probability of saving some useful information. So do kmsg_dump()
> and backend drivers provide more reliable way to save kernel logs as
> compared to kdump? 

IMHO, reliability discussion is meaningless at this point, because
it depends on the hardware/software configuration. For example,
someone has setup a server with a serial logger, serial output
is much more reliable. But it does NOT mean kdump is useless.
For precise information, we actually need kdump but it will be less
reliable than serial logger. On the other hand, if we have no
such external equipments, we'd better try kdump first.
(Actually, the problem of kdump is here, if we try kdump, it never
 return when it fails in the second kernel boot. Thus we can't fail
 back to the other handlers.)

> [..]
>>> I think big debate here is that we should be able to do most of it
>>> in second kernel. 
>>
>> No, that's another topic what we talk about.
>>
>> What I (and others who had argued) consider that in some rare cases,
>> kdump might fail to boot up the second kernel, and only for who worries
>> in those cases, we can give a chance.
> 
> And *rare failure cases* don't exist in other mechanisms which are
> planning to take control before kdump?

No, definitely not. Thus users must bet the safer way. :)
As I said, this option doesn't guarantee improving the safeness
of getting crash information, but just gives another option which
users want.

> You are assuming that any entity
> which runs before kdump is more reliable than kdump. And I don't think
> anybody has any data to prove that. People are just looking for a hook
> to execute things before kdump hoping that it will provide them better
> results.

Yeah, they hope, hope to have a chance to bet the better handler which
they believe. As you said, there are any data to prove that kdump is
safer than others too. Why don't we give a hand?

>>> If you provide a knob to run these in first kernel, this functionality
>>> will never migrate to second kernel.
>>
>> No, there are many use-cases which doesn't (and can't) use kdump
>> because of the limitation of resources etc. For those cases, that
>> functionality never migrate (means move) to the second one.
> 
> What are those use cases? What resources you are referring too. Are you
> planning to do a whole lot after kernel has crashed. That will not make
> much sense.

There are not only the enterprise server users, but also embedded
device users. Sometimes such small devices are distributed on the
remote site or somewhere outside. And they have limited resources.
Kdump is too big to them, but other panic handlers will be useful.

>>> And trying to make them safe in
>>> crashed kernel is a losing battle, I think.
>>
>> Why? the best goal what users expect is both panic-notifiers and kdump
>> runs safely. If one of them fails, that's a bug (except for some rare
>> hardware-related corruption.)
> 
> So you think that running panic-notifiers can be made safe? How would
> we do that?

Step 1. Add the late_kdump option.
Step 2. "Brave" users start using the option with panic-notifiers.
Step 3. They hits problems on that and reports bugs.
Step 4. We fix them.

> What's the special action panicn notifiers are taking which can't be
> done in second kernel. 

No, ant that is out of focus. What I meant is NOT what action
we can take IN the second kernel, but what action we can take
BEFORE the second kernel.

>>> So providing this knob does not help with making these notifiers better.
>>> These notifiers can become better only if migrate the functionality
>>> to second kernel (preferrably in user space). There we can extract all
>>> the data from /proc/vmcore and send it whereever you want.
>>
>> I see, that is also an important work, but that is done in userspace.
>> In kernel space, we can do something to give them a chance.
> 
> If the goal is to send kernel buffers at some location, then it does
> not matter whether a kernel driver does it or a user space application
> extracts buffers and then takes help of driver to send it.

Yeah, I see. What the first kernel can do, is also able to do in the
second kernel too. I strongly agreed. However, what I'd like to do
is to give a chance to execute panic handler before kdump.

>>> But for that you will have to trust kdump and keep on improving it
>>> constantly so that it works reasonably well.
>>
>> I trust you and kdump :) but I also know that in some rare cases that
>> kdump can't finish booting up, at least currently. So, if you sure
>> kdump is improved to boot up the second in any situation, I'm happy
>> to withdraw from this patch.
> 
> Kdump is best effort solution. I don't think anybody can guarantee that
> it will work in all situations.
> 
> And same will be true for different notifiers you are trying to run
> before kdump. By running those notifiers you can't be sure that they
> will always work and there are no corner cases.

Sure, but would you sure there is NO case that the panic-notifier can
run but kdump cannot ?

> So to me, late_kdump will make sense only if you have an alternate
> mechanism which can more reliably save kernel buffers as compared to
> kdump. My feeling is that nobody knows how reliable these kmsg_dump(),
> NVRAM saving hooks are. Proponets of these hooks seem to be believe
> that it will provide them a safety net in case kdump fails.

My point is not rely on the mechanism, but theoretically there are
some case that panic-notifier doesn't make worse, but only kdump
fails. E.g. memory corruption on the second kernel image.
This is the event that occur probabilistically, not the mechanism.

> Given the fact that people have been asking for this for years, I 
> think creating a command line parameter to switch to that behavior
> is probably not a bad idea. Distributions can probably continue to
> run without specifying "late_kdump" and those specific users who wish
> to run kmsg_dump() hooks before kdump, can configure their system with
> "late_kdump" parameter.

Thank you :)

> 
> Thanks
> Vivek

-- 
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com

Re: [PATCH] kernel/panic: Add "late_kdump" option for kdump in unstable condition

[Vivek Goyal]

On Mon, Apr 14, 2014 at 01:51:58PM +0900, Masami Hiramatsu wrote:
> Add a "late_kdump" option to run kdump after running panic
> notifiers and dump kmsg. This can help rare situations which
> kdump drops in failure because of unstable crashed kernel
> or hardware failure (memory corruption on critical data/code),
> or the 2nd kernel is broken by the 1st kernel (it's a broken
> behavior, but who can guarantee that the "crashed" kernel
> works correctly?).
> 
> Usage: add "late_kdump" to kernel boot option. That's all.
> 
> Note that this actually increases risks of the failure of
> kdump. This option should be set only if you worry about
> the rare case of kdump failure rather than increasing the
> chance of success.
> 
> Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> Cc: Eric Biederman <ebiederm@xmission.com>
> Cc: Vivek Goyal <vgoyal@redhat.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Yoshihiro YUNOMAE <yoshihiro.yunomae.ez@hitachi.com>
> Cc: Satoru MORIYA <satoru.moriya.br@hitachi.com>
> Cc: Motohiro Kosaki <Motohiro.Kosaki@us.fujitsu.com>
> Cc: Takenori Nagano <t-nagano@ah.jp.nec.com>
> ---
>  Documentation/kernel-parameters.txt |    7 +++++++
>  kernel/panic.c                      |   24 ++++++++++++++++++++++--
>  2 files changed, 29 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> index 03e50b4..1ba58da 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -2339,6 +2339,13 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
>  			timeout < 0: reboot immediately
>  			Format: <timeout>
>  
> +	late_kdump	Run kdump after running panic-notifiers and dumping
> +			kmsg. This only for the users who doubt kdump always
> +			succeeds in any situation.
> +			Note that this also increases risks of kdump failure,
> +			because some panic notifiers can make the crashed
> +			kernel more unstable.
> +

I am wondering if "crash_kexec_post_notifiers" will be a better name
to represent what we are trying to do here.

>  	parkbd.port=	[HW] Parallel port number the keyboard adapter is
>  			connected to, default is 0.
>  			Format: <parport#>
> diff --git a/kernel/panic.c b/kernel/panic.c
> index d02fa9f..bba42b5 100644
> --- a/kernel/panic.c
> +++ b/kernel/panic.c
> @@ -32,6 +32,7 @@ static unsigned long tainted_mask;
>  static int pause_on_oops;
>  static int pause_on_oops_flag;
>  static DEFINE_SPINLOCK(pause_on_oops_lock);
> +static bool late_kdump;
>  
>  int panic_timeout = CONFIG_PANIC_TIMEOUT;
>  EXPORT_SYMBOL_GPL(panic_timeout);
> @@ -112,9 +113,14 @@ void panic(const char *fmt, ...)
>  	/*
>  	 * If we have crashed and we have a crash kernel loaded let it handle
>  	 * everything else.
> -	 * Do we want to call this before we try to display a message?
> +	 * If we want to call this after we try to display a message, pass
> +	 * the "late_kdump" option to the kernel.
>  	 */
> -	crash_kexec(NULL);
> +	if (!late_kdump)
> +		crash_kexec(NULL);
> +	else
> +		pr_emerg("Warning: late_kdump option is set. Please DO NOT "
> +			"report bugs about kdump failure with this option.\n");

I think above message about DO NOT report bugs seems unnecessary. 

Thanks
Vivek

Re: [PATCH] kernel/panic: Add "late_kdump" option for kdump in unstable condition

[Masami Hiramatsu]

Thank you for review!

(2014/04/16 22:48), Vivek Goyal wrote:
> On Mon, Apr 14, 2014 at 01:51:58PM +0900, Masami Hiramatsu wrote:
>> Add a "late_kdump" option to run kdump after running panic
>> notifiers and dump kmsg. This can help rare situations which
>> kdump drops in failure because of unstable crashed kernel
>> or hardware failure (memory corruption on critical data/code),
>> or the 2nd kernel is broken by the 1st kernel (it's a broken
>> behavior, but who can guarantee that the "crashed" kernel
>> works correctly?).
>>
>> Usage: add "late_kdump" to kernel boot option. That's all.
>>
>> Note that this actually increases risks of the failure of
>> kdump. This option should be set only if you worry about
>> the rare case of kdump failure rather than increasing the
>> chance of success.
>>
>> Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
>> Cc: Eric Biederman <ebiederm@xmission.com>
>> Cc: Vivek Goyal <vgoyal@redhat.com>
>> Cc: Andrew Morton <akpm@linux-foundation.org>
>> Cc: Yoshihiro YUNOMAE <yoshihiro.yunomae.ez@hitachi.com>
>> Cc: Satoru MORIYA <satoru.moriya.br@hitachi.com>
>> Cc: Motohiro Kosaki <Motohiro.Kosaki@us.fujitsu.com>
>> Cc: Takenori Nagano <t-nagano@ah.jp.nec.com>
>> ---
>>  Documentation/kernel-parameters.txt |    7 +++++++
>>  kernel/panic.c                      |   24 ++++++++++++++++++++++--
>>  2 files changed, 29 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
>> index 03e50b4..1ba58da 100644
>> --- a/Documentation/kernel-parameters.txt
>> +++ b/Documentation/kernel-parameters.txt
>> @@ -2339,6 +2339,13 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
>>  			timeout < 0: reboot immediately
>>  			Format: <timeout>
>>  
>> +	late_kdump	Run kdump after running panic-notifiers and dumping
>> +			kmsg. This only for the users who doubt kdump always
>> +			succeeds in any situation.
>> +			Note that this also increases risks of kdump failure,
>> +			because some panic notifiers can make the crashed
>> +			kernel more unstable.
>> +
> 
> I am wondering if "crash_kexec_post_notifiers" will be a better name
> to represent what we are trying to do here.

OK, I'll rename that.

> 
>>  	parkbd.port=	[HW] Parallel port number the keyboard adapter is
>>  			connected to, default is 0.
>>  			Format: <parport#>
>> diff --git a/kernel/panic.c b/kernel/panic.c
>> index d02fa9f..bba42b5 100644
>> --- a/kernel/panic.c
>> +++ b/kernel/panic.c
>> @@ -32,6 +32,7 @@ static unsigned long tainted_mask;
>>  static int pause_on_oops;
>>  static int pause_on_oops_flag;
>>  static DEFINE_SPINLOCK(pause_on_oops_lock);
>> +static bool late_kdump;
>>  
>>  int panic_timeout = CONFIG_PANIC_TIMEOUT;
>>  EXPORT_SYMBOL_GPL(panic_timeout);
>> @@ -112,9 +113,14 @@ void panic(const char *fmt, ...)
>>  	/*
>>  	 * If we have crashed and we have a crash kernel loaded let it handle
>>  	 * everything else.
>> -	 * Do we want to call this before we try to display a message?
>> +	 * If we want to call this after we try to display a message, pass
>> +	 * the "late_kdump" option to the kernel.
>>  	 */
>> -	crash_kexec(NULL);
>> +	if (!late_kdump)
>> +		crash_kexec(NULL);
>> +	else
>> +		pr_emerg("Warning: late_kdump option is set. Please DO NOT "
>> +			"report bugs about kdump failure with this option.\n");
> 
> I think above message about DO NOT report bugs seems unnecessary. 

OK, so I just notify the option is set as below.
"Warning: crash_kexec_post_notifiers is set.\n"

Thank you again!

-- 
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com

Re: [PATCH] kernel/panic: Add "late_kdump" option for kdump in unstable condition

[Vivek Goyal]

On Thu, Apr 17, 2014 at 10:59:14AM +0900, Masami Hiramatsu wrote:

[..]
> >> @@ -112,9 +113,14 @@ void panic(const char *fmt, ...)
> >>  	/*
> >>  	 * If we have crashed and we have a crash kernel loaded let it handle
> >>  	 * everything else.
> >> -	 * Do we want to call this before we try to display a message?
> >> +	 * If we want to call this after we try to display a message, pass
> >> +	 * the "late_kdump" option to the kernel.
> >>  	 */
> >> -	crash_kexec(NULL);
> >> +	if (!late_kdump)
> >> +		crash_kexec(NULL);
> >> +	else
> >> +		pr_emerg("Warning: late_kdump option is set. Please DO NOT "
> >> +			"report bugs about kdump failure with this option.\n");
> > 
> > I think above message about DO NOT report bugs seems unnecessary. 
> 
> OK, so I just notify the option is set as below.
> "Warning: crash_kexec_post_notifiers is set.\n"

I would say for the time being don't put any extra message. User will
think what this warning is supposed to mean. We can introduce one if
we run into many issues.

For the time being we can just look at kernel logs and look at command
line options and see whether crash_kexec_post_notifiers was set or not.

Thanks
Vivek

Re: [PATCH] kernel/panic: Add "late_kdump" option for kdump in unstable condition

[Masami Hiramatsu]

(2014/04/17 22:43), Vivek Goyal wrote:
> On Thu, Apr 17, 2014 at 10:59:14AM +0900, Masami Hiramatsu wrote:
> 
> [..]
>>>> @@ -112,9 +113,14 @@ void panic(const char *fmt, ...)
>>>>  	/*
>>>>  	 * If we have crashed and we have a crash kernel loaded let it handle
>>>>  	 * everything else.
>>>> -	 * Do we want to call this before we try to display a message?
>>>> +	 * If we want to call this after we try to display a message, pass
>>>> +	 * the "late_kdump" option to the kernel.
>>>>  	 */
>>>> -	crash_kexec(NULL);
>>>> +	if (!late_kdump)
>>>> +		crash_kexec(NULL);
>>>> +	else
>>>> +		pr_emerg("Warning: late_kdump option is set. Please DO NOT "
>>>> +			"report bugs about kdump failure with this option.\n");
>>>
>>> I think above message about DO NOT report bugs seems unnecessary. 
>>
>> OK, so I just notify the option is set as below.
>> "Warning: crash_kexec_post_notifiers is set.\n"
> 
> I would say for the time being don't put any extra message. User will
> think what this warning is supposed to mean. We can introduce one if
> we run into many issues.

Ah, I see.

> For the time being we can just look at kernel logs and look at command
> line options and see whether crash_kexec_post_notifiers was set or not.

Yeah, I know. However since the kernel command line is shown in very early
part of kernel logs, I worried about the case that it is pushed out from
kmsg buffer. And usually users will send a bug report with the very last
of the kernel logs because it becomes too long.

Thank you,

-- 
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com

Re: Re: [PATCH] kernel/panic: Add "late_kdump" option for kdump in unstable condition

[Masami Hiramatsu]

(2014/04/18 12:09), Masami Hiramatsu wrote:
> (2014/04/17 22:43), Vivek Goyal wrote:
>> On Thu, Apr 17, 2014 at 10:59:14AM +0900, Masami Hiramatsu wrote:
>>
>> [..]
>>>>> @@ -112,9 +113,14 @@ void panic(const char *fmt, ...)
>>>>>  	/*
>>>>>  	 * If we have crashed and we have a crash kernel loaded let it handle
>>>>>  	 * everything else.
>>>>> -	 * Do we want to call this before we try to display a message?
>>>>> +	 * If we want to call this after we try to display a message, pass
>>>>> +	 * the "late_kdump" option to the kernel.
>>>>>  	 */
>>>>> -	crash_kexec(NULL);
>>>>> +	if (!late_kdump)
>>>>> +		crash_kexec(NULL);
>>>>> +	else
>>>>> +		pr_emerg("Warning: late_kdump option is set. Please DO NOT "
>>>>> +			"report bugs about kdump failure with this option.\n");
>>>>
>>>> I think above message about DO NOT report bugs seems unnecessary. 
>>>
>>> OK, so I just notify the option is set as below.
>>> "Warning: crash_kexec_post_notifiers is set.\n"
>>
>> I would say for the time being don't put any extra message. User will
>> think what this warning is supposed to mean. We can introduce one if
>> we run into many issues.
> 
> Ah, I see.
> 
>> For the time being we can just look at kernel logs and look at command
>> line options and see whether crash_kexec_post_notifiers was set or not.
> 
> Yeah, I know. However since the kernel command line is shown in very early
> part of kernel logs, I worried about the case that it is pushed out from
> kmsg buffer. And usually users will send a bug report with the very last
> of the kernel logs because it becomes too long.

Anyway, I'll send v3 soon to remove that message completely :)

Thank you!

-- 
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com