From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id A367EC43334 for ; Wed, 29 Jun 2022 11:55:45 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232916AbiF2Lzn (ORCPT ); Wed, 29 Jun 2022 07:55:43 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59158 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230326AbiF2Lzm (ORCPT ); Wed, 29 Jun 2022 07:55:42 -0400 Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 2427E387B1; Wed, 29 Jun 2022 04:55:41 -0700 (PDT) Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out1.suse.de (Postfix) with ESMTP id D6707220E0; Wed, 29 Jun 2022 11:55:39 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=susede1; t=1656503739; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=bF/LSSs2g8Q6JLHlya2E6zoiGTFvYRLz8XXwkmSkeI0=; b=GiHzf/Mi0x/V4my9jBprmV4ydlz1A8WxABnZ+n81Wxqgcgfa5dMtxOIA1EtJvo/rN3uJA2 lVEHsSW5xKN4UPsqhddeK8fDq9C8Ja+RTbdCzXAXQbd42plY8G+0Ag9wjXuWsqEC65fpBQ 7IkSyEbV5gRPYtEG9fw8bcHwqH1ITFM= Received: from suse.cz (pathway.suse.cz [10.100.12.24]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by relay2.suse.de (Postfix) with ESMTPS id A79142C141; Wed, 29 Jun 2022 11:55:39 +0000 (UTC) Date: Wed, 29 Jun 2022 13:55:39 +0200 From: Petr Mladek To: Valentin Schneider Cc: "Eric W. Biederman" , linux-kernel@vger.kernel.org, kexec@lists.infradead.org, linux-rt-users@vger.kernel.org, Arnd Bergmann , Thomas Gleixner , Sebastian Andrzej Siewior , Juri Lelli , "Luis Claudio R. Goncalves" , Andrew Morton , Vivek Goyal Subject: Re: [PATCH v2] panic, kexec: Make __crash_kexec() NMI safe Message-ID: <20220629115539.GB12720@pathway.suse.cz> References: <20220620111520.1039685-1-vschneid@redhat.com> <87r13c7jyp.fsf@email.froward.int.ebiederm.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.10.1 (2018-07-13) Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue 2022-06-28 18:33:08, Valentin Schneider wrote: > On 27/06/22 13:42, Valentin Schneider wrote: > > On 25/06/22 12:04, Eric W. Biederman wrote: > >> At this point I recommend going back to being ``unconventional'' with > >> the kexec locking and effectively reverting commit 8c5a1cf0ad3a ("kexec: > >> use a mutex for locking rather than xchg()"). > >> > >> That would also mean that we don't have to worry about the lockdep code > >> doing something weird in the future and breaking kexec. > >> > >> Your change starting to is atomic_cmpxchng is most halfway to a revert > >> of commit 8c5a1cf0ad3a ("kexec: use a mutex for locking rather than > >> xchg()"). So we might as well go the whole way and just document that > >> the kexec on panic code can not use conventional kernel locking > >> primitives and has to dig deep and build it's own. At which point it > >> makes no sense for the rest of the kexec code to use anything different. > >> > > > > Hm, I'm a bit torn about that one, ideally I'd prefer to keep "homegrown" > > locking primitives to just where they are needed (loading & kexec'ing), but > > I'm also not immensely fond of the "hybrid" mutex+cmpxchg approach. > > > > 8c5a1cf0ad3a ("kexec: use a mutex for locking rather than xchg()") was > straightforward enough because it turned > > if (xchg(&lock, 1)) > return -EBUSY; > > into > > if (!mutex_trylock(&lock)) > return -EBUSY; > > Now, most of the kexec_mutex uses are trylocks, except for: > - crash_get_memory_size() > - crash_shrink_memory() > > I really don't want to go down the route of turning those into cmpxchg > try-loops, would it be acceptable to make those use trylocks (i.e. return > -EBUSY if the cmpxchg fails)? IMHO, -EBUSY is acceptable for both crash_get_memory_size() and crash_shrink_memory(). They are used in the sysfs interface. > Otherwise, we keep the mutexes for functions like those which go nowhere > near an NMI. If we go this way then I would hide the locking into some wrappers, like crash_kexec_trylock()/unlock() that would do both mutex and xchg. The xchg part might be hidden in a separate wrapper __crash_kexec_trylock()/unlock() or crash_kexec_atomic_trylock()/unlock(). Best Regards, Petr From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 25C2EC433EF for ; Wed, 29 Jun 2022 11:55:54 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=xEQALPpdj9t3veASqitftq3+q0Y3B458pVCdBNSu62Q=; b=NGh+AIeqIfik+X i8Q8AMjoaRcG66isrq0PMX4iF3HMaXdJRqZ6F2kwRJUpYhd4LVAjFjgbNtpUX4wbmHYaexFF5AmNU ThcJ4uXk3oIGi4i99zGQ7lSTDRmzeHe446Dl6/u6P2XKlsSbH9n/U52WA8gWunMeGgNggTK1DeHiI qlN4vhZTOUon21VMDp3Q0SEjqPmlqfD6VVSDCyDddYOAvWpGkIEV6VXjlHwlBggfelRJFBZwNkugH ykXOgdeYH6Lcus8liEzoFNw6NzNDslHk5lJWBNCjD5ovG7cCxEFjGR2F/e0XnRWdLmYtxJnSHsvsG aUPAe701qxBHtGD8ZFbQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1o6WIO-00Bcde-Af; Wed, 29 Jun 2022 11:55:48 +0000 Received: from smtp-out1.suse.de ([195.135.220.28]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1o6WIL-00Bcaq-Na for kexec@lists.infradead.org; Wed, 29 Jun 2022 11:55:47 +0000 Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out1.suse.de (Postfix) with ESMTP id D6707220E0; Wed, 29 Jun 2022 11:55:39 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=susede1; t=1656503739; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=bF/LSSs2g8Q6JLHlya2E6zoiGTFvYRLz8XXwkmSkeI0=; b=GiHzf/Mi0x/V4my9jBprmV4ydlz1A8WxABnZ+n81Wxqgcgfa5dMtxOIA1EtJvo/rN3uJA2 lVEHsSW5xKN4UPsqhddeK8fDq9C8Ja+RTbdCzXAXQbd42plY8G+0Ag9wjXuWsqEC65fpBQ 7IkSyEbV5gRPYtEG9fw8bcHwqH1ITFM= Received: from suse.cz (pathway.suse.cz [10.100.12.24]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by relay2.suse.de (Postfix) with ESMTPS id A79142C141; Wed, 29 Jun 2022 11:55:39 +0000 (UTC) Date: Wed, 29 Jun 2022 13:55:39 +0200 From: Petr Mladek To: Valentin Schneider Cc: "Eric W. Biederman" , linux-kernel@vger.kernel.org, kexec@lists.infradead.org, linux-rt-users@vger.kernel.org, Arnd Bergmann , Thomas Gleixner , Sebastian Andrzej Siewior , Juri Lelli , "Luis Claudio R. Goncalves" , Andrew Morton , Vivek Goyal Subject: Re: [PATCH v2] panic, kexec: Make __crash_kexec() NMI safe Message-ID: <20220629115539.GB12720@pathway.suse.cz> References: <20220620111520.1039685-1-vschneid@redhat.com> <87r13c7jyp.fsf@email.froward.int.ebiederm.org> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.10.1 (2018-07-13) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220629_045545_989662_1452F5BA X-CRM114-Status: GOOD ( 26.40 ) X-BeenThere: kexec@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "kexec" Errors-To: kexec-bounces+kexec=archiver.kernel.org@lists.infradead.org On Tue 2022-06-28 18:33:08, Valentin Schneider wrote: > On 27/06/22 13:42, Valentin Schneider wrote: > > On 25/06/22 12:04, Eric W. Biederman wrote: > >> At this point I recommend going back to being ``unconventional'' with > >> the kexec locking and effectively reverting commit 8c5a1cf0ad3a ("kexec: > >> use a mutex for locking rather than xchg()"). > >> > >> That would also mean that we don't have to worry about the lockdep code > >> doing something weird in the future and breaking kexec. > >> > >> Your change starting to is atomic_cmpxchng is most halfway to a revert > >> of commit 8c5a1cf0ad3a ("kexec: use a mutex for locking rather than > >> xchg()"). So we might as well go the whole way and just document that > >> the kexec on panic code can not use conventional kernel locking > >> primitives and has to dig deep and build it's own. At which point it > >> makes no sense for the rest of the kexec code to use anything different. > >> > > > > Hm, I'm a bit torn about that one, ideally I'd prefer to keep "homegrown" > > locking primitives to just where they are needed (loading & kexec'ing), but > > I'm also not immensely fond of the "hybrid" mutex+cmpxchg approach. > > > > 8c5a1cf0ad3a ("kexec: use a mutex for locking rather than xchg()") was > straightforward enough because it turned > > if (xchg(&lock, 1)) > return -EBUSY; > > into > > if (!mutex_trylock(&lock)) > return -EBUSY; > > Now, most of the kexec_mutex uses are trylocks, except for: > - crash_get_memory_size() > - crash_shrink_memory() > > I really don't want to go down the route of turning those into cmpxchg > try-loops, would it be acceptable to make those use trylocks (i.e. return > -EBUSY if the cmpxchg fails)? IMHO, -EBUSY is acceptable for both crash_get_memory_size() and crash_shrink_memory(). They are used in the sysfs interface. > Otherwise, we keep the mutexes for functions like those which go nowhere > near an NMI. If we go this way then I would hide the locking into some wrappers, like crash_kexec_trylock()/unlock() that would do both mutex and xchg. The xchg part might be hidden in a separate wrapper __crash_kexec_trylock()/unlock() or crash_kexec_atomic_trylock()/unlock(). Best Regards, Petr _______________________________________________ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec