Date:	Tue, 30 Jun 2015 17:36:06 +0900
From:	Hidehiro Kawai <hidehiro.kawai.ez@...achi.com>
To:	Vivek Goyal <vgoyal@...hat.com>
CC:	dwalker@...o99.com,
	Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>,
	Ralf Baechle <ralf@...ux-mips.org>, linux-mips@...ux-mips.org,
	david.daney@...ium.com, d.hatayama@...fujitsu.com,
	ebiederm@...ssion.com, linux-kernel@...r.kernel.org
Subject: Re: kexec crash kernel running with watchdog enabled

(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@...o99.com wrote:
> On Fri, Jun 26, 2015 at 10:02:00AM +0900, Hidehiro Kawai wrote:
>>>> Hi,
>>>>
>>>> (2015/06/25 1:31), dwalker@...o99.com wrote:
>>>>> On Wed, Jun 24, 2015 at 03:52:48PM +0900, Masami Hiramatsu wrote:
>>>>>> Hi,
>>>>>>
>>>>>> On 2015/06/23 23:05, dwalker@...o99.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@...achi.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


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists