From: "'Dave Young'" <dyoung@redhat.com>
To: "河合英宏 / KAWAI,HIDEHIRO" <hidehiro.kawai.ez@hitachi.com>
Cc: Michal Hocko <mhocko@suse.com>, Toshi Kani <toshi.kani@hpe.com>,
"Peter Zijlstra (Intel)" <peterz@infradead.org>,
Vitaly Kuznetsov <vkuznets@redhat.com>,
Minfei Huang <mnfhuang@gmail.com>,
"H. Peter Anvin" <hpa@zytor.com>,
Daniel Walker <dwalker@fifo99.com>,
Ingo Molnar <mingo@kernel.org>,
Takao Indoh <indou.takao@jp.fujitsu.com>,
Baoquan He <bhe@redhat.com>, "x86@kernel.org" <x86@kernel.org>,
"Lee, Chun-Yi" <joeyli.kernel@gmail.com>,
Borislav Petkov <bp@suse.de>, Vivek Goyal <vgoyal@redhat.com>,
Masami Hiramatsu <mhiramat@kernel.org>,
Petr Mladek <pmladek@suse.com>,
Josh Poimboeuf <jpoimboe@redhat.com>,
Thomas Gleixner <tglx@linutronix.de>,
Ingo Molnar <mingo@redhat.com>,
"kexec@lists.infradead.org" <kexec@lists.infradead.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
HATAYAMA Daisuke <d.hatayama@jp.fujitsu.com>,
"Eric W. Biederman" <ebiederm@xmission.com>,
Tejun Heo <tj@kernel.org>,
Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [V3 PATCH 1/2] x86/panic: Replace smp_send_stop() with kdump friendly version
Date: Tue, 19 Jul 2016 14:52:07 +0800 [thread overview]
Message-ID: <20160719065207.GF3326@dhcp-128-65.nay.redhat.com> (raw)
In-Reply-To: <04EAB7311EE43145B2D3536183D1A84454C83B25@GSjpTKYDCembx31.service.hitachi.net>
Hi,
On 07/19/16 at 05:51am, 河合英宏 / KAWAI,HIDEHIRO wrote:
> Hi,
>
> > From: 'Dave Young' [mailto:dyoung@redhat.com]
> > Sent: Monday, July 18, 2016 6:02 PM
> > On 07/15/16 at 11:50am, 河合英宏 / KAWAI,HIDEHIRO wrote:
> > > Hi Dave,
> > >
> > > Thanks for your reply.
> > >
> > > > From: 'Dave Young' [mailto:dyoung@redhat.com]
> > > > Sent: Wednesday, July 13, 2016 11:04 AM
> > > >
> > > > On 07/12/16 at 02:49am, 河合英宏 / KAWAI,HIDEHIRO wrote:
> > > > > Hi Dave,
> > > > >
> > > > > Thanks for the comments.
> > > > >
> > > > > > From: Dave Young [mailto:dyoung@redhat.com]
> > > > > > Sent: Monday, July 11, 2016 5:35 PM
> > > > > >
> > > > > > On 07/05/16 at 08:33pm, Hidehiro Kawai wrote:
> [snip]
> > > > > > As for this patch I'm not sure it is safe to replace the
> > > > > > smp_send_stop with the kdump friendly function. I'm also not sure if
> > > > > > the kdump friendly function is safe for kdump. Will glad to hear
> > > > > > opinions from other arch experts.
> > > > >
> > > > > This stuff depends on architectures, so I speak only about
> > > > > x86 (the logic doesn't change on other architectures at this time).
> > > > >
> > > > > kdump path with crash_kexec_post_notifiers disabled:
> > > > > panic()
> > > > > __crash_kexec()
> > > > > crash_setup_regs()
> > > > > crash_save_vmcoreinfo()
> > > > > machine_crash_shutdown()
> > > > > native_machine_crash_shutdown()
> > > > > panic_smp_send_stop() /* mostly same as original
> > > > > * kdump_nmi_shootdown_cpus()
> > > > > */
> > > > >
> > > > > kdump path with crash_kexec_post_notifiers enabled:
> > > > > panic()
> > > > > panic_smp_send_stop()
> > > > > __crash_kexec()
> > > > > crash_setup_regs()
> > > > > crash_save_vmcoreinfo()
> > > > > machine_crash_shutdown()
> > > > > native_machine_crash_shutdown()
> > > > > panic_smp_send_stop() // do nothing
> > > > >
> > > > > The difference is that stopping other CPUs before crash_setup_regs()
> > > > > and crash_save_vmcoreinfo() or not. Since crash_setup_regs() and
> > > > > crash_save_vmcoreinfo() just save information to some memory area,
> > > > > they wouldn't be affected by panic_smp_send_stop(). This means
> > > > > placing panic_smp_send_stop before __crash_kexec is safe.
> > > > >
> > > > > BTW, I noticed my patch breaks Xen kernel. I'll fix it in the next
> > > > > version.
> > > >
> > > > But it does breaks stuff which depends on cpu not being disabled like problem 1 you mentioned in patch log.
> > >
> > > As I mentioned in the description of this patch, we should stop
> > > other CPUs ASAP to preserve current state either
> > > crash_kexec_post_notifiers is enabled or not.
> > > Then, all remaining procedures should work well
> > > after stopping other CPUs (but keep the CPU map online).
> > >
> > > Vivek also mentioned similar things:
> > > https://lkml.org/lkml/2015/7/14/433
> >
> > The implementation in this patchset is different from suggestion in above link?
> >
> > I think Vivek's suggestion is a good idea, to drop smp_send_stop and do below:
> >
> > stop_cpus_save_register_state;
> >
> > if (!crash_kexec_post_notifiers)
> > crash_kexec()
> > atomic_notifier_call_chain()
> > kmsg_dump()
> >
> > I'm just commenting from code flow point of view, the detail implementation
> > definitely need more comments from Arch experts.
> >
> > Any reason did not move the kdump friendly function to earlier point like
> > before previous __crash_kexec() below?
> > if (!crash_kexec_post_notifiers) {
> > printk_nmi_flush_on_panic();
> > __crash_kexec(NULL);
> > }
>
> The reason why the implementation differs from Vivek's is to keep
> the current code flow if crash_kexec_post_notifiers is not specified.
>
> If we apply Vivek's or your suggestion, it may always cause kdump
> to fail on MIPS OCTEON due to Problem 1. I don't want to make things
> any worse. I may post a patch for MIPS OCTEON, but I can't test it.
> For other architectures, I'm not sure what problems there are.
> So at first, I want to fix the case where crash_kexec_post_notifiers is
> specified on x86. Then, if all other architectures support
> `stop other CPUs before crash_kexec', switch to your or Vivek's
> suggesting code.
>
> Is this acceptable?
Maybe you can find someone who can test MIPS OCTEON so that they can give
some thoughts first and maybe test a fix?
[dyoung@localhost linux]$ ./scripts/get_maintainer.pl -f arch/mips/cavium-octeon
Ralf Baechle <ralf@linux-mips.org> (supporter:MIPS,commit_signer:32/35=91%)
David Daney <david.daney@cavium.com> (commit_signer:21/35=60%,authored:8/35=23%)
Aaro Koskinen <aaro.koskinen@iki.fi> (commit_signer:15/35=43%,authored:8/35=23%)
Janne Huttunen <janne.huttunen@nokia.com>
(commit_signer:7/35=20%,authored:7/35=20%)
Thomas Gleixner <tglx@linutronix.de> (commit_signer:4/35=11%,authored:2/35=6%)
linux-mips@linux-mips.org (open list:MIPS)
linux-kernel@vger.kernel.org (open list)
Thanks
Dave
next prev parent reply other threads:[~2016-07-19 6:52 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-07-05 11:33 [V3 PATCH 0/2] kexec: crash_kexec_post_notifiers boot option related fixes Hidehiro Kawai
2016-07-05 11:33 ` [V3 PATCH 1/2] x86/panic: Replace smp_send_stop() with kdump friendly version Hidehiro Kawai
2016-07-11 8:34 ` Dave Young
2016-07-12 2:49 ` 河合英宏 / KAWAI,HIDEHIRO
2016-07-13 2:03 ` 'Dave Young'
2016-07-15 11:50 ` 河合英宏 / KAWAI,HIDEHIRO
2016-07-18 9:02 ` 'Dave Young'
2016-07-19 5:51 ` 河合英宏 / KAWAI,HIDEHIRO
2016-07-19 6:52 ` 'Dave Young' [this message]
2016-07-19 11:23 ` 河合英宏 / KAWAI,HIDEHIRO
2016-07-12 3:11 ` Xunlei Pang
2016-07-12 3:56 ` 河合英宏 / KAWAI,HIDEHIRO
2016-07-12 6:57 ` Xunlei Pang
2016-07-12 7:12 ` 河合英宏 / KAWAI,HIDEHIRO
2016-07-12 7:26 ` Xunlei Pang
2016-07-05 11:33 ` [V3 PATCH 2/2] kexec: Use core_param for crash_kexec_post_notifiers boot option Hidehiro Kawai
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160719065207.GF3326@dhcp-128-65.nay.redhat.com \
--to=dyoung@redhat.com \
--cc=akpm@linux-foundation.org \
--cc=bhe@redhat.com \
--cc=bp@suse.de \
--cc=d.hatayama@jp.fujitsu.com \
--cc=dwalker@fifo99.com \
--cc=ebiederm@xmission.com \
--cc=hidehiro.kawai.ez@hitachi.com \
--cc=hpa@zytor.com \
--cc=indou.takao@jp.fujitsu.com \
--cc=joeyli.kernel@gmail.com \
--cc=jpoimboe@redhat.com \
--cc=kexec@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mhiramat@kernel.org \
--cc=mhocko@suse.com \
--cc=mingo@kernel.org \
--cc=mingo@redhat.com \
--cc=mnfhuang@gmail.com \
--cc=peterz@infradead.org \
--cc=pmladek@suse.com \
--cc=tglx@linutronix.de \
--cc=tj@kernel.org \
--cc=toshi.kani@hpe.com \
--cc=vgoyal@redhat.com \
--cc=vkuznets@redhat.com \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).