All of lore.kernel.org
 help / color / mirror / Atom feed
From: "河合英宏 / KAWAI,HIDEHIRO" <hidehiro.kawai.ez@hitachi.com>
To: "'Borislav Petkov'" <bp@alien8.de>
Cc: "Jonathan Corbet" <corbet@lwn.net>,
	"Peter Zijlstra" <peterz@infradead.org>,
	"Ingo Molnar" <mingo@kernel.org>,
	"Eric W. Biederman" <ebiederm@xmission.com>,
	"H. Peter Anvin" <hpa@zytor.com>,
	"Andrew Morton" <akpm@linux-foundation.org>,
	"Thomas Gleixner" <tglx@linutronix.de>,
	"Vivek Goyal" <vgoyal@redhat.com>, "Baoquan He" <bhe@redhat.com>,
	"linux-doc@vger.kernel.org" <linux-doc@vger.kernel.org>,
	"x86@kernel.org" <x86@kernel.org>,
	"kexec@lists.infradead.org" <kexec@lists.infradead.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"Michal Hocko" <mhocko@kernel.org>,
	"平松雅巳 / HIRAMATU,MASAMI" <masami.hiramatsu.pt@hitachi.com>
Subject: RE: [V5 PATCH 3/4] kexec: Fix race between panic() and crash_kexec() called directly
Date: Thu, 3 Dec 2015 02:01:38 +0000	[thread overview]
Message-ID: <04EAB7311EE43145B2D3536183D1A84454A3CD95@GSjpTKYDCembx31.service.hitachi.net> (raw)
In-Reply-To: <20151202154023.GH3783@pd.tnic>

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1563 bytes --]

> On Wed, Dec 02, 2015 at 11:57:38AM +0000, 河合英宏 / KAWAI,HIDEHIRO wrote:
> > We can do so, but I think resetting panic_cpu always would be
> > simpler and safer.

I'll state in detail.

When we call crash_kexec() without entering panic() and return from
it, panic() should be called eventually.  But the code paths are
a bit complicated and there are many implementations for each
architecture.  So one day, this assumption may be broken; the CPU
doesn't call panic().  Or the CPU may fail to call panic() because
we are already in insane state.  It would be nervous, but allowing
another CPU to process panic routines by resetting panic_cpu
is safer approach.

> Well, I think executing code needlessly *especially* at panic time is
> not all that rosy either.
> 
> Besides something like this:
> 
> 	static bool kexec_failed;
> 
> 	...
> 
>         if (crash_kexec_post_notifiers && !kexec_failed)
> 		kexec_failed = __crash_kexec(NULL);
> 
> is as simple as it gets.

Since this code is executed only once due to panic_cpu,
I think introducing this logic is not much valuable.
Also, current implementation is already quite simple:

