From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755994AbbI3Luf (ORCPT ); Wed, 30 Sep 2015 07:50:35 -0400 Received: from bombadil.infradead.org ([198.137.202.9]:53346 "EHLO bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751541AbbI3Lud (ORCPT ); Wed, 30 Sep 2015 07:50:33 -0400 Date: Wed, 30 Sep 2015 13:50:25 +0200 From: Peter Zijlstra To: Hidehiro Kawai Cc: Jonathan Corbet , Ingo Molnar , "Eric W. Biederman" , "H. Peter Anvin" , Andrew Morton , Thomas Gleixner , Vivek Goyal , linux-doc@vger.kernel.org, x86@kernel.org, kexec@lists.infradead.org, linux-kernel@vger.kernel.org, Michal Hocko , Ingo Molnar , Masami Hiramatsu Subject: Re: [V4 PATCH 2/4] panic/x86: Allow cpus to save registers even if they are looping in NMI context Message-ID: <20150930115025.GG2881@worktop.programming.kicks-ass.net> References: <20150925112803.4258.94241.stgit@softrs> <20150925112807.4258.12180.stgit@softrs> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150925112807.4258.12180.stgit@softrs> User-Agent: Mutt/1.5.22.1 (2013-10-16) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Sep 25, 2015 at 08:28:07PM +0900, Hidehiro Kawai wrote: > --- a/arch/x86/kernel/reboot.c > +++ b/arch/x86/kernel/reboot.c > @@ -718,6 +718,7 @@ void machine_crash_shutdown(struct pt_regs *regs) > static nmi_shootdown_cb shootdown_callback; > > static atomic_t waiting_for_crash_ipi; > +static int crash_ipi_done; > > static int crash_nmi_callback(unsigned int val, struct pt_regs *regs) > { > @@ -779,6 +780,7 @@ void nmi_shootdown_cpus(nmi_shootdown_cb callback) > wmb(); > > smp_send_nmi_allbutself(); > + crash_ipi_done = 1; /* Kick cpus looping in nmi context */ I would suggest using WRITE_ONCE() for that, because without the volatile the compiler need not actually emit the store until after the whole waiting thing _IF_ it can inline the whole thing. Currently udelay() will end up being a function call and will therefore force the store to be emitted, but I'd rather not rely on that. > > msecs = 1000; /* Wait at most a second for the other cpus to stop */ > while ((atomic_read(&waiting_for_crash_ipi) > 0) && msecs) {