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 4DBA5C433EF for ; Tue, 10 May 2022 12:14:32 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S241610AbiEJMSX (ORCPT ); Tue, 10 May 2022 08:18:23 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:47148 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S241592AbiEJMSS (ORCPT ); Tue, 10 May 2022 08:18:18 -0400 Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id BBE1B244F2E; Tue, 10 May 2022 05:14:20 -0700 (PDT) Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out1.suse.de (Postfix) with ESMTP id 6756A21B7F; Tue, 10 May 2022 12:14:19 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=susede1; t=1652184859; 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=z0qCXGbgchboCgbitrJZObbFnXC/YqP7fhEXmnIw2vY=; b=OcxDFXmKKoVdDJUxSKREkD35h3cfh7L/UHzdGP/cbflejeipwrciY4AbOjnQu6DKNCs7aq Tea2QFXBj209QcyaHp7BuefH2IIusivsxr3vlgfi6p9lZaMFpVOppWn8lIP/+rweBZrcrm +Lwt8IUC7jK6VwI0ouYkElgPt01wPwE= Received: from suse.cz (unknown [10.100.208.146]) (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 552522C141; Tue, 10 May 2022 12:14:16 +0000 (UTC) Date: Tue, 10 May 2022 14:14:16 +0200 From: Petr Mladek To: "Guilherme G. Piccoli" Cc: akpm@linux-foundation.org, bhe@redhat.com, kexec@lists.infradead.org, linux-kernel@vger.kernel.org, bcm-kernel-feedback-list@broadcom.com, coresight@lists.linaro.org, linuxppc-dev@lists.ozlabs.org, linux-alpha@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-edac@vger.kernel.org, linux-hyperv@vger.kernel.org, linux-leds@vger.kernel.org, linux-mips@vger.kernel.org, linux-parisc@vger.kernel.org, linux-pm@vger.kernel.org, linux-remoteproc@vger.kernel.org, linux-s390@vger.kernel.org, linux-tegra@vger.kernel.org, linux-um@lists.infradead.org, linux-xtensa@linux-xtensa.org, netdev@vger.kernel.org, openipmi-developer@lists.sourceforge.net, rcu@vger.kernel.org, sparclinux@vger.kernel.org, xen-devel@lists.xenproject.org, x86@kernel.org, kernel-dev@igalia.com, kernel@gpiccoli.net, halves@canonical.com, fabiomirmar@gmail.com, alejandro.j.jimenez@oracle.com, andriy.shevchenko@linux.intel.com, arnd@arndb.de, bp@alien8.de, corbet@lwn.net, d.hatayama@jp.fujitsu.com, dave.hansen@linux.intel.com, dyoung@redhat.com, feng.tang@intel.com, gregkh@linuxfoundation.org, mikelley@microsoft.com, hidehiro.kawai.ez@hitachi.com, jgross@suse.com, john.ogness@linutronix.de, keescook@chromium.org, luto@kernel.org, mhiramat@kernel.org, mingo@redhat.com, paulmck@kernel.org, peterz@infradead.org, rostedt@goodmis.org, senozhatsky@chromium.org, stern@rowland.harvard.edu, tglx@linutronix.de, vgoyal@redhat.com, vkuznets@redhat.com, will@kernel.org, Christophe JAILLET , Mihai Carabas , Shile Zhang , Wang ShaoBo , zhenwei pi Subject: Re: [PATCH 05/30] misc/pvpanic: Convert regular spinlock into trylock on panic path Message-ID: References: <20220427224924.592546-1-gpiccoli@igalia.com> <20220427224924.592546-6-gpiccoli@igalia.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20220427224924.592546-6-gpiccoli@igalia.com> Precedence: bulk List-ID: X-Mailing-List: linux-remoteproc@vger.kernel.org On Wed 2022-04-27 19:48:59, Guilherme G. Piccoli wrote: > The pvpanic driver relies on panic notifiers to execute a callback > on panic event. Such function is executed in atomic context - the > panic function disables local IRQs, preemption and all other CPUs > that aren't running the panic code. > > With that said, it's dangerous to use regular spinlocks in such path, > as introduced by commit b3c0f8774668 ("misc/pvpanic: probe multiple instances"). > This patch fixes that by replacing regular spinlocks with the trylock > safer approach. It seems that the lock is used just to manipulating a list. A super safe solution would be to use the rcu API: rcu_add_rcu() and list_del_rcu() under rcu_read_lock(). The spin lock will not be needed and the list will always be valid. The advantage would be that it will always call members that were successfully added earlier. That said, I am not familiar with pvpanic and am not sure if it is worth it. > It also fixes an old comment (about a long gone framebuffer code) and > the notifier priority - we should execute hypervisor notifiers early, > deferring this way the panic action to the hypervisor, as expected by > the users that are setting up pvpanic. This should be done in a separate patch. It changes the behavior. Also there might be a discussion whether it really should be the maximal priority. 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 lists.ozlabs.org (lists.ozlabs.org [112.213.38.117]) (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 030ABC433F5 for ; Tue, 10 May 2022 22:38:16 +0000 (UTC) Received: from boromir.ozlabs.org (localhost [IPv6:::1]) by lists.ozlabs.org (Postfix) with ESMTP id 4KyXxg2k9Kz3cKM for ; Wed, 11 May 2022 08:38:15 +1000 (AEST) Authentication-Results: lists.ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=suse.com header.i=@suse.com header.a=rsa-sha256 header.s=susede1 header.b=OcxDFXmK; dkim-atps=neutral Authentication-Results: lists.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=suse.com (client-ip=195.135.220.29; helo=smtp-out2.suse.de; envelope-from=pmladek@suse.com; receiver=) Authentication-Results: lists.ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=suse.com header.i=@suse.com header.a=rsa-sha256 header.s=susede1 header.b=OcxDFXmK; dkim-atps=neutral Received: from smtp-out2.suse.de (smtp-out2.suse.de [195.135.220.29]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 4KyH5p5Qkrz3c94 for ; Tue, 10 May 2022 22:14:22 +1000 (AEST) Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out2.suse.de (Postfix) with ESMTP id 671FF1F8B8; Tue, 10 May 2022 12:14:19 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=susede1; t=1652184859; 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=z0qCXGbgchboCgbitrJZObbFnXC/YqP7fhEXmnIw2vY=; b=OcxDFXmKKoVdDJUxSKREkD35h3cfh7L/UHzdGP/cbflejeipwrciY4AbOjnQu6DKNCs7aq Tea2QFXBj209QcyaHp7BuefH2IIusivsxr3vlgfi6p9lZaMFpVOppWn8lIP/+rweBZrcrm +Lwt8IUC7jK6VwI0ouYkElgPt01wPwE= Received: from suse.cz (unknown [10.100.208.146]) (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 552522C141; Tue, 10 May 2022 12:14:16 +0000 (UTC) Date: Tue, 10 May 2022 14:14:16 +0200 From: Petr Mladek To: "Guilherme G. Piccoli" Subject: Re: [PATCH 05/30] misc/pvpanic: Convert regular spinlock into trylock on panic path Message-ID: References: <20220427224924.592546-1-gpiccoli@igalia.com> <20220427224924.592546-6-gpiccoli@igalia.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20220427224924.592546-6-gpiccoli@igalia.com> X-Mailman-Approved-At: Wed, 11 May 2022 08:37:45 +1000 X-BeenThere: linuxppc-dev@lists.ozlabs.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: linux-hyperv@vger.kernel.org, halves@canonical.com, gregkh@linuxfoundation.org, peterz@infradead.org, alejandro.j.jimenez@oracle.com, linux-remoteproc@vger.kernel.org, feng.tang@intel.com, linux-mips@vger.kernel.org, hidehiro.kawai.ez@hitachi.com, sparclinux@vger.kernel.org, will@kernel.org, tglx@linutronix.de, linux-leds@vger.kernel.org, linux-s390@vger.kernel.org, mikelley@microsoft.com, john.ogness@linutronix.de, bhe@redhat.com, corbet@lwn.net, paulmck@kernel.org, fabiomirmar@gmail.com, x86@kernel.org, zhenwei pi , mingo@redhat.com, bcm-kernel-feedback-list@broadcom.com, xen-devel@lists.xenproject.org, dyoung@redhat.com, vgoyal@redhat.com, linux-xtensa@linux-xtensa.org, dave.hansen@linux.intel.com, keescook@chromium.org, arnd@arndb.de, linux-pm@vger.kernel.org, Mihai Carabas , coresight@lists.linaro.org, Shile Zhang , linux-um@lists.infradead.org, rostedt@goodmis.org, rcu@vger.kernel.org, Wang ShaoBo , bp@alien8.de, luto@kernel.org, linux-tegra@vger.kernel.org, openipmi-developer@lists.sourceforge.net, andriy.shevchenko@linux.intel.com, vkuznets@redhat.com, linux-arm-kernel@lists.infradead.org, linux-edac@vger.kernel.org, jgross@suse.com, linux-parisc@vger.kernel.org, netdev@vger.kernel.org, kernel@gpiccoli.net, kexec@lists.infradead.org, linux-kernel@vger.kernel.org, stern@rowland.harvard.edu, senozhatsky@chromium.org, d.hatayama@jp.fujitsu.com, mhiramat@kernel.org, kernel-dev@igalia.com, linux-alpha@vger.kernel.org, Christophe JAILLET , akpm@linux-foundation.org, linuxppc-dev@lists.ozlabs.org Errors-To: linuxppc-dev-bounces+linuxppc-dev=archiver.kernel.org@lists.ozlabs.org Sender: "Linuxppc-dev" On Wed 2022-04-27 19:48:59, Guilherme G. Piccoli wrote: > The pvpanic driver relies on panic notifiers to execute a callback > on panic event. Such function is executed in atomic context - the > panic function disables local IRQs, preemption and all other CPUs > that aren't running the panic code. > > With that said, it's dangerous to use regular spinlocks in such path, > as introduced by commit b3c0f8774668 ("misc/pvpanic: probe multiple instances"). > This patch fixes that by replacing regular spinlocks with the trylock > safer approach. It seems that the lock is used just to manipulating a list. A super safe solution would be to use the rcu API: rcu_add_rcu() and list_del_rcu() under rcu_read_lock(). The spin lock will not be needed and the list will always be valid. The advantage would be that it will always call members that were successfully added earlier. That said, I am not familiar with pvpanic and am not sure if it is worth it. > It also fixes an old comment (about a long gone framebuffer code) and > the notifier priority - we should execute hypervisor notifiers early, > deferring this way the panic action to the hypervisor, as expected by > the users that are setting up pvpanic. This should be done in a separate patch. It changes the behavior. Also there might be a discussion whether it really should be the maximal priority. Best Regards, Petr From mboxrd@z Thu Jan 1 00:00:00 1970 From: Petr Mladek Date: Tue, 10 May 2022 14:14:16 +0200 Subject: [PATCH 05/30] misc/pvpanic: Convert regular spinlock into trylock on panic path In-Reply-To: <20220427224924.592546-6-gpiccoli@igalia.com> References: <20220427224924.592546-1-gpiccoli@igalia.com> <20220427224924.592546-6-gpiccoli@igalia.com> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: kexec@lists.infradead.org On Wed 2022-04-27 19:48:59, Guilherme G. Piccoli wrote: > The pvpanic driver relies on panic notifiers to execute a callback > on panic event. Such function is executed in atomic context - the > panic function disables local IRQs, preemption and all other CPUs > that aren't running the panic code. > > With that said, it's dangerous to use regular spinlocks in such path, > as introduced by commit b3c0f8774668 ("misc/pvpanic: probe multiple instances"). > This patch fixes that by replacing regular spinlocks with the trylock > safer approach. It seems that the lock is used just to manipulating a list. A super safe solution would be to use the rcu API: rcu_add_rcu() and list_del_rcu() under rcu_read_lock(). The spin lock will not be needed and the list will always be valid. The advantage would be that it will always call members that were successfully added earlier. That said, I am not familiar with pvpanic and am not sure if it is worth it. > It also fixes an old comment (about a long gone framebuffer code) and > the notifier priority - we should execute hypervisor notifiers early, > deferring this way the panic action to the hypervisor, as expected by > the users that are setting up pvpanic. This should be done in a separate patch. It changes the behavior. Also there might be a discussion whether it really should be the maximal priority. Best Regards, Petr From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Tue, 10 May 2022 14:14:16 +0200 From: Petr Mladek Subject: Re: [PATCH 05/30] misc/pvpanic: Convert regular spinlock into trylock on panic path Message-ID: References: <20220427224924.592546-1-gpiccoli@igalia.com> <20220427224924.592546-6-gpiccoli@igalia.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20220427224924.592546-6-gpiccoli@igalia.com> List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-um" Errors-To: linux-um-bounces+geert=linux-m68k.org@lists.infradead.org To: "Guilherme G. Piccoli" Cc: akpm@linux-foundation.org, bhe@redhat.com, kexec@lists.infradead.org, linux-kernel@vger.kernel.org, bcm-kernel-feedback-list@broadcom.com, coresight@lists.linaro.org, linuxppc-dev@lists.ozlabs.org, linux-alpha@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-edac@vger.kernel.org, linux-hyperv@vger.kernel.org, linux-leds@vger.kernel.org, linux-mips@vger.kernel.org, linux-parisc@vger.kernel.org, linux-pm@vger.kernel.org, linux-remoteproc@vger.kernel.org, linux-s390@vger.kernel.org, linux-tegra@vger.kernel.org, linux-um@lists.infradead.org, linux-xtensa@linux-xtensa.org, netdev@vger.kernel.org, openipmi-developer@lists.sourceforge.net, rcu@vger.kernel.org, sparclinux@vger.kernel.org, xen-devel@lists.xenproject.org, x86@kernel.org, kernel-dev@igalia.com, kernel@gpiccoli.net, halves@canonical.com, fabiomirmar@gmail.com, alejandro.j.jimenez@oracle.com, andriy.shevchenko@linux.intel.com, arnd@arndb.de, bp@alien8.de, corbet@lwn.net, d.hatayama@jp.fujitsu.com, dave.hansen@linux.intel.com, dyoung@redhat.com, feng.tang@intel.com, gregkh@linuxfoundation.org, mikelley@microsoft.com, hidehiro.kawai.ez@hitachi.com, jgross@suse.com, john.ogness@linutronix.de, keescook@chromium.org, luto@kernel.org, mhiramat@kernel.org, mingo@redhat.com, paulmck@kernel.org, peterz@infradead.org, rostedt@goodmis.org, senozhatsky@chromium.org, stern@rowland.harvard.edu, tglx@linutronix.de, vgoyal@redhat.com, vkuznets@redhat.com, will@kernel.org, Christophe JAILLET , Mihai Carabas , Shile Zhang , Wang ShaoBo , zhenwei pi On Wed 2022-04-27 19:48:59, Guilherme G. Piccoli wrote: > The pvpanic driver relies on panic notifiers to execute a callback > on panic event. Such function is executed in atomic context - the > panic function disables local IRQs, preemption and all other CPUs > that aren't running the panic code. > > With that said, it's dangerous to use regular spinlocks in such path, > as introduced by commit b3c0f8774668 ("misc/pvpanic: probe multiple instances"). > This patch fixes that by replacing regular spinlocks with the trylock > safer approach. It seems that the lock is used just to manipulating a list. A super safe solution would be to use the rcu API: rcu_add_rcu() and list_del_rcu() under rcu_read_lock(). The spin lock will not be needed and the list will always be valid. The advantage would be that it will always call members that were successfully added earlier. That said, I am not familiar with pvpanic and am not sure if it is worth it. > It also fixes an old comment (about a long gone framebuffer code) and > the notifier priority - we should execute hypervisor notifiers early, > deferring this way the panic action to the hypervisor, as expected by > the users that are setting up pvpanic. This should be done in a separate patch. It changes the behavior. Also there might be a discussion whether it really should be the maximal priority. Best Regards, Petr _______________________________________________ linux-um mailing list linux-um@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-um From mboxrd@z Thu Jan 1 00:00:00 1970 From: Petr Mladek Subject: Re: [PATCH 05/30] misc/pvpanic: Convert regular spinlock into trylock on panic path Date: Tue, 10 May 2022 14:14:16 +0200 Message-ID: References: <20220427224924.592546-1-gpiccoli@igalia.com> <20220427224924.592546-6-gpiccoli@igalia.com> Mime-Version: 1.0 Return-path: DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=susede1; t=1652184859; 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=z0qCXGbgchboCgbitrJZObbFnXC/YqP7fhEXmnIw2vY=; b=OcxDFXmKKoVdDJUxSKREkD35h3cfh7L/UHzdGP/cbflejeipwrciY4AbOjnQu6DKNCs7aq Tea2QFXBj209QcyaHp7BuefH2IIusivsxr3vlgfi6p9lZaMFpVOppWn8lIP/+rweBZrcrm +Lwt8IUC7jK6VwI0ouYkElgPt01wPwE= Content-Disposition: inline In-Reply-To: <20220427224924.592546-6-gpiccoli@igalia.com> List-ID: Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: "Guilherme G. Piccoli" Cc: akpm@linux-foundation.org, bhe@redhat.com, kexec@lists.infradead.org, linux-kernel@vger.kernel.org, bcm-kernel-feedback-list@broadcom.com, coresight@lists.linaro.org, linuxppc-dev@lists.ozlabs.org, linux-alpha@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-edac@vger.kernel.org, linux-hyperv@vger.kernel.org, linux-leds@vger.kernel.org, linux-mips@vger.kernel.org, linux-parisc@vger.kernel.org, linux-pm@vger.kernel.org, linux-remoteproc@vger.kernel.org, linux-s390@vger.kernel.org, linux-tegra@vger.kernel.org, linux-um@lists.infradead.org, linux-xtensa@linux-xtensa.org, netdev@vger.kernel.org, openipmi-developer@lists.sourceforge.net, rcu@vger.kernel.org, sparclinux@vger.kernel.org, xen-devel@lists.xenproject.org, x86@kernel.org, kernel-dev@igalia.com, kernel@gpiccoli.net On Wed 2022-04-27 19:48:59, Guilherme G. Piccoli wrote: > The pvpanic driver relies on panic notifiers to execute a callback > on panic event. Such function is executed in atomic context - the > panic function disables local IRQs, preemption and all other CPUs > that aren't running the panic code. > > With that said, it's dangerous to use regular spinlocks in such path, > as introduced by commit b3c0f8774668 ("misc/pvpanic: probe multiple instances"). > This patch fixes that by replacing regular spinlocks with the trylock > safer approach. It seems that the lock is used just to manipulating a list. A super safe solution would be to use the rcu API: rcu_add_rcu() and list_del_rcu() under rcu_read_lock(). The spin lock will not be needed and the list will always be valid. The advantage would be that it will always call members that were successfully added earlier. That said, I am not familiar with pvpanic and am not sure if it is worth it. > It also fixes an old comment (about a long gone framebuffer code) and > the notifier priority - we should execute hypervisor notifiers early, > deferring this way the panic action to the hypervisor, as expected by > the users that are setting up pvpanic. This should be done in a separate patch. It changes the behavior. Also there might be a discussion whether it really should be the maximal priority. Best Regards, Petr