Re: kexec crash kernel running with watchdog enabled

Re: kexec crash kernel running with watchdog enabled

[Masami Hiramatsu]

Hi,

On 2015/06/23 23:05, dwalker@fifo99.com wrote:
> 
> Hi,
> 
> There was a commit in kernel/panic.c which altered when the kexec crash kernel is executed,
> 
> commit f06e5153f4ae2e2f3b0300f0e260e40cb7fefd45
> Author: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> Date:   Fri Jun 6 14:37:07 2014 -0700
> 
>     kernel/panic.c: add "crash_kexec_post_notifiers" option for kdump after panic_notifers
> 
> 
> This made it possible for smp_send_stop() to be executed prior to calling the kexec crash
> kernel.
> 
> The issue is that smp_send_stop() offlines the cores, and other code depend on the cores being online.
> 
> In my case on Octeon here's an example,
> 
> panic()
>  crash_kexec()
>   machine_crash_shutdown()
>    octeon_generic_shutdown()
> 
> Inside octeon_generic_shutdown() the Octeon watchdog is shutdown for_each_online_cpu(), but since
> most of the cpu's already got offlined in smp_send_stop() it means the watchdog is still alive on
> those cores. This results in a reboot during the crash kernel execution.

Ah, I see.

> Another example seem to be in default_machine_crash_shutdown() where crash_kexec_prepare_cpus() depends
> on an IPI for saving the registers on different cores. However, the cpu's are all offlined with
> interrupts disabled so they won't be running those IPI's in this case.
> 
> I'm looking for any advice on how this should be fixed, or if it's already fixed. I'm not going to be
> submitting a patch so if anyone wants to submit one feel free to do so.

Hmm, IMHO, when the cpu goes to offline in appropriate way(smp_send_stop), it should stop
watchdog timer on the offlined cpu too.
Or, you can also register crash handler which stops all watchdogs, but it's a bit tricky.

Thank you,

-- 
Masami HIRAMATSU
Linux Technology Research Center, System Productivity Research Dept.
Center for Technology Innovation - Systems Engineering
Hitachi, Ltd., Research & Development Group
E-mail: masami.hiramatsu.pt@hitachi.com

Re: kexec crash kernel running with watchdog enabled

[dwalker]

On Wed, Jun 24, 2015 at 03:52:48PM +0900, Masami Hiramatsu wrote:
> Hi,
> 
> On 2015/06/23 23:05, dwalker@fifo99.com wrote:
> > 
> > Hi,
> > 
> > There was a commit in kernel/panic.c which altered when the kexec crash kernel is executed,
> > 
> > commit f06e5153f4ae2e2f3b0300f0e260e40cb7fefd45
> > Author: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> > Date:   Fri Jun 6 14:37:07 2014 -0700
> > 
> >     kernel/panic.c: add "crash_kexec_post_notifiers" option for kdump after panic_notifers
> > 
> > 
> > This made it possible for smp_send_stop() to be executed prior to calling the kexec crash
> > kernel.
> > 
> > The issue is that smp_send_stop() offlines the cores, and other code depend on the cores being online.
> > 
> > In my case on Octeon here's an example,
> > 
> > panic()
> >  crash_kexec()
> >   machine_crash_shutdown()
> >    octeon_generic_shutdown()
> > 
> > Inside octeon_generic_shutdown() the Octeon watchdog is shutdown for_each_online_cpu(), but since
> > most of the cpu's already got offlined in smp_send_stop() it means the watchdog is still alive on
> > those cores. This results in a reboot during the crash kernel execution.
> 
> Ah, I see.
> 
> > Another example seem to be in default_machine_crash_shutdown() where crash_kexec_prepare_cpus() depends
> > on an IPI for saving the registers on different cores. However, the cpu's are all offlined with
> > interrupts disabled so they won't be running those IPI's in this case.
> > 
> > I'm looking for any advice on how this should be fixed, or if it's already fixed. I'm not going to be
> > submitting a patch so if anyone wants to submit one feel free to do so.
> 
> Hmm, IMHO, when the cpu goes to offline in appropriate way(smp_send_stop), it should stop
> watchdog timer on the offlined cpu too.
> Or, you can also register crash handler which stops all watchdogs, but it's a bit tricky.
> 

That doesn't really fix all the issue tho. As I was explaining generic MIPS code depends on the cpu's
effectively being online for crash data collection (with an IPI). This issue may effect other architectures also,
because smp_send_stop() offlines the cpu on other architectures also. I haven't surveyed the other architectures
enough to know what issue could happen from this tho.

