From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Date: Tue, 25 Aug 2015 16:52:58 +0200 From: Peter Zijlstra Subject: Re: [V3 PATCH 3/4] kexec: Fix race between panic() and crash_kexec() called directly Message-ID: <20150825145258.GS16853@twins.programming.kicks-ass.net> References: <20150806054543.25766.29590.stgit@softrs> <20150806054543.25766.5914.stgit@softrs> <20150820230845.GF3161@worktop.event.rightround.com> <04EAB7311EE43145B2D3536183D1A8445493C868@GSjpTKYDCembx31.service.hitachi.net> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <04EAB7311EE43145B2D3536183D1A8445493C868@GSjpTKYDCembx31.service.hitachi.net> List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Sender: "kexec" Errors-To: kexec-bounces+dwmw2=infradead.org@lists.infradead.org To: =?utf-8?B?5rKz5ZCI6Iux5a6PIC8gS0FXQUnvvIxISURFSElSTw==?= Cc: "x86@kernel.org" , Jonathan Corbet , "linux-doc@vger.kernel.org" , "kexec@lists.infradead.org" , "linux-kernel@vger.kernel.org" , Michal Hocko , Thomas Gleixner , "Eric W. Biederman" , "H. Peter Anvin" , =?utf-8?B?5bmz5p2+6ZuF5bezIC8gSElSQU1BVFXvvIxNQVNBTUk=?= , Andrew Morton , Ingo Molnar , Vivek Goyal T24gU2F0LCBBdWcgMjIsIDIwMTUgYXQgMDI6MzU6MjRBTSArMDAwMCwg5rKz5ZCI6Iux5a6PIC8g S0FXQUnvvIxISURFSElSTyB3cm90ZToKPiA+IEZyb206IFBldGVyIFppamxzdHJhIFttYWlsdG86 cGV0ZXJ6QGluZnJhZGVhZC5vcmddCj4gPiAKPiA+IE9uIFRodSwgQXVnIDA2LCAyMDE1IGF0IDAy OjQ1OjQzUE0gKzA5MDAsIEhpZGVoaXJvIEthd2FpIHdyb3RlOgo+ID4gPiAgdm9pZCBjcmFzaF9r ZXhlYyhzdHJ1Y3QgcHRfcmVncyAqcmVncykKPiA+ID4gIHsKPiA+ID4gKwlpbnQgb2xkX2NwdSwg dGhpc19jcHU7Cj4gPiA+ICsKPiA+ID4gKwkvKgo+ID4gPiArCSAqIGBvbGRfY3B1ID09IC0xJyBt ZWFucyB3ZSBhcmUgdGhlIGZpcnN0IGNvbWVyIGFuZCBjcmFzaF9rZXhlYygpCj4gPiA+ICsJICog d2FzIGNhbGxlZCB3aXRob3V0IGVudGVyaW5nIHBhbmljKCkuCj4gPiA+ICsJICogYG9sZF9jcHUg PT0gdGhpc19jcHUnIG1lYW5zIGNyYXNoX2tleGVjKCkgd2FzIGNhbGxlZCBmcm9tIHBhbmljKCku Cj4gPiA+ICsJICovCj4gPiA+ICsJdGhpc19jcHUgPSByYXdfc21wX3Byb2Nlc3Nvcl9pZCgpOwo+ ID4gPiArCW9sZF9jcHUgPSBhdG9taWNfY21weGNoZygmcGFuaWNfY3B1LCAtMSwgdGhpc19jcHUp Owo+ID4gPiArCWlmIChvbGRfY3B1ICE9IC0xICYmIG9sZF9jcHUgIT0gdGhpc19jcHUpCj4gPiA+ ICsJCXJldHVybjsKPiA+IAo+ID4gVGhpcyBhbGxvd3MgcmVjdXJzaXZlIGNhbGxpbmcgb2YgY3Jh c2hfa2V4ZWMoKSwgdGhlIENoYW5nZWxvZyBkaWQgbm90Cj4gPiBtZW50aW9uIHRoYXQuIElzIHRo aXMgcmVhbGx5IHJlcXVpcmVkPwo+IAo+IFdoYXQgcGFydCBhcmUgeW91IGFyZ3Vpbmc/ICBSZWN1 cnNpdmUgY2FsbCBvZiBjcmFzaF9rZXhlYygpIGRvZXNuJ3QKPiBoYXBwZW4uICBJbiB0aGUgZmly c3QgcGxhY2UsIG9uZSBvZiB0aGUgcHVycG9zZSBvZiB0aGlzIHBhdGNoIGlzCj4gdG8gcHJldmVu dCBhIHJlY3Vyc2l2ZSBjYWxsIG9mIGNyYXNoX2tleGVjKCkgaW4gdGhlIGZvbGxvd2luZyBjYXNl Cj4gYXMgSSBzdGF0ZWQgaW4gdGhlIGRlc2NyaXB0aW9uOgo+IAo+IENQVSAwOgo+ICAgb29wc19l bmQoKQo+ICAgICBjcmFzaF9rZXhlYygpCj4gICAgICAgbXV0ZXhfdHJ5bG9jaygpIC8vIGFjcXVp cmVkCj4gICAgICAgICA8Tk1JPgo+ICAgICAgICAgaW9fY2hlY2tfZXJyb3IoKQo+ICAgICAgICAg ICBwYW5pYygpCj4gICAgICAgICAgICAgY3Jhc2hfa2V4ZWMoKQo+ICAgICAgICAgICAgICAgbXV0 ZXhfdHJ5bG9jaygpIC8vIGZhaWxlZCB0byBhY3F1aXJlCj4gICAgICAgICAgICAgaW5maW5pdGUg bG9vcAo+IAoKWWVzLCBidXQgd2hhdCB0byB3ZSB3YW50IHRvIGRvIHRoZXJlPyBJdCBzZWVtcyB0 byBtZSB0aGF0IGlzIHdyb25nLCB3ZQpkbyBub3Qgd2FudCB0byBsZXQgYSByZWN1cnNpdmUgY3Jh c2hfa2V4ZWMoKSBwcm9jZWVkLgoKV2hlcmVhcyB0aGUgY29uZGl0aW9uIHlvdSBjcmVhdGVkIGV4 cGxpY2l0bHkgYWxsb3dzIHRoaXMgcmVjdXJzaW9uIGJ5CnZpcnR1ZSBvZiB0aGUgJ29sZF9jcHUg IT0gdGhpc19jcHUnIGNoZWNrLgoKWW91IGNoYW5nZWxvZyBkb2VzIG5vdCBleHBsYWluIHdoeSB5 b3Ugd2FudCBhIHJlY3Vyc2l2ZSBjcmFzaF9rZXhlYygpLgoKPiBBbHNvLCB0aGUgbG9naWMgZG9l c24ndCBjaGFuZ2UgZm9ybSBWMSAoYWx0aG91Z2ggdGhlIGltcGxlbWVudGF0aW9uCj4gY2hhbmdl ZCksIHNvIEkgZGlkbid0IGFkZCBjaGFuZ2Vsb2dzIGFueSBtb3JlLgoKSSBjYW5ub3QgcmVtZW1i ZXIgVjEsIG5vciBjYW4gYW55IHByaW9yIHBhdGNoIGJlIHJlbGV2YW50LgoKCl9fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fCmtleGVjIG1haWxpbmcgbGlzdApr ZXhlY0BsaXN0cy5pbmZyYWRlYWQub3JnCmh0dHA6Ly9saXN0cy5pbmZyYWRlYWQub3JnL21haWxt YW4vbGlzdGluZm8va2V4ZWMK From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755666AbbHYOxL (ORCPT ); Tue, 25 Aug 2015 10:53:11 -0400 Received: from casper.infradead.org ([85.118.1.10]:34678 "EHLO casper.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753280AbbHYOxI (ORCPT ); Tue, 25 Aug 2015 10:53:08 -0400 Date: Tue, 25 Aug 2015 16:52:58 +0200 From: Peter Zijlstra To: =?utf-8?B?5rKz5ZCI6Iux5a6PIC8gS0FXQUnvvIxISURFSElSTw==?= 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 , =?utf-8?B?5bmz5p2+6ZuF5bezIC8gSElSQU1BVFXvvIxNQVNBTUk=?= Subject: Re: [V3 PATCH 3/4] kexec: Fix race between panic() and crash_kexec() called directly Message-ID: <20150825145258.GS16853@twins.programming.kicks-ass.net> References: <20150806054543.25766.29590.stgit@softrs> <20150806054543.25766.5914.stgit@softrs> <20150820230845.GF3161@worktop.event.rightround.com> <04EAB7311EE43145B2D3536183D1A8445493C868@GSjpTKYDCembx31.service.hitachi.net> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <04EAB7311EE43145B2D3536183D1A8445493C868@GSjpTKYDCembx31.service.hitachi.net> User-Agent: Mutt/1.5.21 (2012-12-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sat, Aug 22, 2015 at 02:35:24AM +0000, 河合英宏 / KAWAI，HIDEHIRO wrote: > > From: Peter Zijlstra [mailto:peterz@infradead.org] > > > > On Thu, Aug 06, 2015 at 02:45:43PM +0900, Hidehiro Kawai wrote: > > > void crash_kexec(struct pt_regs *regs) > > > { > > > + int old_cpu, this_cpu; > > > + > > > + /* > > > + * `old_cpu == -1' means we are the first comer and crash_kexec() > > > + * was called without entering panic(). > > > + * `old_cpu == this_cpu' means crash_kexec() was called from panic(). > > > + */ > > > + this_cpu = raw_smp_processor_id(); > > > + old_cpu = atomic_cmpxchg(&panic_cpu, -1, this_cpu); > > > + if (old_cpu != -1 && old_cpu != this_cpu) > > > + return; > > > > This allows recursive calling of crash_kexec(), the Changelog did not > > mention that. Is this really required? > > What part are you arguing? Recursive call of crash_kexec() doesn't > happen. In the first place, one of the purpose of this patch is > to prevent a recursive call of crash_kexec() in the following case > as I stated in the description: > > CPU 0: > oops_end() > crash_kexec() > mutex_trylock() // acquired > > io_check_error() > panic() > crash_kexec() > mutex_trylock() // failed to acquire > infinite loop > Yes, but what to we want to do there? It seems to me that is wrong, we do not want to let a recursive crash_kexec() proceed. Whereas the condition you created explicitly allows this recursion by virtue of the 'old_cpu != this_cpu' check. You changelog does not explain why you want a recursive crash_kexec(). > Also, the logic doesn't change form V1 (although the implementation > changed), so I didn't add changelogs any more. I cannot remember V1, nor can any prior patch be relevant.