[1/3] panic: Disable crash_kexec_post_notifiers if kdump is not available

Commit Message

Hidehiro Kawai July 10, 2015, 11:33 a.m. UTC

You can call panic notifiers and kmsg dumpers before kdump by
specifying "crash_kexec_post_notifiers" as a boot parameter.
However, it doesn't make sense if kdump is not available.  In that
case, disable "crash_kexec_post_notifiers" boot parameter so that
you can't change the value of the parameter.

Signed-off-by: Hidehiro Kawai <[e-mail]>
Cc: Andrew Morton <[e-mail]>
Cc: Eric Biederman <[e-mail]>
Cc: Vivek Goyal <[e-mail]>
---
 kernel/panic.c |    2 ++
 1 file changed, 2 insertions(+)



--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Eric W. Biederman July 10, 2015, 1:41 p.m. UTC | #1

Hidehiro Kawai <[e-mail]> writes:

> You can call panic notifiers and kmsg dumpers before kdump by
> specifying "crash_kexec_post_notifiers" as a boot parameter.
> However, it doesn't make sense if kdump is not available.  In that
> case, disable "crash_kexec_post_notifiers" boot parameter so that
> you can't change the value of the parameter.

Nacked-by: "Eric W. Biederman" <[e-mail]>

You are confusing kexec on panic and CONFIG_CRASH_DUMP
which is about the tools for reading the state of the previous kernel.

Eric