Is it possible to move the smp_send_stop() below the notifiers ? I'm just throwing out ideas.

Daniel

Re: kexec crash kernel running with watchdog enabled

[Eric W. Biederman]

dwalker@fifo99.com writes:

> On Wed, Jun 24, 2015 at 03:52:48PM +0900, Masami Hiramatsu wrote:
>> Hi,
>> 
>> On 2015/06/23 23:05, dwalker@fifo99.com wrote:
>> > 
>> > Hi,
>> > 
>> > There was a commit in kernel/panic.c which altered when the kexec crash kernel is executed,
>> > 
>> > commit f06e5153f4ae2e2f3b0300f0e260e40cb7fefd45
>> > Author: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
>> > Date:   Fri Jun 6 14:37:07 2014 -0700
>> > 
>> >     kernel/panic.c: add "crash_kexec_post_notifiers" option for kdump after panic_notifers
>> > 
>> > 
>> > This made it possible for smp_send_stop() to be executed prior to calling the kexec crash
>> > kernel.
>> > 
>> > The issue is that smp_send_stop() offlines the cores, and other code depend on the cores being online.
>> > 
>> > In my case on Octeon here's an example,
>> > 
>> > panic()
>> >  crash_kexec()
>> >   machine_crash_shutdown()
>> >    octeon_generic_shutdown()
>> > 
>> > Inside octeon_generic_shutdown() the Octeon watchdog is shutdown for_each_online_cpu(), but since
>> > most of the cpu's already got offlined in smp_send_stop() it means the watchdog is still alive on
>> > those cores. This results in a reboot during the crash kernel execution.
>> 
>> Ah, I see.
>> 
>> > Another example seem to be in default_machine_crash_shutdown() where crash_kexec_prepare_cpus() depends
>> > on an IPI for saving the registers on different cores. However, the cpu's are all offlined with
>> > interrupts disabled so they won't be running those IPI's in this case.
>> > 
>> > I'm looking for any advice on how this should be fixed, or if it's already fixed. I'm not going to be
>> > submitting a patch so if anyone wants to submit one feel free to do so.
>> 
>> Hmm, IMHO, when the cpu goes to offline in appropriate way(smp_send_stop), it should stop
>> watchdog timer on the offlined cpu too.
>> Or, you can also register crash handler which stops all watchdogs, but it's a bit tricky.
>> 
>
> That doesn't really fix all the issue tho. As I was explaining generic MIPS code depends on the cpu's
> effectively being online for crash data collection (with an IPI). This issue may effect other architectures also,
> because smp_send_stop() offlines the cpu on other architectures also. I haven't surveyed the other architectures
> enough to know what issue could happen from this tho.
>
> Is it possible to move the smp_send_stop() below the notifiers ? I'm
> just throwing out ideas.

In general if you want reliability don't pass the kernel command line to
run the panic notifiers.

Eric

Re: kexec crash kernel running with watchdog enabled

[dwalker]

On Wed, Jun 24, 2015 at 12:06:03PM -0500, Eric W. Biederman wrote:
> dwalker@fifo99.com writes:
> 
> > On Wed, Jun 24, 2015 at 03:52:48PM +0900, Masami Hiramatsu wrote:
> >> Hi,
> >> 
> >> On 2015/06/23 23:05, dwalker@fifo99.com wrote:
> >> > 
> >> > Hi,
> >> > 
> >> > There was a commit in kernel/panic.c which altered when the kexec crash kernel is executed,
> >> > 
> >> > commit f06e5153f4ae2e2f3b0300f0e260e40cb7fefd45
> >> > Author: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> >> > Date:   Fri Jun 6 14:37:07 2014 -0700
> >> > 
> >> >     kernel/panic.c: add "crash_kexec_post_notifiers" option for kdump after panic_notifers
> >> > 
> >> > 
> >> > This made it possible for smp_send_stop() to be executed prior to calling the kexec crash
> >> > kernel.
> >> > 
> >> > The issue is that smp_send_stop() offlines the cores, and other code depend on the cores being online.
> >> > 
> >> > In my case on Octeon here's an example,
> >> > 
> >> > panic()
> >> >  crash_kexec()
> >> >   machine_crash_shutdown()
> >> >    octeon_generic_shutdown()
> >> > 
> >> > Inside octeon_generic_shutdown() the Octeon watchdog is shutdown for_each_online_cpu(), but since
> >> > most of the cpu's already got offlined in smp_send_stop() it means the watchdog is still alive on
> >> > those cores. This results in a reboot during the crash kernel execution.
> >> 
> >> Ah, I see.
> >> 
> >> > Another example seem to be in default_machine_crash_shutdown() where crash_kexec_prepare_cpus() depends
> >> > on an IPI for saving the registers on different cores. However, the cpu's are all offlined with
> >> > interrupts disabled so they won't be running those IPI's in this case.
> >> > 
> >> > I'm looking for any advice on how this should be fixed, or if it's already fixed. I'm not going to be
> >> > submitting a patch so if anyone wants to submit one feel free to do so.
> >> 
> >> Hmm, IMHO, when the cpu goes to offline in appropriate way(smp_send_stop), it should stop
> >> watchdog timer on the offlined cpu too.
> >> Or, you can also register crash handler which stops all watchdogs, but it's a bit tricky.
> >> 
> >
> > That doesn't really fix all the issue tho. As I was explaining generic MIPS code depends on the cpu's
> > effectively being online for crash data collection (with an IPI). This issue may effect other architectures also,
> > because smp_send_stop() offlines the cpu on other architectures also. I haven't surveyed the other architectures
> > enough to know what issue could happen from this tho.
> >
> > Is it possible to move the smp_send_stop() below the notifiers ? I'm
> > just throwing out ideas.
> 
> In general if you want reliability don't pass the kernel command line to
> run the panic notifiers.


