linux-kernel - RE: Re: [V4 PATCH 2/2] mips/panic: Replace smp_send_stop() with kdump friendly version in panic path

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <04EAB7311EE43145B2D3536183D1A84454CBBB00@GSjpTKYDCembx31.service.hitachi.net>
Date:	Mon, 15 Aug 2016 11:35:59 +0000
From:	河合英宏 / KAWAI，HIDEHIRO 
	<hidehiro.kawai.ez@...achi.com>
To:	"'Corey Minyard'" <cminyard@...sta.com>,
	Dave Young <dyoung@...hat.com>
CC:	Andrew Morton <akpm@...ux-foundation.org>,
	"Eric W. Biederman" <ebiederm@...ssion.com>,
	Baoquan He <bhe@...hat.com>,
	Ralf Baechle <ralf@...ux-mips.org>,
	"x86@...nel.org" <x86@...nel.org>,
	David Daney <david.daney@...ium.com>,
	Xunlei Pang <xpang@...hat.com>,
	Aaro Koskinen <aaro.koskinen@....fi>,
	"kexec@...ts.infradead.org" <kexec@...ts.infradead.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	HATAYAMA Daisuke <d.hatayama@...fujitsu.com>,
	"linux-mips@...ux-mips.org" <linux-mips@...ux-mips.org>,
	Masami Hiramatsu <mhiramat@...nel.org>,
	"Steven J. Hill" <steven.hill@...ium.com>,
	"xen-devel@...ts.xenproject.org" <xen-devel@...ts.xenproject.org>,
	"Daniel Walker" <dwalker@...o99.com>,
	Vivek Goyal <vgoyal@...hat.com>
Subject: RE: Re: [V4 PATCH 2/2] mips/panic: Replace smp_send_stop() with
 kdump friendly version in panic path

Hi Corey,

> From: Corey Minyard [mailto:cminyard@...sta.com]
> Sent: Friday, August 12, 2016 10:56 PM
> I'll try to test this, but I have one comment inline...

Thank you very much!

> On 08/11/2016 10:17 PM, Dave Young wrote:
> > On 08/10/16 at 05:09pm, Hidehiro Kawai wrote:
[snip]
> >> diff --git a/arch/mips/kernel/crash.c b/arch/mips/kernel/crash.c
> >> index 610f0f3..1723b17 100644
> >> --- a/arch/mips/kernel/crash.c
> >> +++ b/arch/mips/kernel/crash.c
> >> @@ -47,9 +47,14 @@ static void crash_shutdown_secondary(void *passed_regs)
> >>
> >>   static void crash_kexec_prepare_cpus(void)
> >>   {
> >> +	static int cpus_stopped;
> >>   	unsigned int msecs;
> >> +	unsigned int ncpus;
> >>
> >> -	unsigned int ncpus = num_online_cpus() - 1;/* Excluding the panic cpu */
> >> +	if (cpus_stopped)
> >> +		return;
> 
> Wouldn't you want an atomic operation and some special handling here to
> ensure that only one CPU does this?  So if a CPU comes in here and
> another CPU is already in the process stopping the CPUs it won't result in a
> deadlock.

Because this function can be called only one panicking CPU,
there is no problem.

There are two paths which crash_kexec_prepare_cpus is called.

Path 1 (panic path):
panic()
  crash_smp_send_stop()
    crash_kexec_prepare_cpus()

Path 2 (oops path):
crash_kexec()
  __crash_kexec()
    machine_crash_shutdown()
      default_machine_crash_shutdown() // for MIPS
        crash_kexec_prepare_cpus()

Here, panic() and crash_kexec() run exclusively via
panic_cpu atomic variable.  So we can use cpus_stopped as
normal variable.

Best regards,

Hidehiro Kawai