> Signed-off-by: Hidehiro Kawai <[e-mail]>
> Cc: Andrew Morton <[e-mail]>
> Cc: Eric Biederman <[e-mail]>
> Cc: Vivek Goyal <[e-mail]>
> ---
>  kernel/panic.c |    2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/kernel/panic.c b/kernel/panic.c
> index 04e91ff..5252331 100644
> --- a/kernel/panic.c
> +++ b/kernel/panic.c
> @@ -502,12 +502,14 @@ __visible void __stack_chk_fail(void)
>  core_param(pause_on_oops, pause_on_oops, int, 0644);
>  core_param(panic_on_warn, panic_on_warn, int, 0644);
>  
> +#ifdef CONFIG_CRASH_DUMP
>  static int __init setup_crash_kexec_post_notifiers(char *s)
>  {
>  	crash_kexec_post_notifiers = true;
>  	return 0;
>  }
>  early_param("crash_kexec_post_notifiers", setup_crash_kexec_post_notifiers);
> +#endif
>  
>  static int __init oops_setup(char *s)
>  {
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Daniel Walker July 13, 2015, 8:26 p.m. UTC | #2

On Fri, Jul 10, 2015 at 08:41:28AM -0500, Eric W. Biederman wrote:
> Hidehiro Kawai <[e-mail]> writes:
> 
> > You can call panic notifiers and kmsg dumpers before kdump by
> > specifying "crash_kexec_post_notifiers" as a boot parameter.
> > However, it doesn't make sense if kdump is not available.  In that
> > case, disable "crash_kexec_post_notifiers" boot parameter so that
> > you can't change the value of the parameter.
> 
> Nacked-by: "Eric W. Biederman" <[e-mail]>

I think it would make sense if he just replaced "kdump" with "kexec".

Daniel
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Eric W. Biederman July 14, 2015, 1:19 a.m. UTC | #3

[e-mail] writes:

> On Fri, Jul 10, 2015 at 08:41:28AM -0500, Eric W. Biederman wrote:
>> Hidehiro Kawai <[e-mail]> writes:
>> 
>> > You can call panic notifiers and kmsg dumpers before kdump by
>> > specifying "crash_kexec_post_notifiers" as a boot parameter.
>> > However, it doesn't make sense if kdump is not available.  In that
>> > case, disable "crash_kexec_post_notifiers" boot parameter so that
>> > you can't change the value of the parameter.
>> 
>> Nacked-by: "Eric W. Biederman" <[e-mail]>
>
> I think it would make sense if he just replaced "kdump" with "kexec".

It would be less insane, however it still makes no sense as without
kexec on panic support crash_kexec is a noop.  So the value of the
seeting makes no difference.

Eric
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Hidehiro Kawai July 14, 2015, 1:56 a.m. UTC | #4

Hello Eric and Daniel,

(2015/07/14 5:26), [e-mail] wrote:
> On Fri, Jul 10, 2015 at 08:41:28AM -0500, Eric W. Biederman wrote:
>> Hidehiro Kawai <[e-mail]> writes:
>>
>>> You can call panic notifiers and kmsg dumpers before kdump by
>>> specifying "crash_kexec_post_notifiers" as a boot parameter.
>>> However, it doesn't make sense if kdump is not available.  In that
>>> case, disable "crash_kexec_post_notifiers" boot parameter so that
>>> you can't change the value of the parameter.
>>
>> Nacked-by: "Eric W. Biederman" <[e-mail]>
>>
>> You are confusing kexec on panic and CONFIG_CRASH_DUMP
>> which is about the tools for reading the state of the previous kernel.

You are right.  I mistook the meaning of CONFIG_CRASH_DUMP.

> I think it would make sense if he just replaced "kdump" with "kexec".

If a user specified crash_kexec_post_notifiers option with kexec
being totally disabled, it just disables notifier and kmsg dumper calls
(please see PATCH 3/3). So as Daniel said, I also think it makes sense
to hide crash_kexec_post_notifiers option if kexec is disabled.

Regards,

Daniel Walker July 14, 2015, 1:59 p.m. UTC | #5

On Mon, Jul 13, 2015 at 08:19:45PM -0500, Eric W. Biederman wrote:
> [e-mail] writes:
> 
> > On Fri, Jul 10, 2015 at 08:41:28AM -0500, Eric W. Biederman wrote:
> >> Hidehiro Kawai <[e-mail]> writes:
> >> 
> >> > You can call panic notifiers and kmsg dumpers before kdump by
> >> > specifying "crash_kexec_post_notifiers" as a boot parameter.
> >> > However, it doesn't make sense if kdump is not available.  In that
> >> > case, disable "crash_kexec_post_notifiers" boot parameter so that
> >> > you can't change the value of the parameter.
> >> 
> >> Nacked-by: "Eric W. Biederman" <[e-mail]>
> >
> > I think it would make sense if he just replaced "kdump" with "kexec".
> 
> It would be less insane, however it still makes no sense as without
> kexec on panic support crash_kexec is a noop.  So the value of the
> seeting makes no difference.

Can you explain more, I don't really understand what you mean. Are you suggesting
the whole "crash_kexec_post_notifiers" feature has no value ?

Daniel
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Vivek Goyal July 14, 2015, 2:20 p.m. UTC | #6

On Tue, Jul 14, 2015 at 01:59:19PM +0000, [e-mail] wrote:
> On Mon, Jul 13, 2015 at 08:19:45PM -0500, Eric W. Biederman wrote:
> > [e-mail] writes:
> > 
> > > On Fri, Jul 10, 2015 at 08:41:28AM -0500, Eric W. Biederman wrote:
> > >> Hidehiro Kawai <[e-mail]> writes:
> > >> 
> > >> > You can call panic notifiers and kmsg dumpers before kdump by
> > >> > specifying "crash_kexec_post_notifiers" as a boot parameter.
> > >> > However, it doesn't make sense if kdump is not available.  In that
> > >> > case, disable "crash_kexec_post_notifiers" boot parameter so that
> > >> > you can't change the value of the parameter.
> > >> 
> > >> Nacked-by: "Eric W. Biederman" <[e-mail]>
> > >
> > > I think it would make sense if he just replaced "kdump" with "kexec".
> > 
> > It would be less insane, however it still makes no sense as without
> > kexec on panic support crash_kexec is a noop.  So the value of the
> > seeting makes no difference.
> 
> Can you explain more, I don't really understand what you mean. Are you suggesting
> the whole "crash_kexec_post_notifiers" feature has no value ?

If CONFIG_KEXEC=n, then crash_kexec() is a nop. So it does not matter
whether crash_kexec() is called before panic notifiers or after.

IOW, what do you gain by disabling crash_kexec_post_notifiers, in 
case of CONFIG_KEXEC=n?

Thanks
Vivek
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Vivek Goyal July 14, 2015, 3:02 p.m. UTC | #7

On Tue, Jul 14, 2015 at 01:59:19PM +0000, [e-mail] wrote:
> On Mon, Jul 13, 2015 at 08:19:45PM -0500, Eric W. Biederman wrote:
> > [e-mail] writes:
> > 
> > > On Fri, Jul 10, 2015 at 08:41:28AM -0500, Eric W. Biederman wrote:
> > >> Hidehiro Kawai <[e-mail]> writes:
> > >> 
> > >> > You can call panic notifiers and kmsg dumpers before kdump by
> > >> > specifying "crash_kexec_post_notifiers" as a boot parameter.
> > >> > However, it doesn't make sense if kdump is not available.  In that
> > >> > case, disable "crash_kexec_post_notifiers" boot parameter so that
> > >> > you can't change the value of the parameter.
> > >> 
> > >> Nacked-by: "Eric W. Biederman" <[e-mail]>
> > >
> > > I think it would make sense if he just replaced "kdump" with "kexec".
> > 
> > It would be less insane, however it still makes no sense as without
> > kexec on panic support crash_kexec is a noop.  So the value of the
> > seeting makes no difference.
> 
> Can you explain more, I don't really understand what you mean. Are you suggesting
> the whole "crash_kexec_post_notifiers" feature has no value ?

Daniel,

BTW, why are you using crash_kexec_post_notifiers commandline? Why not
without it?

Thanks
Vivek
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Daniel Walker July 14, 2015, 3:34 p.m. UTC | #8

On Tue, Jul 14, 2015 at 11:02:08AM -0400, Vivek Goyal wrote:
> On Tue, Jul 14, 2015 at 01:59:19PM +0000, [e-mail] wrote:
> > On Mon, Jul 13, 2015 at 08:19:45PM -0500, Eric W. Biederman wrote:
> > > [e-mail] writes:
> > > 
> > > > On Fri, Jul 10, 2015 at 08:41:28AM -0500, Eric W. Biederman wrote:
> > > >> Hidehiro Kawai <[e-mail]> writes:
> > > >> 
> > > >> > You can call panic notifiers and kmsg dumpers before kdump by
> > > >> > specifying "crash_kexec_post_notifiers" as a boot parameter.
> > > >> > However, it doesn't make sense if kdump is not available.  In that
> > > >> > case, disable "crash_kexec_post_notifiers" boot parameter so that
> > > >> > you can't change the value of the parameter.
> > > >> 
> > > >> Nacked-by: "Eric W. Biederman" <[e-mail]>
> > > >
> > > > I think it would make sense if he just replaced "kdump" with "kexec".
> > > 
> > > It would be less insane, however it still makes no sense as without
> > > kexec on panic support crash_kexec is a noop.  So the value of the
> > > seeting makes no difference.
> > 
> > Can you explain more, I don't really understand what you mean. Are you suggesting
> > the whole "crash_kexec_post_notifiers" feature has no value ?
> 
> Daniel,
> 
> BTW, why are you using crash_kexec_post_notifiers commandline? Why not
> without it?

It was explained in the prior thread but to rehash, the notifiers are used to do a switch
over from the crashed machine to another redundant machine.

Daniel
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Vivek Goyal July 14, 2015, 3:40 p.m. UTC | #9

On Tue, Jul 14, 2015 at 03:34:30PM +0000, [e-mail] wrote:
> On Tue, Jul 14, 2015 at 11:02:08AM -0400, Vivek Goyal wrote:
> > On Tue, Jul 14, 2015 at 01:59:19PM +0000, [e-mail] wrote:
> > > On Mon, Jul 13, 2015 at 08:19:45PM -0500, Eric W. Biederman wrote:
> > > > [e-mail] writes:
> > > > 
> > > > > On Fri, Jul 10, 2015 at 08:41:28AM -0500, Eric W. Biederman wrote:
> > > > >> Hidehiro Kawai <[e-mail]> writes:
> > > > >> 
> > > > >> > You can call panic notifiers and kmsg dumpers before kdump by
> > > > >> > specifying "crash_kexec_post_notifiers" as a boot parameter.
> > > > >> > However, it doesn't make sense if kdump is not available.  In that
> > > > >> > case, disable "crash_kexec_post_notifiers" boot parameter so that
> > > > >> > you can't change the value of the parameter.
> > > > >> 
> > > > >> Nacked-by: "Eric W. Biederman" <[e-mail]>
> > > > >
> > > > > I think it would make sense if he just replaced "kdump" with "kexec".
> > > > 
> > > > It would be less insane, however it still makes no sense as without
> > > > kexec on panic support crash_kexec is a noop.  So the value of the
> > > > seeting makes no difference.
> > > 
> > > Can you explain more, I don't really understand what you mean. Are you suggesting
> > > the whole "crash_kexec_post_notifiers" feature has no value ?
> > 
> > Daniel,
> > 
> > BTW, why are you using crash_kexec_post_notifiers commandline? Why not
> > without it?
> 
> It was explained in the prior thread but to rehash, the notifiers are used to do a switch
> over from the crashed machine to another redundant machine.

So why not detect failure using polling or issue notifications from second
kernel.

IOW, expecting that a crashed machine will be able to deliver notification
reliably is falwed to begin with, IMHO.

If a machine is failing, there are high chance it can't deliver you the
notification. Detecting that failure suing some kind of polling mechanism
might be more reliable. And it will make even kdump mechanism more
reliable so that it does not have to run panic notifiers after the crash.

Thanks
Vivek
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Daniel Walker July 14, 2015, 3:48 p.m. UTC | #10

On Tue, Jul 14, 2015 at 11:40:40AM -0400, Vivek Goyal wrote:
> On Tue, Jul 14, 2015 at 03:34:30PM +0000, [e-mail] wrote:
> > On Tue, Jul 14, 2015 at 11:02:08AM -0400, Vivek Goyal wrote:
> > > On Tue, Jul 14, 2015 at 01:59:19PM +0000, [e-mail] wrote:
> > > > On Mon, Jul 13, 2015 at 08:19:45PM -0500, Eric W. Biederman wrote:
> > > > > [e-mail] writes:
> > > > > 
> > > > > > On Fri, Jul 10, 2015 at 08:41:28AM -0500, Eric W. Biederman wrote:
> > > > > >> Hidehiro Kawai <[e-mail]> writes:
> > > > > >> 
> > > > > >> > You can call panic notifiers and kmsg dumpers before kdump by
> > > > > >> > specifying "crash_kexec_post_notifiers" as a boot parameter.
> > > > > >> > However, it doesn't make sense if kdump is not available.  In that
> > > > > >> > case, disable "crash_kexec_post_notifiers" boot parameter so that
> > > > > >> > you can't change the value of the parameter.
> > > > > >> 
> > > > > >> Nacked-by: "Eric W. Biederman" <[e-mail]>
> > > > > >
> > > > > > I think it would make sense if he just replaced "kdump" with "kexec".
> > > > > 
> > > > > It would be less insane, however it still makes no sense as without
> > > > > kexec on panic support crash_kexec is a noop.  So the value of the
> > > > > seeting makes no difference.
> > > > 
> > > > Can you explain more, I don't really understand what you mean. Are you suggesting
> > > > the whole "crash_kexec_post_notifiers" feature has no value ?
> > > 
> > > Daniel,
> > > 
> > > BTW, why are you using crash_kexec_post_notifiers commandline? Why not
> > > without it?
> > 
> > It was explained in the prior thread but to rehash, the notifiers are used to do a switch
> > over from the crashed machine to another redundant machine.
> 
> So why not detect failure using polling or issue notifications from second
> kernel.
> 
> IOW, expecting that a crashed machine will be able to deliver notification
> reliably is falwed to begin with, IMHO.

It's flawed to think you can kexec, but you still do it right ? I've not gotten into
the deep details of this switching process, but that's how this interface is used.
 
> If a machine is failing, there are high chance it can't deliver you the
> notification. Detecting that failure suing some kind of polling mechanism
> might be more reliable. And it will make even kdump mechanism more
> reliable so that it does not have to run panic notifiers after the crash.

I think what your suggesting is that my company should change how it's hardware works
and that's not really an option for me. This isn't a simple thing like checking over the
network if the machine is down or not, this is way more complex hardware design.

Daniel
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Vivek Goyal July 14, 2015, 4:16 p.m. UTC | #11

On Tue, Jul 14, 2015 at 03:48:33PM +0000, [e-mail] wrote:
> On Tue, Jul 14, 2015 at 11:40:40AM -0400, Vivek Goyal wrote:
> > On Tue, Jul 14, 2015 at 03:34:30PM +0000, [e-mail] wrote:
> > > On Tue, Jul 14, 2015 at 11:02:08AM -0400, Vivek Goyal wrote:
> > > > On Tue, Jul 14, 2015 at 01:59:19PM +0000, [e-mail] wrote:
> > > > > On Mon, Jul 13, 2015 at 08:19:45PM -0500, Eric W. Biederman wrote:
> > > > > > [e-mail] writes:
> > > > > > 
> > > > > > > On Fri, Jul 10, 2015 at 08:41:28AM -0500, Eric W. Biederman wrote:
> > > > > > >> Hidehiro Kawai <[e-mail]> writes:
> > > > > > >> 
> > > > > > >> > You can call panic notifiers and kmsg dumpers before kdump by
> > > > > > >> > specifying "crash_kexec_post_notifiers" as a boot parameter.
> > > > > > >> > However, it doesn't make sense if kdump is not available.  In that
> > > > > > >> > case, disable "crash_kexec_post_notifiers" boot parameter so that
> > > > > > >> > you can't change the value of the parameter.
> > > > > > >> 
> > > > > > >> Nacked-by: "Eric W. Biederman" <[e-mail]>
> > > > > > >
> > > > > > > I think it would make sense if he just replaced "kdump" with "kexec".
> > > > > > 
> > > > > > It would be less insane, however it still makes no sense as without
> > > > > > kexec on panic support crash_kexec is a noop.  So the value of the
> > > > > > seeting makes no difference.
> > > > > 
> > > > > Can you explain more, I don't really understand what you mean. Are you suggesting
> > > > > the whole "crash_kexec_post_notifiers" feature has no value ?
> > > > 
> > > > Daniel,
> > > > 
> > > > BTW, why are you using crash_kexec_post_notifiers commandline? Why not
> > > > without it?
> > > 
> > > It was explained in the prior thread but to rehash, the notifiers are used to do a switch
> > > over from the crashed machine to another redundant machine.
> > 
> > So why not detect failure using polling or issue notifications from second
> > kernel.
> > 
> > IOW, expecting that a crashed machine will be able to deliver notification
> > reliably is falwed to begin with, IMHO.
> 
> It's flawed to think you can kexec, but you still do it right ? I've not gotten into
> the deep details of this switching process, but that's how this interface is used.

Sure. But the deal here is that users of interface know that sometimes it
can be unreliable. And in the absence of more reliable mechanism, somewhat
less reliable mechanism is fine. 

>  
> > If a machine is failing, there are high chance it can't deliver you the
> > notification. Detecting that failure suing some kind of polling mechanism
> > might be more reliable. And it will make even kdump mechanism more
> > reliable so that it does not have to run panic notifiers after the crash.
> 
> I think what your suggesting is that my company should change how it's hardware works
> and that's not really an option for me. This isn't a simple thing like checking over the
> network if the machine is down or not, this is way more complex hardware design.

That means you are ready to live with an unreliable design. There might be
cases where notifier does not get run properly and you will not do switch
despite the fact that OS has failed. I was just trying to nudge you in
a direction which could be more reliable mechanism.

Thanks
Vivek
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Eric W. Biederman July 14, 2015, 5:06 p.m. UTC | #12

Vivek Goyal <[e-mail]> writes:

> On Tue, Jul 14, 2015 at 03:48:33PM +0000, [e-mail] wrote:
>> On Tue, Jul 14, 2015 at 11:40:40AM -0400, Vivek Goyal wrote:
>> > On Tue, Jul 14, 2015 at 03:34:30PM +0000, [e-mail] wrote:
>> > > On Tue, Jul 14, 2015 at 11:02:08AM -0400, Vivek Goyal wrote:
>> > > > On Tue, Jul 14, 2015 at 01:59:19PM +0000, [e-mail] wrote:
>> > > > > On Mon, Jul 13, 2015 at 08:19:45PM -0500, Eric W. Biederman wrote:
>> > > > > > [e-mail] writes:
>> > > > > > 
>> > > > > > > On Fri, Jul 10, 2015 at 08:41:28AM -0500, Eric W. Biederman wrote:
>> > > > > > >> Hidehiro Kawai <[e-mail]> writes:
>> > > > > > >> 
>> > > > > > >> > You can call panic notifiers and kmsg dumpers before kdump by
>> > > > > > >> > specifying "crash_kexec_post_notifiers" as a boot parameter.
>> > > > > > >> > However, it doesn't make sense if kdump is not available.  In that
>> > > > > > >> > case, disable "crash_kexec_post_notifiers" boot parameter so that
>> > > > > > >> > you can't change the value of the parameter.
>> > > > > > >> 
>> > > > > > >> Nacked-by: "Eric W. Biederman" <[e-mail]>
>> > > > > > >
>> > > > > > > I think it would make sense if he just replaced "kdump" with "kexec".
>> > > > > > 
>> > > > > > It would be less insane, however it still makes no sense as without
>> > > > > > kexec on panic support crash_kexec is a noop.  So the value of the
>> > > > > > seeting makes no difference.
>> > > > > 
>> > > > > Can you explain more, I don't really understand what you mean. Are you suggesting
>> > > > > the whole "crash_kexec_post_notifiers" feature has no value ?
>> > > > 
>> > > > Daniel,
>> > > > 
>> > > > BTW, why are you using crash_kexec_post_notifiers commandline? Why not
>> > > > without it?
>> > > 
>> > > It was explained in the prior thread but to rehash, the notifiers are used to do a switch
>> > > over from the crashed machine to another redundant machine.
>> > 
>> > So why not detect failure using polling or issue notifications from second
>> > kernel.
>> > 
>> > IOW, expecting that a crashed machine will be able to deliver notification
>> > reliably is falwed to begin with, IMHO.
>> 
>> It's flawed to think you can kexec, but you still do it right ? I've not gotten into
>> the deep details of this switching process, but that's how this interface is used.
>
> Sure. But the deal here is that users of interface know that sometimes it
> can be unreliable. And in the absence of more reliable mechanism, somewhat
> less reliable mechanism is fine. 
>
>>  
>> > If a machine is failing, there are high chance it can't deliver you the
>> > notification. Detecting that failure suing some kind of polling mechanism
>> > might be more reliable. And it will make even kdump mechanism more
>> > reliable so that it does not have to run panic notifiers after the crash.
>> 
>> I think what your suggesting is that my company should change how it's hardware works
>> and that's not really an option for me. This isn't a simple thing like checking over the
>> network if the machine is down or not, this is way more complex hardware design.
>
> That means you are ready to live with an unreliable design. There might be
> cases where notifier does not get run properly and you will not do switch
> despite the fact that OS has failed. I was just trying to nudge you in
> a direction which could be more reliable mechanism.

Sigh I see some deep confusion going on here.

The panic notifiers are just that panic notifiers.  They have not been
nor should they be tied to kexec.   If those notifiers force a switch
over of between machines I fail to see why you would care if it was
kexec or another panic situation that is forcing that switchover.

Now if you want a reliable design, I strongly recommend as I have been
recommending for the 15 years that magic failover code be placed in
either the new kernel or a stub that preceedes the new kernel.

That gives the greatest reliabilty we know how to engineer, and it lets
you do whatever you need to do.

Especially if it is not desirable for the panic notifiers to run without
the presence of kexec, I very strongly recommend not using them at all
and just writing a stub of code that can run before a new kernel starts.

Eric
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Daniel Walker July 14, 2015, 5:29 p.m. UTC | #13

On Tue, Jul 14, 2015 at 12:06:15PM -0500, Eric W. Biederman wrote:
> Vivek Goyal <[e-mail]> writes:
> 
> > On Tue, Jul 14, 2015 at 03:48:33PM +0000, [e-mail] wrote:
> >> On Tue, Jul 14, 2015 at 11:40:40AM -0400, Vivek Goyal wrote:
> >> > On Tue, Jul 14, 2015 at 03:34:30PM +0000, [e-mail] wrote:
> >> > > On Tue, Jul 14, 2015 at 11:02:08AM -0400, Vivek Goyal wrote:
> >> > > > On Tue, Jul 14, 2015 at 01:59:19PM +0000, [e-mail] wrote:
> >> > > > > On Mon, Jul 13, 2015 at 08:19:45PM -0500, Eric W. Biederman wrote:
> >> > > > > > [e-mail] writes:
> >> > > > > > 
> >> > > > > > > On Fri, Jul 10, 2015 at 08:41:28AM -0500, Eric W. Biederman wrote:
> >> > > > > > >> Hidehiro Kawai <[e-mail]> writes:
> >> > > > > > >> 
> >> > > > > > >> > You can call panic notifiers and kmsg dumpers before kdump by
> >> > > > > > >> > specifying "crash_kexec_post_notifiers" as a boot parameter.
> >> > > > > > >> > However, it doesn't make sense if kdump is not available.  In that
> >> > > > > > >> > case, disable "crash_kexec_post_notifiers" boot parameter so that
> >> > > > > > >> > you can't change the value of the parameter.
> >> > > > > > >> 
> >> > > > > > >> Nacked-by: "Eric W. Biederman" <[e-mail]>
> >> > > > > > >
> >> > > > > > > I think it would make sense if he just replaced "kdump" with "kexec".
> >> > > > > > 
> >> > > > > > It would be less insane, however it still makes no sense as without
> >> > > > > > kexec on panic support crash_kexec is a noop.  So the value of the
> >> > > > > > seeting makes no difference.
> >> > > > > 
> >> > > > > Can you explain more, I don't really understand what you mean. Are you suggesting
> >> > > > > the whole "crash_kexec_post_notifiers" feature has no value ?
> >> > > > 
> >> > > > Daniel,
> >> > > > 
> >> > > > BTW, why are you using crash_kexec_post_notifiers commandline? Why not
> >> > > > without it?
> >> > > 
> >> > > It was explained in the prior thread but to rehash, the notifiers are used to do a switch
> >> > > over from the crashed machine to another redundant machine.
> >> > 
> >> > So why not detect failure using polling or issue notifications from second
> >> > kernel.
> >> > 
> >> > IOW, expecting that a crashed machine will be able to deliver notification
> >> > reliably is falwed to begin with, IMHO.
> >> 
> >> It's flawed to think you can kexec, but you still do it right ? I've not gotten into
> >> the deep details of this switching process, but that's how this interface is used.
> >
> > Sure. But the deal here is that users of interface know that sometimes it
> > can be unreliable. And in the absence of more reliable mechanism, somewhat
> > less reliable mechanism is fine. 
> >
> >>  
> >> > If a machine is failing, there are high chance it can't deliver you the
> >> > notification. Detecting that failure suing some kind of polling mechanism
> >> > might be more reliable. And it will make even kdump mechanism more
> >> > reliable so that it does not have to run panic notifiers after the crash.
> >> 
> >> I think what your suggesting is that my company should change how it's hardware works
> >> and that's not really an option for me. This isn't a simple thing like checking over the
> >> network if the machine is down or not, this is way more complex hardware design.
> >
> > That means you are ready to live with an unreliable design. There might be
> > cases where notifier does not get run properly and you will not do switch
> > despite the fact that OS has failed. I was just trying to nudge you in
> > a direction which could be more reliable mechanism.
> 
> Sigh I see some deep confusion going on here.
> 
> The panic notifiers are just that panic notifiers.  They have not been
> nor should they be tied to kexec.   If those notifiers force a switch
> over of between machines I fail to see why you would care if it was
> kexec or another panic situation that is forcing that switchover.

Hidehiro isn't fixing the failover situation on my side, he's fixing register
information collection when crash_kexec_post_notifiers is used.

Daniel
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Vivek Goyal July 14, 2015, 5:55 p.m. UTC | #14

On Tue, Jul 14, 2015 at 05:29:53PM +0000, [e-mail] wrote:

[..]
> > >> > If a machine is failing, there are high chance it can't deliver you the
> > >> > notification. Detecting that failure suing some kind of polling mechanism
> > >> > might be more reliable. And it will make even kdump mechanism more
> > >> > reliable so that it does not have to run panic notifiers after the crash.
> > >> 
> > >> I think what your suggesting is that my company should change how it's hardware works
> > >> and that's not really an option for me. This isn't a simple thing like checking over the
> > >> network if the machine is down or not, this is way more complex hardware design.
> > >
> > > That means you are ready to live with an unreliable design. There might be
> > > cases where notifier does not get run properly and you will not do switch
> > > despite the fact that OS has failed. I was just trying to nudge you in
> > > a direction which could be more reliable mechanism.
> > 
> > Sigh I see some deep confusion going on here.
> > 
> > The panic notifiers are just that panic notifiers.  They have not been
> > nor should they be tied to kexec.   If those notifiers force a switch
> > over of between machines I fail to see why you would care if it was
> > kexec or another panic situation that is forcing that switchover.
> 
> Hidehiro isn't fixing the failover situation on my side, he's fixing register
> information collection when crash_kexec_post_notifiers is used.

Sure. Given that we have created this new parameter, let us fix it so that
we can capture the other cpu register state in crash dump.

I am little disappointed that it was not tested well when this parameter was
introuced. We should have atleast tested it to the extent to see if there
is proper cpu state present for all cpus in the crash dump.

At that point of time it looked like a simple modification
to allow panic notifiers before crash_kexec().

Thanks
Vivek
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Eric W. Biederman July 14, 2015, 6:01 p.m. UTC | #15

Vivek Goyal <[e-mail]> writes:

> On Tue, Jul 14, 2015 at 05:29:53PM +0000, [e-mail] wrote:
>
> [..]
>> > >> > If a machine is failing, there are high chance it can't deliver you the
>> > >> > notification. Detecting that failure suing some kind of polling mechanism
>> > >> > might be more reliable. And it will make even kdump mechanism more
>> > >> > reliable so that it does not have to run panic notifiers after the crash.
>> > >> 
>> > >> I think what your suggesting is that my company should change how it's hardware works
>> > >> and that's not really an option for me. This isn't a simple thing like checking over the
>> > >> network if the machine is down or not, this is way more complex hardware design.
>> > >
>> > > That means you are ready to live with an unreliable design. There might be
>> > > cases where notifier does not get run properly and you will not do switch
>> > > despite the fact that OS has failed. I was just trying to nudge you in
>> > > a direction which could be more reliable mechanism.
>> > 
>> > Sigh I see some deep confusion going on here.
>> > 
>> > The panic notifiers are just that panic notifiers.  They have not been
>> > nor should they be tied to kexec.   If those notifiers force a switch
>> > over of between machines I fail to see why you would care if it was
>> > kexec or another panic situation that is forcing that switchover.
>> 
>> Hidehiro isn't fixing the failover situation on my side, he's fixing register
>> information collection when crash_kexec_post_notifiers is used.
>
> Sure. Given that we have created this new parameter, let us fix it so that
> we can capture the other cpu register state in crash dump.
>
> I am little disappointed that it was not tested well when this parameter was
> introuced. We should have atleast tested it to the extent to see if there
> is proper cpu state present for all cpus in the crash dump.
>
> At that point of time it looked like a simple modification
> to allow panic notifiers before crash_kexec().

Either that or we say no one cares enough, and it known broken so let's
just revert the fool thing.

I honestly can't see how to support panic notifiers, before kexec.
There is no way to tell what is being done and all of the pieces
including smp_send_stop are known to be buggy.

It isn't like this latest set of patches was reviewed/tested much
better, as the first patch was wrong.

Eric
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Vivek Goyal July 14, 2015, 6:23 p.m. UTC | #16

On Tue, Jul 14, 2015 at 01:01:12PM -0500, Eric W. Biederman wrote:
> Vivek Goyal <[e-mail]> writes:
> 
> > On Tue, Jul 14, 2015 at 05:29:53PM +0000, [e-mail] wrote:
> >
> > [..]
> >> > >> > If a machine is failing, there are high chance it can't deliver you the
> >> > >> > notification. Detecting that failure suing some kind of polling mechanism
> >> > >> > might be more reliable. And it will make even kdump mechanism more
> >> > >> > reliable so that it does not have to run panic notifiers after the crash.
> >> > >> 
> >> > >> I think what your suggesting is that my company should change how it's hardware works
> >> > >> and that's not really an option for me. This isn't a simple thing like checking over the
> >> > >> network if the machine is down or not, this is way more complex hardware design.
> >> > >
> >> > > That means you are ready to live with an unreliable design. There might be
> >> > > cases where notifier does not get run properly and you will not do switch
> >> > > despite the fact that OS has failed. I was just trying to nudge you in
> >> > > a direction which could be more reliable mechanism.
> >> > 
> >> > Sigh I see some deep confusion going on here.
> >> > 
> >> > The panic notifiers are just that panic notifiers.  They have not been
> >> > nor should they be tied to kexec.   If those notifiers force a switch
> >> > over of between machines I fail to see why you would care if it was
> >> > kexec or another panic situation that is forcing that switchover.
> >> 
> >> Hidehiro isn't fixing the failover situation on my side, he's fixing register
> >> information collection when crash_kexec_post_notifiers is used.
> >
> > Sure. Given that we have created this new parameter, let us fix it so that
> > we can capture the other cpu register state in crash dump.
> >
> > I am little disappointed that it was not tested well when this parameter was
> > introuced. We should have atleast tested it to the extent to see if there
> > is proper cpu state present for all cpus in the crash dump.
> >
> > At that point of time it looked like a simple modification
> > to allow panic notifiers before crash_kexec().
> 
> Either that or we say no one cares enough, and it known broken so let's
> just revert the fool thing.

Masami, you introduced this option. Are you fine with the revert? Is it
really being used and tested?

> I honestly can't see how to support panic notifiers, before kexec.
> There is no way to tell what is being done and all of the pieces
> including smp_send_stop are known to be buggy.

we should be able to replace smp_send_stop() with what crash_kexec() is
doing to stop the machine? If yes, then it should be fine I guess. This
parameter description clearly says that specify it at your own risk. So
we are not issuing a big support statement for successful kdump after
panic notifiers. If it is something fixable, otherwise user needs
to deal with it.

Thanks
Vivek
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Masami Hiramatsu July 15, 2015, 5:16 a.m. UTC | #17

On 2015/07/15 3:23, Vivek Goyal wrote:
> On Tue, Jul 14, 2015 at 01:01:12PM -0500, Eric W. Biederman wrote:
>> Vivek Goyal <[e-mail]> writes:
>>
>>> On Tue, Jul 14, 2015 at 05:29:53PM +0000, [e-mail] wrote:
>>>
>>> [..]
>>>>>>>> If a machine is failing, there are high chance it can't deliver you the
>>>>>>>> notification. Detecting that failure suing some kind of polling mechanism
>>>>>>>> might be more reliable. And it will make even kdump mechanism more
>>>>>>>> reliable so that it does not have to run panic notifiers after the crash.
>>>>>>>
>>>>>>> I think what your suggesting is that my company should change how it's hardware works
>>>>>>> and that's not really an option for me. This isn't a simple thing like checking over the
>>>>>>> network if the machine is down or not, this is way more complex hardware design.
>>>>>>
>>>>>> That means you are ready to live with an unreliable design. There might be
>>>>>> cases where notifier does not get run properly and you will not do switch
>>>>>> despite the fact that OS has failed. I was just trying to nudge you in
>>>>>> a direction which could be more reliable mechanism.
>>>>>
>>>>> Sigh I see some deep confusion going on here.
>>>>>
>>>>> The panic notifiers are just that panic notifiers.  They have not been
>>>>> nor should they be tied to kexec.   If those notifiers force a switch
>>>>> over of between machines I fail to see why you would care if it was
>>>>> kexec or another panic situation that is forcing that switchover.
>>>>
>>>> Hidehiro isn't fixing the failover situation on my side, he's fixing register
>>>> information collection when crash_kexec_post_notifiers is used.
>>>
>>> Sure. Given that we have created this new parameter, let us fix it so that
>>> we can capture the other cpu register state in crash dump.
>>>
>>> I am little disappointed that it was not tested well when this parameter was
>>> introuced. We should have atleast tested it to the extent to see if there
>>> is proper cpu state present for all cpus in the crash dump.
>>>
>>> At that point of time it looked like a simple modification
>>> to allow panic notifiers before crash_kexec().
>>
>> Either that or we say no one cares enough, and it known broken so let's
>> just revert the fool thing.
> 
> Masami, you introduced this option. Are you fine with the revert? Is it
> really being used and tested?

Actually, it is tested but under very limited situation. I think we
need a clear acceptance criteria, IOW, we need a testset for kdump
so that we can make things better.
Would you have it? maybe we can push it into kselftest.

>> I honestly can't see how to support panic notifiers, before kexec.
>> There is no way to tell what is being done and all of the pieces
>> including smp_send_stop are known to be buggy.
> 
> we should be able to replace smp_send_stop() with what crash_kexec() is
> doing to stop the machine? If yes, then it should be fine I guess. This
> parameter description clearly says that specify it at your own risk. So
> we are not issuing a big support statement for successful kdump after
> panic notifiers. If it is something fixable, otherwise user needs
> to deal with it.

Agreed (as I've sent in other replay).

Thank you,

Hidehiro Kawai July 15, 2015, 10:49 a.m. UTC | #18

(2015/07/15 0:40), Vivek Goyal wrote:
> On Tue, Jul 14, 2015 at 03:34:30PM +0000, [e-mail] wrote:
>> On Tue, Jul 14, 2015 at 11:02:08AM -0400, Vivek Goyal wrote:
>>> On Tue, Jul 14, 2015 at 01:59:19PM +0000, [e-mail] wrote:
>>>> On Mon, Jul 13, 2015 at 08:19:45PM -0500, Eric W. Biederman wrote:
>>>>> [e-mail] writes:
>>>>>
>>>>>> On Fri, Jul 10, 2015 at 08:41:28AM -0500, Eric W. Biederman wrote:
>>>>>>> Hidehiro Kawai <[e-mail]> writes:
>>>>>>>
>>>>>>>> You can call panic notifiers and kmsg dumpers before kdump by
>>>>>>>> specifying "crash_kexec_post_notifiers" as a boot parameter.
>>>>>>>> However, it doesn't make sense if kdump is not available.  In that
>>>>>>>> case, disable "crash_kexec_post_notifiers" boot parameter so that
>>>>>>>> you can't change the value of the parameter.
>>>>>>>
>>>>>>> Nacked-by: "Eric W. Biederman" <[e-mail]>
>>>>>>
>>>>>> I think it would make sense if he just replaced "kdump" with "kexec".
>>>>>
>>>>> It would be less insane, however it still makes no sense as without
>>>>> kexec on panic support crash_kexec is a noop.  So the value of the
>>>>> seeting makes no difference.
>>>>
>>>> Can you explain more, I don't really understand what you mean. Are you suggesting
>>>> the whole "crash_kexec_post_notifiers" feature has no value ?
>>>
>>> Daniel,
>>>
>>> BTW, why are you using crash_kexec_post_notifiers commandline? Why not
>>> without it?
>>
>> It was explained in the prior thread but to rehash, the notifiers are used to do a switch
>> over from the crashed machine to another redundant machine.
> 
> So why not detect failure using polling or issue notifications from second
> kernel.

Polling is not sufficient because some kernel parts may be
alive even if the responder of the polling is dead.  We want
to notify the failure after stopping other CPUs.

Notifying from second kernel needs to wait for the kernel
booted up and device initialization if needed, and this
is not applicable if we want to do fast switchover.

Notifying just before second kernel, as Eric stated, is
one of the reliable option although we can't do complicate
things there.  For example, we can notify the failure by
writing some specific I/O registers in purgatory codes
provided by kexec command.  Since the purgatory codes are
currently embedded into kexec command, so we might need to
modify the mechanism to be pluggable because how to notify
will differ among vendors.

Anyway, this is the case of switchover use case.  If we want
to save minimal information before kdump, notifiers or
kmsg_dump() can be used.

> IOW, expecting that a crashed machine will be able to deliver notification
> reliably is falwed to begin with, IMHO.

I think it depends on what callback is used.  Most of panic
notifiers just do memory copy or I/O register access.
Of course, there are relatively complicate notifiers too,
and I'm preparing patch sets for hardening for that case.

Regards,
Hidehiro Kawai


--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/kernel/panic.c b/kernel/panic.c
index 04e91ff..5252331 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -502,12 +502,14 @@  __visible void __stack_chk_fail(void)
 core_param(pause_on_oops, pause_on_oops, int, 0644);
 core_param(panic_on_warn, panic_on_warn, int, 0644);
 
+#ifdef CONFIG_CRASH_DUMP
 static int __init setup_crash_kexec_post_notifiers(char *s)
 {
 	crash_kexec_post_notifiers = true;
 	return 0;
 }
 early_param("crash_kexec_post_notifiers", setup_crash_kexec_post_notifiers);
+#endif
 
 static int __init oops_setup(char *s)
 {