We use the panic notifiers to "fail over" , or switch to a working machine. We could do
that in the crash kernel, but we would have to wait several seconds which is too long. Do
you know of a more stable way to accomplish that ?

Daniel

Re: kexec crash kernel running with watchdog enabled

[Hidehiro Kawai]

Hi,

(2015/06/25 1:31), dwalker@fifo99.com wrote:
> On Wed, Jun 24, 2015 at 03:52:48PM +0900, Masami Hiramatsu wrote:
>> Hi,
>>
>> On 2015/06/23 23:05, dwalker@fifo99.com wrote:
>>>
>>> Hi,
>>>
>>> There was a commit in kernel/panic.c which altered when the kexec crash kernel is executed,
>>>
>>> commit f06e5153f4ae2e2f3b0300f0e260e40cb7fefd45
>>> Author: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
>>> Date:   Fri Jun 6 14:37:07 2014 -0700
>>>
>>>     kernel/panic.c: add "crash_kexec_post_notifiers" option for kdump after panic_notifers
>>>
>>>
>>> This made it possible for smp_send_stop() to be executed prior to calling the kexec crash
>>> kernel.
>>>
>>> The issue is that smp_send_stop() offlines the cores, and other code depend on the cores being online.
>>>
>>> In my case on Octeon here's an example,
>>>
>>> panic()
>>>  crash_kexec()
>>>   machine_crash_shutdown()
>>>    octeon_generic_shutdown()
>>>
>>> Inside octeon_generic_shutdown() the Octeon watchdog is shutdown for_each_online_cpu(), but since
>>> most of the cpu's already got offlined in smp_send_stop() it means the watchdog is still alive on
>>> those cores. This results in a reboot during the crash kernel execution.
>>
>> Ah, I see.
>>
>>> Another example seem to be in default_machine_crash_shutdown() where crash_kexec_prepare_cpus() depends
>>> on an IPI for saving the registers on different cores. However, the cpu's are all offlined with
>>> interrupts disabled so they won't be running those IPI's in this case.
>>>
>>> I'm looking for any advice on how this should be fixed, or if it's already fixed. I'm not going to be
>>> submitting a patch so if anyone wants to submit one feel free to do so.
>>
>> Hmm, IMHO, when the cpu goes to offline in appropriate way(smp_send_stop), it should stop
>> watchdog timer on the offlined cpu too.
>> Or, you can also register crash handler which stops all watchdogs, but it's a bit tricky.
>>
> 
> That doesn't really fix all the issue tho. As I was explaining generic MIPS code depends on the cpu's
> effectively being online for crash data collection (with an IPI). This issue may effect other architectures also,
> because smp_send_stop() offlines the cpu on other architectures also. I haven't surveyed the other architectures
> enough to know what issue could happen from this tho.
> 
> Is it possible to move the smp_send_stop() below the notifiers ? I'm just throwing out ideas.

No, that doesn't works.  Some notifiers assume that they run in
single core mode.

Another possible solution is to add notifiers just after
machine_crash_shutdown() like this:

void panic(const char *fmt, ...)
...
-	if (!crash_kexec_post_notifiers)
-		crash_kexec(NULL);
+	crash_kexec(NULL, buf);

  and