panic()
{
...
	__crash_kexec(NULL) {
		if (mutex_trylock(&kexec_mutex)) {
			if (kexec_crash_image) {
				/* don't return */
			}
		}
		mutex_unlock(&kexec_mutex)
	}

How do you think?

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


ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

WARNING: multiple messages have this Message-ID (diff)
From: "河合英宏 / KAWAI,HIDEHIRO" <hidehiro.kawai.ez@hitachi.com>
To: 'Borislav Petkov' <bp@alien8.de>
Cc: "x86@kernel.org" <x86@kernel.org>, "Baoquan He" <bhe@redhat.com>,
	"Jonathan Corbet" <corbet@lwn.net>,
	"Peter Zijlstra" <peterz@infradead.org>,
	"linux-doc@vger.kernel.org" <linux-doc@vger.kernel.org>,
	"kexec@lists.infradead.org" <kexec@lists.infradead.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"Michal Hocko" <mhocko@kernel.org>,
	"Thomas Gleixner" <tglx@linutronix.de>,
	"Eric W. Biederman" <ebiederm@xmission.com>,
	"H. Peter Anvin" <hpa@zytor.com>,
	"平松雅巳 / HIRAMATU,MASAMI" <masami.hiramatsu.pt@hitachi.com>,
	"Andrew Morton" <akpm@linux-foundation.org>,
	"Ingo Molnar" <mingo@kernel.org>,
	"Vivek Goyal" <vgoyal@redhat.com>
Subject: RE: [V5 PATCH 3/4] kexec: Fix race between panic() and crash_kexec() called directly
Date: Thu, 3 Dec 2015 02:01:38 +0000	[thread overview]
Message-ID: <04EAB7311EE43145B2D3536183D1A84454A3CD95@GSjpTKYDCembx31.service.hitachi.net> (raw)
In-Reply-To: <20151202154023.GH3783@pd.tnic>

> On Wed, Dec 02, 2015 at 11:57:38AM +0000, 河合英宏 / KAWAI,HIDEHIRO wrote:
> > We can do so, but I think resetting panic_cpu always would be
> > simpler and safer.

I'll state in detail.

When we call crash_kexec() without entering panic() and return from
it, panic() should be called eventually.  But the code paths are
a bit complicated and there are many implementations for each
architecture.  So one day, this assumption may be broken; the CPU
doesn't call panic().  Or the CPU may fail to call panic() because
we are already in insane state.  It would be nervous, but allowing
another CPU to process panic routines by resetting panic_cpu
is safer approach.

> Well, I think executing code needlessly *especially* at panic time is
> not all that rosy either.
> 
> Besides something like this:
> 
> 	static bool kexec_failed;
> 
> 	...
> 
>         if (crash_kexec_post_notifiers && !kexec_failed)
> 		kexec_failed = __crash_kexec(NULL);
> 
> is as simple as it gets.

Since this code is executed only once due to panic_cpu,
I think introducing this logic is not much valuable.
Also, current implementation is already quite simple:

panic()
{
...
	__crash_kexec(NULL) {
		if (mutex_trylock(&kexec_mutex)) {
			if (kexec_crash_image) {
				/* don't return */
			}
		}
		mutex_unlock(&kexec_mutex)
	}

How do you think?

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


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

  reply	other threads:[~2015-12-03  2:01 UTC|newest]

Thread overview: 70+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-11-20  9:36 [V5 PATCH 0/4] Fix race issues among panic, NMI and crash_kexec Hidehiro Kawai
2015-11-20  9:36 ` Hidehiro Kawai
2015-11-20  9:36 ` [V5 PATCH 1/4] panic/x86: Fix re-entrance problem due to panic on NMI Hidehiro Kawai
2015-11-20  9:36   ` Hidehiro Kawai
2015-11-23 18:49   ` Borislav Petkov
2015-11-23 18:49     ` Borislav Petkov
2015-11-24  4:06     ` 河合英宏 / KAWAI,HIDEHIRO
2015-11-24  4:06       ` 河合英宏 / KAWAI,HIDEHIRO
2015-11-24 12:45   ` Michal Hocko
2015-11-24 12:45     ` Michal Hocko
2015-11-24 15:05   ` Steven Rostedt
2015-11-24 15:05     ` Steven Rostedt
2015-11-24 15:12     ` Steven Rostedt
2015-11-24 15:12       ` Steven Rostedt
2015-11-24 20:27     ` Michal Hocko
2015-11-24 20:27       ` Michal Hocko
2015-11-24 20:45       ` Steven Rostedt
2015-11-24 20:45         ` Steven Rostedt
2015-11-20  9:36 ` [V5 PATCH 2/4] panic/x86: Allow cpus to save registers even if they are looping in NMI context Hidehiro Kawai
2015-11-20  9:36   ` Hidehiro Kawai
2015-11-24 10:48   ` Borislav Petkov
2015-11-24 10:48     ` Borislav Petkov
2015-11-24 19:37     ` Steven Rostedt
2015-11-24 19:37       ` Steven Rostedt
2015-11-24 20:16       ` Borislav Petkov
2015-11-24 20:16         ` Borislav Petkov
2015-11-25  5:57       ` 河合英宏 / KAWAI,HIDEHIRO
2015-11-25  5:57         ` 河合英宏 / KAWAI,HIDEHIRO
2015-11-25  5:51     ` 河合英宏 / KAWAI,HIDEHIRO
2015-11-25  5:51       ` 河合英宏 / KAWAI,HIDEHIRO
2015-11-25  8:56       ` Borislav Petkov
2015-11-25  8:56         ` Borislav Petkov
2015-11-25  9:46         ` 河合英宏 / KAWAI,HIDEHIRO
2015-11-25  9:46           ` 河合英宏 / KAWAI,HIDEHIRO
2015-11-25  9:57           ` Borislav Petkov
2015-11-25  9:57             ` Borislav Petkov
2015-11-25 15:11             ` 河合英宏 / KAWAI,HIDEHIRO
2015-11-25 15:11               ` 河合英宏 / KAWAI,HIDEHIRO
2015-11-24 12:58   ` Michal Hocko
2015-11-24 12:58     ` Michal Hocko
2015-12-03  2:23   ` 河合英宏 / KAWAI,HIDEHIRO
2015-12-03  2:23     ` 河合英宏 / KAWAI,HIDEHIRO
2015-11-20  9:36 ` [V5 PATCH 3/4] kexec: Fix race between panic() and crash_kexec() called directly Hidehiro Kawai
2015-11-20  9:36   ` Hidehiro Kawai
2015-11-24 13:05   ` Michal Hocko
2015-11-24 13:05     ` Michal Hocko
2015-11-24 20:35   ` Steven Rostedt
2015-11-24 20:35     ` Steven Rostedt
2015-11-25  6:28     ` 河合英宏 / KAWAI,HIDEHIRO
2015-11-25  6:28       ` 河合英宏 / KAWAI,HIDEHIRO
2015-11-25  9:54   ` Borislav Petkov
2015-11-25  9:54     ` Borislav Petkov
2015-12-02 11:57     ` 河合英宏 / KAWAI,HIDEHIRO
2015-12-02 11:57       ` 河合英宏 / KAWAI,HIDEHIRO
2015-12-02 15:40       ` Borislav Petkov
2015-12-02 15:40         ` Borislav Petkov
2015-12-03  2:01         ` 河合英宏 / KAWAI,HIDEHIRO [this message]
2015-12-03  2:01           ` 河合英宏 / KAWAI,HIDEHIRO
2015-12-03  9:35           ` Borislav Petkov
2015-12-03  9:35             ` Borislav Petkov
2015-12-03 11:29             ` 河合英宏 / KAWAI,HIDEHIRO
2015-12-03 11:29               ` 河合英宏 / KAWAI,HIDEHIRO
2015-12-03 12:22               ` Borislav Petkov
2015-12-03 12:22                 ` Borislav Petkov
2015-11-20  9:36 ` [V5 PATCH 4/4] x86/apic: Introduce apic_extnmi boot option Hidehiro Kawai
2015-11-20  9:36   ` Hidehiro Kawai
2015-11-25 11:49   ` Borislav Petkov
2015-11-25 11:49     ` Borislav Petkov
2015-11-25 15:29     ` 河合英宏 / KAWAI,HIDEHIRO
2015-11-25 15:29       ` 河合英宏 / KAWAI,HIDEHIRO

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=04EAB7311EE43145B2D3536183D1A84454A3CD95@GSjpTKYDCembx31.service.hitachi.net \
    --to=hidehiro.kawai.ez@hitachi.com \
    --cc=akpm@linux-foundation.org \
    --cc=bhe@redhat.com \
    --cc=bp@alien8.de \
    --cc=corbet@lwn.net \
    --cc=ebiederm@xmission.com \
    --cc=hpa@zytor.com \
    --cc=kexec@lists.infradead.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=masami.hiramatsu.pt@hitachi.com \
    --cc=mhocko@kernel.org \
    --cc=mingo@kernel.org \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    --cc=vgoyal@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.