-void crash_kexec(struct pt_regs *regs)
+void crash_kexec(struct pt_regs *regs, char *msg)
 ...
 		if (kexec_crash_image) {
 			struct pt_regs fixed_regs;
 
 			crash_setup_regs(&fixed_regs, regs);
 			crash_save_vmcoreinfo();
 			machine_crash_shutdown(&fixed_regs);
+			if (crash_kexec_post_notifiers) {
+				kmsg_dump(KMSG_DUMP_PANIC);
+				atomic_notifier_call_chain(&panic_notifier_list, 0, msg);
+			}
                        machine_kexec(kexec_crash_image);

Most of archs stop other cores in machine_crash_shutdown(),
so it will work well.  Furthermore, it simplifies the special
case where crash_kexec() is called without entering panic().

However, we need some tweaks for sh and s390 cases.  As for sh,
it seems not to stop other cores in the crash_kexec() sequence
(kdump support is incompleted?).  For s390, smp_send_stop() is
called in machine_kexec() but not machine_crash_shutdown().

Regards,

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

Re: kexec crash kernel running with watchdog enabled

[dwalker]

On Fri, Jun 26, 2015 at 10:02:00AM +0900, Hidehiro Kawai wrote:
> Hi,
> 
> (2015/06/25 1:31), dwalker@fifo99.com wrote:
> > On Wed, Jun 24, 2015 at 03:52:48PM +0900, Masami Hiramatsu wrote:
> >> Hi,
> >>
> >> On 2015/06/23 23:05, dwalker@fifo99.com wrote:
> >>>
> >>> Hi,
> >>>
> >>> There was a commit in kernel/panic.c which altered when the kexec crash kernel is executed,
> >>>
> >>> commit f06e5153f4ae2e2f3b0300f0e260e40cb7fefd45
> >>> Author: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> >>> Date:   Fri Jun 6 14:37:07 2014 -0700
> >>>
> >>>     kernel/panic.c: add "crash_kexec_post_notifiers" option for kdump after panic_notifers
> >>>
> >>>
> >>> This made it possible for smp_send_stop() to be executed prior to calling the kexec crash
> >>> kernel.
> >>>
> >>> The issue is that smp_send_stop() offlines the cores, and other code depend on the cores being online.
> >>>
> >>> In my case on Octeon here's an example,
> >>>
> >>> panic()
> >>>  crash_kexec()
> >>>   machine_crash_shutdown()
> >>>    octeon_generic_shutdown()
> >>>
> >>> Inside octeon_generic_shutdown() the Octeon watchdog is shutdown for_each_online_cpu(), but since
> >>> most of the cpu's already got offlined in smp_send_stop() it means the watchdog is still alive on
> >>> those cores. This results in a reboot during the crash kernel execution.
> >>
> >> Ah, I see.
> >>
> >>> Another example seem to be in default_machine_crash_shutdown() where crash_kexec_prepare_cpus() depends
> >>> on an IPI for saving the registers on different cores. However, the cpu's are all offlined with
> >>> interrupts disabled so they won't be running those IPI's in this case.
> >>>
> >>> I'm looking for any advice on how this should be fixed, or if it's already fixed. I'm not going to be
> >>> submitting a patch so if anyone wants to submit one feel free to do so.
> >>
> >> Hmm, IMHO, when the cpu goes to offline in appropriate way(smp_send_stop), it should stop
> >> watchdog timer on the offlined cpu too.
> >> Or, you can also register crash handler which stops all watchdogs, but it's a bit tricky.
> >>
> > 
> > That doesn't really fix all the issue tho. As I was explaining generic MIPS code depends on the cpu's
> > effectively being online for crash data collection (with an IPI). This issue may effect other architectures also,
> > because smp_send_stop() offlines the cpu on other architectures also. I haven't surveyed the other architectures
> > enough to know what issue could happen from this tho.
> > 
> > Is it possible to move the smp_send_stop() below the notifiers ? I'm just throwing out ideas.
> 
> No, that doesn't works.  Some notifiers assume that they run in
> single core mode.
> 
> Another possible solution is to add notifiers just after
> machine_crash_shutdown() like this:
> 
> void panic(const char *fmt, ...)
> ...
> -	if (!crash_kexec_post_notifiers)
> -		crash_kexec(NULL);
> +	crash_kexec(NULL, buf);
> 
>   and
> 
> -void crash_kexec(struct pt_regs *regs)
> +void crash_kexec(struct pt_regs *regs, char *msg)
>  ...
>  		if (kexec_crash_image) {
>  			struct pt_regs fixed_regs;
>  
>  			crash_setup_regs(&fixed_regs, regs);
>  			crash_save_vmcoreinfo();
>  			machine_crash_shutdown(&fixed_regs);
> +			if (crash_kexec_post_notifiers) {
> +				kmsg_dump(KMSG_DUMP_PANIC);
> +				atomic_notifier_call_chain(&panic_notifier_list, 0, msg);
> +			}
>                         machine_kexec(kexec_crash_image);
> 
> Most of archs stop other cores in machine_crash_shutdown(),
> so it will work well.  Furthermore, it simplifies the special
> case where crash_kexec() is called without entering panic().
> 
> However, we need some tweaks for sh and s390 cases.  As for sh,
> it seems not to stop other cores in the crash_kexec() sequence
> (kdump support is incompleted?).  For s390, smp_send_stop() is
> called in machine_kexec() but not machine_crash_shutdown().

You could add an ifdef into the __setup() to filter out s390 and sh, until we figure out what
to do there. So the "crash_kexec_post_notifiers" wouldn't be available for those platforms.

Daniel

Re: kexec crash kernel running with watchdog enabled

[Hidehiro Kawai]

Hi,

(2015/06/27 3:33), dwalker@fifo99.com wrote:> On Fri, Jun 26, 2015 at 10:02:00AM +0900, Hidehiro Kawai wrote:
>> Hi,
>>
>> (2015/06/25 1:31), dwalker@fifo99.com wrote:
>>> On Wed, Jun 24, 2015 at 03:52:48PM +0900, Masami Hiramatsu wrote:
>>>> Hi,
>>>>
>>>> On 2015/06/23 23:05, dwalker@fifo99.com wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>> There was a commit in kernel/panic.c which altered when the kexec crash kernel is executed,
>>>>>
>>>>> commit f06e5153f4ae2e2f3b0300f0e260e40cb7fefd45
>>>>> Author: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
>>>>> Date:   Fri Jun 6 14:37:07 2014 -0700
>>>>>
>>>>>     kernel/panic.c: add "crash_kexec_post_notifiers" option for kdump after panic_notifers
>>>>>
>>>>>
>>>>> This made it possible for smp_send_stop() to be executed prior to calling the kexec crash
>>>>> kernel.
>>>>>
>>>>> The issue is that smp_send_stop() offlines the cores, and other code depend on the cores being online.
>>>>>
>>>>> In my case on Octeon here's an example,
>>>>>
>>>>> panic()
>>>>>  crash_kexec()
>>>>>   machine_crash_shutdown()
>>>>>    octeon_generic_shutdown()
>>>>>
>>>>> Inside octeon_generic_shutdown() the Octeon watchdog is shutdown for_each_online_cpu(), but since
>>>>> most of the cpu's already got offlined in smp_send_stop() it means the watchdog is still alive on
>>>>> those cores. This results in a reboot during the crash kernel execution.
>>>>
>>>> Ah, I see.
>>>>
>>>>> Another example seem to be in default_machine_crash_shutdown() where crash_kexec_prepare_cpus() depends
>>>>> on an IPI for saving the registers on different cores. However, the cpu's are all offlined with
>>>>> interrupts disabled so they won't be running those IPI's in this case.
>>>>>
>>>>> I'm looking for any advice on how this should be fixed, or if it's already fixed. I'm not going to be
>>>>> submitting a patch so if anyone wants to submit one feel free to do so.
>>>>
>>>> Hmm, IMHO, when the cpu goes to offline in appropriate way(smp_send_stop), it should stop
>>>> watchdog timer on the offlined cpu too.
>>>> Or, you can also register crash handler which stops all watchdogs, but it's a bit tricky.
>>>>
>>>
>>> That doesn't really fix all the issue tho. As I was explaining generic MIPS code depends on the cpu's
>>> effectively being online for crash data collection (with an IPI). This issue may effect other architectures also,
>>> because smp_send_stop() offlines the cpu on other architectures also. I haven't surveyed the other architectures
>>> enough to know what issue could happen from this tho.
>>>
>>> Is it possible to move the smp_send_stop() below the notifiers ? I'm just throwing out ideas.
>>
>> No, that doesn't works.  Some notifiers assume that they run in
>> single core mode.
>>
>> Another possible solution is to add notifiers just after
>> machine_crash_shutdown() like this:
>>
>> void panic(const char *fmt, ...)
>> ...
>> -	if (!crash_kexec_post_notifiers)
>> -		crash_kexec(NULL);
>> +	crash_kexec(NULL, buf);
>>
>>   and
>>
>> -void crash_kexec(struct pt_regs *regs)
>> +void crash_kexec(struct pt_regs *regs, char *msg)
>>  ...
>>  		if (kexec_crash_image) {
>>  			struct pt_regs fixed_regs;
>>  
>>  			crash_setup_regs(&fixed_regs, regs);
>>  			crash_save_vmcoreinfo();
>>  			machine_crash_shutdown(&fixed_regs);
>> +			if (crash_kexec_post_notifiers) {
>> +				kmsg_dump(KMSG_DUMP_PANIC);
>> +				atomic_notifier_call_chain(&panic_notifier_list, 0, msg);
>> +			}
>>                         machine_kexec(kexec_crash_image);
>>
>> Most of archs stop other cores in machine_crash_shutdown(),
>> so it will work well.  Furthermore, it simplifies the special
>> case where crash_kexec() is called without entering panic().
>>
>> However, we need some tweaks for sh and s390 cases.  As for sh,
>> it seems not to stop other cores in the crash_kexec() sequence
>> (kdump support is incompleted?).  For s390, smp_send_stop() is
>> called in machine_kexec() but not machine_crash_shutdown().
> 
> You could add an ifdef into the __setup() to filter out s390 and sh, until we figure out what
> to do there. So the "crash_kexec_post_notifiers" wouldn't be available for those platforms.

I agree on disabling the "crash_kexec_post_notifiers" feature for
s390 and sh at this time.  Also, we should make this feature effective
only if CONFIG_CRASH_DUMP=y.  Otherwise, it makes no sense.

I'll prepare the bug fix patch.  Please wait a moment.

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

Re: kexec crash kernel running with watchdog enabled

[Vivek Goyal]

On Mon, Jun 29, 2015 at 06:53:29PM +0900, Hidehiro Kawai wrote:
> Hi,
> 
> (2015/06/27 3:33), dwalker@fifo99.com wrote:> On Fri, Jun 26, 2015 at 10:02:00AM +0900, Hidehiro Kawai wrote:
> >> Hi,
> >>
> >> (2015/06/25 1:31), dwalker@fifo99.com wrote:
> >>> On Wed, Jun 24, 2015 at 03:52:48PM +0900, Masami Hiramatsu wrote:
> >>>> Hi,
> >>>>
> >>>> On 2015/06/23 23:05, dwalker@fifo99.com wrote:
> >>>>>
> >>>>> Hi,
> >>>>>
> >>>>> There was a commit in kernel/panic.c which altered when the kexec crash kernel is executed,
> >>>>>
> >>>>> commit f06e5153f4ae2e2f3b0300f0e260e40cb7fefd45
> >>>>> Author: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> >>>>> Date:   Fri Jun 6 14:37:07 2014 -0700
> >>>>>
> >>>>>     kernel/panic.c: add "crash_kexec_post_notifiers" option for kdump after panic_notifers
> >>>>>
> >>>>>
> >>>>> This made it possible for smp_send_stop() to be executed prior to calling the kexec crash
> >>>>> kernel.
> >>>>>
> >>>>> The issue is that smp_send_stop() offlines the cores, and other code depend on the cores being online.
> >>>>>
> >>>>> In my case on Octeon here's an example,
> >>>>>
> >>>>> panic()
> >>>>>  crash_kexec()
> >>>>>   machine_crash_shutdown()
> >>>>>    octeon_generic_shutdown()
> >>>>>
> >>>>> Inside octeon_generic_shutdown() the Octeon watchdog is shutdown for_each_online_cpu(), but since
> >>>>> most of the cpu's already got offlined in smp_send_stop() it means the watchdog is still alive on
> >>>>> those cores. This results in a reboot during the crash kernel execution.
> >>>>
> >>>> Ah, I see.
> >>>>
> >>>>> Another example seem to be in default_machine_crash_shutdown() where crash_kexec_prepare_cpus() depends
> >>>>> on an IPI for saving the registers on different cores. However, the cpu's are all offlined with
> >>>>> interrupts disabled so they won't be running those IPI's in this case.
> >>>>>
> >>>>> I'm looking for any advice on how this should be fixed, or if it's already fixed. I'm not going to be
> >>>>> submitting a patch so if anyone wants to submit one feel free to do so.
> >>>>
> >>>> Hmm, IMHO, when the cpu goes to offline in appropriate way(smp_send_stop), it should stop
> >>>> watchdog timer on the offlined cpu too.
> >>>> Or, you can also register crash handler which stops all watchdogs, but it's a bit tricky.
> >>>>
> >>>
> >>> That doesn't really fix all the issue tho. As I was explaining generic MIPS code depends on the cpu's
> >>> effectively being online for crash data collection (with an IPI). This issue may effect other architectures also,
> >>> because smp_send_stop() offlines the cpu on other architectures also. I haven't surveyed the other architectures
> >>> enough to know what issue could happen from this tho.
> >>>
> >>> Is it possible to move the smp_send_stop() below the notifiers ? I'm just throwing out ideas.
> >>
> >> No, that doesn't works.  Some notifiers assume that they run in
> >> single core mode.
> >>
> >> Another possible solution is to add notifiers just after
> >> machine_crash_shutdown() like this:
> >>
> >> void panic(const char *fmt, ...)
> >> ...
> >> -	if (!crash_kexec_post_notifiers)
> >> -		crash_kexec(NULL);
> >> +	crash_kexec(NULL, buf);
> >>
> >>   and
> >>
> >> -void crash_kexec(struct pt_regs *regs)
> >> +void crash_kexec(struct pt_regs *regs, char *msg)
> >>  ...
> >>  		if (kexec_crash_image) {
> >>  			struct pt_regs fixed_regs;
> >>  
> >>  			crash_setup_regs(&fixed_regs, regs);
> >>  			crash_save_vmcoreinfo();
> >>  			machine_crash_shutdown(&fixed_regs);
> >> +			if (crash_kexec_post_notifiers) {
> >> +				kmsg_dump(KMSG_DUMP_PANIC);
> >> +				atomic_notifier_call_chain(&panic_notifier_list, 0, msg);
> >> +			}
> >>                         machine_kexec(kexec_crash_image);
> >>
> >> Most of archs stop other cores in machine_crash_shutdown(),
> >> so it will work well.  Furthermore, it simplifies the special
> >> case where crash_kexec() is called without entering panic().
> >>
> >> However, we need some tweaks for sh and s390 cases.  As for sh,
> >> it seems not to stop other cores in the crash_kexec() sequence
> >> (kdump support is incompleted?).  For s390, smp_send_stop() is
> >> called in machine_kexec() but not machine_crash_shutdown().
> > 
> > You could add an ifdef into the __setup() to filter out s390 and sh, until we figure out what
> > to do there. So the "crash_kexec_post_notifiers" wouldn't be available for those platforms.
> 
> I agree on disabling the "crash_kexec_post_notifiers" feature for
> s390 and sh at this time.  Also, we should make this feature effective
> only if CONFIG_CRASH_DUMP=y.  Otherwise, it makes no sense.
> 
> I'll prepare the bug fix patch.  Please wait a moment.

Why to add a patch for that. Just inform the user that don't need
crash_kexec_post_notifiers on s390 and sh?

Thanks
Vivek

Re: kexec crash kernel running with watchdog enabled

[Hidehiro Kawai]

(2015/06/29 21:26), Vivek Goyal wrote:
> On Mon, Jun 29, 2015 at 06:53:29PM +0900, Hidehiro Kawai wrote:
>> Hi,
>>
>> (2015/06/27 3:33), dwalker@fifo99.com wrote:
> On Fri, Jun 26, 2015 at 10:02:00AM +0900, Hidehiro Kawai wrote:
>>>> Hi,
>>>>
>>>> (2015/06/25 1:31), dwalker@fifo99.com wrote:
>>>>> On Wed, Jun 24, 2015 at 03:52:48PM +0900, Masami Hiramatsu wrote:
>>>>>> Hi,
>>>>>>
>>>>>> On 2015/06/23 23:05, dwalker@fifo99.com wrote:
>>>>>>>
>>>>>>> Hi,
>>>>>>>
>>>>>>> There was a commit in kernel/panic.c which altered when the kexec crash kernel is executed,
>>>>>>>
>>>>>>> commit f06e5153f4ae2e2f3b0300f0e260e40cb7fefd45
>>>>>>> Author: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
>>>>>>> Date:   Fri Jun 6 14:37:07 2014 -0700
>>>>>>>
>>>>>>>     kernel/panic.c: add "crash_kexec_post_notifiers" option for kdump after panic_notifers
>>>>>>>
>>>>>>>
>>>>>>> This made it possible for smp_send_stop() to be executed prior to calling the kexec crash
>>>>>>> kernel.
>>>>>>>
>>>>>>> The issue is that smp_send_stop() offlines the cores, and other code depend on the cores being online.
>>>>>>>
>>>>>>> In my case on Octeon here's an example,
>>>>>>>
>>>>>>> panic()
>>>>>>>  crash_kexec()
>>>>>>>   machine_crash_shutdown()
>>>>>>>    octeon_generic_shutdown()
>>>>>>>
>>>>>>> Inside octeon_generic_shutdown() the Octeon watchdog is shutdown for_each_online_cpu(), but since
>>>>>>> most of the cpu's already got offlined in smp_send_stop() it means the watchdog is still alive on
>>>>>>> those cores. This results in a reboot during the crash kernel execution.
>>>>>>
>>>>>> Ah, I see.
>>>>>>
>>>>>>> Another example seem to be in default_machine_crash_shutdown() where crash_kexec_prepare_cpus() depends
>>>>>>> on an IPI for saving the registers on different cores. However, the cpu's are all offlined with
>>>>>>> interrupts disabled so they won't be running those IPI's in this case.
>>>>>>>
>>>>>>> I'm looking for any advice on how this should be fixed, or if it's already fixed. I'm not going to be
>>>>>>> submitting a patch so if anyone wants to submit one feel free to do so.
>>>>>>
>>>>>> Hmm, IMHO, when the cpu goes to offline in appropriate way(smp_send_stop), it should stop
>>>>>> watchdog timer on the offlined cpu too.
>>>>>> Or, you can also register crash handler which stops all watchdogs, but it's a bit tricky.
>>>>>>
>>>>>
>>>>> That doesn't really fix all the issue tho. As I was explaining generic MIPS code depends on the cpu's
>>>>> effectively being online for crash data collection (with an IPI). This issue may effect other architectures also,
>>>>> because smp_send_stop() offlines the cpu on other architectures also. I haven't surveyed the other architectures
>>>>> enough to know what issue could happen from this tho.
>>>>>
>>>>> Is it possible to move the smp_send_stop() below the notifiers ? I'm just throwing out ideas.
>>>>
>>>> No, that doesn't works.  Some notifiers assume that they run in
>>>> single core mode.
>>>>
>>>> Another possible solution is to add notifiers just after
>>>> machine_crash_shutdown() like this:
>>>>
>>>> void panic(const char *fmt, ...)
>>>> ...
>>>> -	if (!crash_kexec_post_notifiers)
>>>> -		crash_kexec(NULL);
>>>> +	crash_kexec(NULL, buf);
>>>>
>>>>   and
>>>>
>>>> -void crash_kexec(struct pt_regs *regs)
>>>> +void crash_kexec(struct pt_regs *regs, char *msg)
>>>>  ...
>>>>  		if (kexec_crash_image) {
>>>>  			struct pt_regs fixed_regs;
>>>>  
>>>>  			crash_setup_regs(&fixed_regs, regs);
>>>>  			crash_save_vmcoreinfo();
>>>>  			machine_crash_shutdown(&fixed_regs);
>>>> +			if (crash_kexec_post_notifiers) {
>>>> +				kmsg_dump(KMSG_DUMP_PANIC);
>>>> +				atomic_notifier_call_chain(&panic_notifier_list, 0, msg);
>>>> +			}
>>>>                         machine_kexec(kexec_crash_image);
>>>>
>>>> Most of archs stop other cores in machine_crash_shutdown(),
>>>> so it will work well.  Furthermore, it simplifies the special
>>>> case where crash_kexec() is called without entering panic().
>>>>
>>>> However, we need some tweaks for sh and s390 cases.  As for sh,
>>>> it seems not to stop other cores in the crash_kexec() sequence
>>>> (kdump support is incompleted?).  For s390, smp_send_stop() is
>>>> called in machine_kexec() but not machine_crash_shutdown().
>>>
>>> You could add an ifdef into the __setup() to filter out s390 and sh, until we figure out what
>>> to do there. So the "crash_kexec_post_notifiers" wouldn't be available for those platforms.
>>
>> I agree on disabling the "crash_kexec_post_notifiers" feature for
>> s390 and sh at this time.  Also, we should make this feature effective
>> only if CONFIG_CRASH_DUMP=y.  Otherwise, it makes no sense.
>>
>> I'll prepare the bug fix patch.  Please wait a moment.
> 
> Why to add a patch for that. Just inform the user that don't need
> crash_kexec_post_notifiers on s390 and sh?

I thought it is relatively risky to run notifiers under other cores
being alive, but it's OK for me to just add the note to the source code
and document.

By the way, it turned out that sh doesn't support kdump with SMP, so
we don't need to stop other cores on sh.

Anyway, I'll send a patch which fixes a bug reported by Daniel, and
it will also fix a bug reported by Hatayama-san.

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