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 766CAC4321E for ; Fri, 29 Apr 2022 18:05:51 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1379902AbiD2SJI (ORCPT ); Fri, 29 Apr 2022 14:09:08 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:49202 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1379882AbiD2SIz (ORCPT ); Fri, 29 Apr 2022 14:08:55 -0400 Received: from fanzine2.igalia.com (fanzine.igalia.com [178.60.130.6]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 9FC102DD61; Fri, 29 Apr 2022 11:05:33 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=igalia.com; s=20170329; h=Content-Transfer-Encoding:Content-Type:In-Reply-To:From: References:Cc:To:Subject:MIME-Version:Date:Message-ID:Sender:Reply-To: Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender: Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=CfgJSltNTETfm69VgWZeJ1xxMOJuVJJeVDaV1//hOM4=; b=slyIgXmeUq77MnrSUr5lBtJx9z m86cp15BT6gKICc3jCqWbY2B0tpa99u5bAed7DYYXL/rMf6XHJRMLSIrTGUcJCHpVtGl/+APLcmrh RXQpa9HZXWVydxhVU95+hp+ZiaA0uHFZXPx5zEc5qg8trUY5PT1GKrapnos8zeTpFOAoQJEAx9vvn efpqnWK3jhpJIbXy7DD12LOMiGnr6GupNgQCAiVUmjcrI2J6NMnifvEjcuVQUbIlOFxyQL8HlLyj5 ydadfwYuZkTNnyuPFseli5ffoVSkxD0MS5bvuilrJblEdywU2jKWaFEjPPnVXRJf3ScYrp3fr3KhT ij4JQKSA==; Received: from [179.113.53.197] (helo=[192.168.1.60]) by fanzine2.igalia.com with esmtpsa (Cipher TLS1.3:ECDHE_X25519__RSA_PSS_RSAE_SHA256__AES_128_GCM:128) (Exim) id 1nkUzF-0006RF-Du; Fri, 29 Apr 2022 20:05:01 +0200 Message-ID: <0147d038-571b-0802-c210-ccd4d52cd5dd@igalia.com> Date: Fri, 29 Apr 2022 15:04:22 -0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.8.0 Subject: Re: [PATCH 19/30] panic: Add the panic hypervisor notifier list Content-Language: en-US To: "Michael Kelley (LINUX)" , "akpm@linux-foundation.org" , "bhe@redhat.com" , "pmladek@suse.com" , "kexec@lists.infradead.org" Cc: "linux-kernel@vger.kernel.org" , "bcm-kernel-feedback-list@broadcom.com" , "linuxppc-dev@lists.ozlabs.org" , "linux-alpha@vger.kernel.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" , "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 , "will@kernel.org" , Alexander Gordeev , Andrea Parri , Ard Biesheuvel , Benjamin Herrenschmidt , Brian Norris , Christian Borntraeger , Christophe JAILLET , David Gow , "David S. Miller" , Dexuan Cui , Doug Berger , Evan Green , Florian Fainelli , Haiyang Zhang , Hari Bathini , Heiko Carstens , Julius Werner , Justin Chen , KY Srinivasan , Lee Jones , Markus Mayer , Michael Ellerman , Mihai Carabas , Nicholas Piggin , Paul Mackerras , Pavel Machek , Scott Branden , Sebastian Reichel , Shile Zhang , Stephen Hemminger , Sven Schnelle , Thomas Bogendoerfer , Tianyu Lan , Vasily Gorbik , Wang ShaoBo , Wei Liu , zhenwei pi References: <20220427224924.592546-1-gpiccoli@igalia.com> <20220427224924.592546-20-gpiccoli@igalia.com> From: "Guilherme G. Piccoli" In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-remoteproc@vger.kernel.org On 29/04/2022 14:30, Michael Kelley (LINUX) wrote: > From: Guilherme G. Piccoli Sent: Wednesday, April 27, 2022 3:49 PM >> [...] >> >> @@ -2843,7 +2843,7 @@ static void __exit vmbus_exit(void) >> if (ms_hyperv.misc_features & HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE) { >> kmsg_dump_unregister(&hv_kmsg_dumper); >> unregister_die_notifier(&hyperv_die_report_block); >> - atomic_notifier_chain_unregister(&panic_notifier_list, >> + atomic_notifier_chain_unregister(&panic_hypervisor_list, >> &hyperv_panic_report_block); >> } >> > > Using the hypervisor_list here produces a bit of a mismatch. In many cases > this notifier will do nothing, and will defer to the kmsg_dump() mechanism > to notify the hypervisor about the panic. Running the kmsg_dump() > mechanism is linked to the info_list, so I'm thinking the Hyper-V panic report > notifier should be on the info_list as well. That way the reporting behavior > is triggered at the same point in the panic path regardless of which > reporting mechanism is used. > Hi Michael, thanks for your feedback! I agree that your idea could work, but...there is one downside: imagine the kmsg_dump() approach is not set in some Hyper-V guest, then we would rely in the regular notification mechanism [hv_die_panic_notify_crash()], right? But...you want then to run this notifier in the informational list, which...won't execute *by default* before kdump if no kmsg_dump() is set. So, this logic is convoluted when you mix it with the default level concept + kdump. May I suggest something? If possible, take a run with this patch set + DEBUG_NOTIFIER=y, in *both* cases (with and without the kmsg_dump() set). I did that and they run almost at the same time...I've checked the notifiers called, it's like almost nothing runs in-between. I feel the panic notification mechanism does really fit with a hypervisor list, it's a good match with the nature of the list, which aims at informing the panic notification to the hypervisor/FW. Of course we can modify it if you prefer...but please take into account the kdump case and how it complicates the logic. Let me know your considerations, in case you can experiment with the patch set as-is. Cheers, Guilherme 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 B44F6C433EF for ; Fri, 29 Apr 2022 23:27:19 +0000 (UTC) Received: from boromir.ozlabs.org (localhost [IPv6:::1]) by lists.ozlabs.org (Postfix) with ESMTP id 4KqpYK4tPRz3bcv for ; Sat, 30 Apr 2022 09:27:17 +1000 (AEST) Authentication-Results: lists.ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=igalia.com header.i=@igalia.com header.a=rsa-sha256 header.s=20170329 header.b=slyIgXme; dkim-atps=neutral Authentication-Results: lists.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=igalia.com (client-ip=178.60.130.6; helo=fanzine2.igalia.com; envelope-from=gpiccoli@igalia.com; receiver=) Authentication-Results: lists.ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=igalia.com header.i=@igalia.com header.a=rsa-sha256 header.s=20170329 header.b=slyIgXme; dkim-atps=neutral Received: from fanzine2.igalia.com (fanzine.igalia.com [178.60.130.6]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (P-256) server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 4KqgQG3Z7Lz2xgJ for ; Sat, 30 Apr 2022 04:05:40 +1000 (AEST) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=igalia.com; s=20170329; h=Content-Transfer-Encoding:Content-Type:In-Reply-To:From: References:Cc:To:Subject:MIME-Version:Date:Message-ID:Sender:Reply-To: Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender: Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=CfgJSltNTETfm69VgWZeJ1xxMOJuVJJeVDaV1//hOM4=; b=slyIgXmeUq77MnrSUr5lBtJx9z m86cp15BT6gKICc3jCqWbY2B0tpa99u5bAed7DYYXL/rMf6XHJRMLSIrTGUcJCHpVtGl/+APLcmrh RXQpa9HZXWVydxhVU95+hp+ZiaA0uHFZXPx5zEc5qg8trUY5PT1GKrapnos8zeTpFOAoQJEAx9vvn efpqnWK3jhpJIbXy7DD12LOMiGnr6GupNgQCAiVUmjcrI2J6NMnifvEjcuVQUbIlOFxyQL8HlLyj5 ydadfwYuZkTNnyuPFseli5ffoVSkxD0MS5bvuilrJblEdywU2jKWaFEjPPnVXRJf3ScYrp3fr3KhT ij4JQKSA==; Received: from [179.113.53.197] (helo=[192.168.1.60]) by fanzine2.igalia.com with esmtpsa (Cipher TLS1.3:ECDHE_X25519__RSA_PSS_RSAE_SHA256__AES_128_GCM:128) (Exim) id 1nkUzF-0006RF-Du; Fri, 29 Apr 2022 20:05:01 +0200 Message-ID: <0147d038-571b-0802-c210-ccd4d52cd5dd@igalia.com> Date: Fri, 29 Apr 2022 15:04:22 -0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.8.0 Subject: Re: [PATCH 19/30] panic: Add the panic hypervisor notifier list Content-Language: en-US To: "Michael Kelley (LINUX)" , "akpm@linux-foundation.org" , "bhe@redhat.com" , "pmladek@suse.com" , "kexec@lists.infradead.org" References: <20220427224924.592546-1-gpiccoli@igalia.com> <20220427224924.592546-20-gpiccoli@igalia.com> From: "Guilherme G. Piccoli" In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Mailman-Approved-At: Sat, 30 Apr 2022 09:26:41 +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: Paul Mackerras , Justin Chen , Pavel Machek , Alexander Gordeev , KY Srinivasan , Wei Liu , "stern@rowland.harvard.edu" , "xen-devel@lists.xenproject.org" , Christian Borntraeger , "linux-pm@vger.kernel.org" , "linux-um@lists.infradead.org" , Nicholas Piggin , "luto@kernel.org" , Mihai Carabas , "tglx@linutronix.de" , "gregkh@linuxfoundation.org" , "linux-kernel@vger.kernel.org" , "senozhatsky@chromium.org" , "d.hatayama@jp.fujitsu.com" , Sven Schnelle , "linux-hyperv@vger.kernel.org" , "dave.hansen@linux.intel.com" , "linux-s390@vger.kernel.org" , Stephen Hemminger , "john.ogness@linutronix.de" , "vgoyal@redhat.com" , "mhiramat@kernel.org" , Andrea Parri , "linux-xtensa@linux-xtensa.org" , Vasily Gorbik , Scott Branden , Florian Fainelli , Markus Mayer , "hidehiro.kawai.ez@hitachi.com" , "kernel-dev@igalia.com" , "fabiomirmar@gmail.com" , "halves@canonical.com" , "alejandro.j.jimenez@oracle.com" , "feng.tang@intel.com" , zhenwei pi , "will@kernel.org" , Doug Berger , "corbet@lwn.net" , Dexuan Cui , Evan Green , "bcm-kernel-feedback-list@broadcom.com" , Tianyu Lan , "keescook@chromium.org" , "arnd@arndb.de" , Haiyang Zhang , "rostedt@goodmis.org" , "rcu@vger.kernel.org" , "bp@alien8.de" , "openipmi-developer@lists.sourceforge.net" , Thomas Bogendoerfer , "linux-parisc@vger.kernel.org" , Sebastian Reichel , "linux-alpha@vger.kernel.org" , Brian Norris , "David S. Miller" , "peterz@infradead.org" , "linux-remoteproc@vger.kernel.org" , "sparclinux@vger.kernel.org" , Lee Jones , Ard Biesheuvel , "linux-leds@vger.kernel.org" , "x86@kernel.org" , "mingo@redhat.com" , "dyoung@redhat.com" , "paulmck@kernel.org" , Heiko Carstens , Shile Zhang , Wang ShaoBo , Christophe JAILLET , David Gow , "linux-tegra@vger.kernel.org" , "andriy.shevchenko@linux.intel.com" , Hari Bathini , "linux-edac@vger.kernel.org" , "jgross@suse.com" , "netdev@vger.kernel.org" , "kernel@gpiccoli.net" , "linux-mips@vger.kernel.org" , Julius Werner , vkuznets , "linuxppc-dev@lists.ozlabs.org" Errors-To: linuxppc-dev-bounces+linuxppc-dev=archiver.kernel.org@lists.ozlabs.org Sender: "Linuxppc-dev" On 29/04/2022 14:30, Michael Kelley (LINUX) wrote: > From: Guilherme G. Piccoli Sent: Wednesday, April 27, 2022 3:49 PM >> [...] >> >> @@ -2843,7 +2843,7 @@ static void __exit vmbus_exit(void) >> if (ms_hyperv.misc_features & HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE) { >> kmsg_dump_unregister(&hv_kmsg_dumper); >> unregister_die_notifier(&hyperv_die_report_block); >> - atomic_notifier_chain_unregister(&panic_notifier_list, >> + atomic_notifier_chain_unregister(&panic_hypervisor_list, >> &hyperv_panic_report_block); >> } >> > > Using the hypervisor_list here produces a bit of a mismatch. In many cases > this notifier will do nothing, and will defer to the kmsg_dump() mechanism > to notify the hypervisor about the panic. Running the kmsg_dump() > mechanism is linked to the info_list, so I'm thinking the Hyper-V panic report > notifier should be on the info_list as well. That way the reporting behavior > is triggered at the same point in the panic path regardless of which > reporting mechanism is used. > Hi Michael, thanks for your feedback! I agree that your idea could work, but...there is one downside: imagine the kmsg_dump() approach is not set in some Hyper-V guest, then we would rely in the regular notification mechanism [hv_die_panic_notify_crash()], right? But...you want then to run this notifier in the informational list, which...won't execute *by default* before kdump if no kmsg_dump() is set. So, this logic is convoluted when you mix it with the default level concept + kdump. May I suggest something? If possible, take a run with this patch set + DEBUG_NOTIFIER=y, in *both* cases (with and without the kmsg_dump() set). I did that and they run almost at the same time...I've checked the notifiers called, it's like almost nothing runs in-between. I feel the panic notification mechanism does really fit with a hypervisor list, it's a good match with the nature of the list, which aims at informing the panic notification to the hypervisor/FW. Of course we can modify it if you prefer...but please take into account the kdump case and how it complicates the logic. Let me know your considerations, in case you can experiment with the patch set as-is. Cheers, Guilherme From mboxrd@z Thu Jan 1 00:00:00 1970 From: Guilherme G. Piccoli Date: Fri, 29 Apr 2022 15:04:22 -0300 Subject: [PATCH 19/30] panic: Add the panic hypervisor notifier list In-Reply-To: References: <20220427224924.592546-1-gpiccoli@igalia.com> <20220427224924.592546-20-gpiccoli@igalia.com> Message-ID: <0147d038-571b-0802-c210-ccd4d52cd5dd@igalia.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: kexec@lists.infradead.org On 29/04/2022 14:30, Michael Kelley (LINUX) wrote: > From: Guilherme G. Piccoli Sent: Wednesday, April 27, 2022 3:49 PM >> [...] >> >> @@ -2843,7 +2843,7 @@ static void __exit vmbus_exit(void) >> if (ms_hyperv.misc_features & HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE) { >> kmsg_dump_unregister(&hv_kmsg_dumper); >> unregister_die_notifier(&hyperv_die_report_block); >> - atomic_notifier_chain_unregister(&panic_notifier_list, >> + atomic_notifier_chain_unregister(&panic_hypervisor_list, >> &hyperv_panic_report_block); >> } >> > > Using the hypervisor_list here produces a bit of a mismatch. In many cases > this notifier will do nothing, and will defer to the kmsg_dump() mechanism > to notify the hypervisor about the panic. Running the kmsg_dump() > mechanism is linked to the info_list, so I'm thinking the Hyper-V panic report > notifier should be on the info_list as well. That way the reporting behavior > is triggered at the same point in the panic path regardless of which > reporting mechanism is used. > Hi Michael, thanks for your feedback! I agree that your idea could work, but...there is one downside: imagine the kmsg_dump() approach is not set in some Hyper-V guest, then we would rely in the regular notification mechanism [hv_die_panic_notify_crash()], right? But...you want then to run this notifier in the informational list, which...won't execute *by default* before kdump if no kmsg_dump() is set. So, this logic is convoluted when you mix it with the default level concept + kdump. May I suggest something? If possible, take a run with this patch set + DEBUG_NOTIFIER=y, in *both* cases (with and without the kmsg_dump() set). I did that and they run almost at the same time...I've checked the notifiers called, it's like almost nothing runs in-between. I feel the panic notification mechanism does really fit with a hypervisor list, it's a good match with the nature of the list, which aims at informing the panic notification to the hypervisor/FW. Of course we can modify it if you prefer...but please take into account the kdump case and how it complicates the logic. Let me know your considerations, in case you can experiment with the patch set as-is. Cheers, Guilherme From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Message-ID: <0147d038-571b-0802-c210-ccd4d52cd5dd@igalia.com> Date: Fri, 29 Apr 2022 15:04:22 -0300 MIME-Version: 1.0 Subject: Re: [PATCH 19/30] panic: Add the panic hypervisor notifier list Content-Language: en-US References: <20220427224924.592546-1-gpiccoli@igalia.com> <20220427224924.592546-20-gpiccoli@igalia.com> From: "Guilherme G. Piccoli" In-Reply-To: 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: "Michael Kelley (LINUX)" , "akpm@linux-foundation.org" , "bhe@redhat.com" , "pmladek@suse.com" , "kexec@lists.infradead.org" Cc: "linux-kernel@vger.kernel.org" , "bcm-kernel-feedback-list@broadcom.com" , "linuxppc-dev@lists.ozlabs.org" , "linux-alpha@vger.kernel.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" , "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 , "will@kernel.org" , Alexander Gordeev , Andrea Parri , Ard Biesheuvel , Benjamin Herrenschmidt , Brian Norris , Christian Borntraeger , Christophe JAILLET , David Gow , "David S. Miller" , Dexuan Cui , Doug Berger , Evan Green , Florian Fainelli , Haiyang Zhang , Hari Bathini , Heiko Carstens , Julius Werner , Justin Chen , KY Srinivasan , Lee Jones , Markus Mayer , Michael Ellerman , Mihai Carabas , Nicholas Piggin , Paul Mackerras , Pavel Machek , Scott Branden , Sebastian Reichel , Shile Zhang , Stephen Hemminger , Sven Schnelle , Thomas Bogendoerfer , Tianyu Lan , Vasily Gorbik , Wang ShaoBo , Wei Liu , zhenwei pi On 29/04/2022 14:30, Michael Kelley (LINUX) wrote: > From: Guilherme G. Piccoli Sent: Wednesday, April 27, 2022 3:49 PM >> [...] >> >> @@ -2843,7 +2843,7 @@ static void __exit vmbus_exit(void) >> if (ms_hyperv.misc_features & HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE) { >> kmsg_dump_unregister(&hv_kmsg_dumper); >> unregister_die_notifier(&hyperv_die_report_block); >> - atomic_notifier_chain_unregister(&panic_notifier_list, >> + atomic_notifier_chain_unregister(&panic_hypervisor_list, >> &hyperv_panic_report_block); >> } >> > > Using the hypervisor_list here produces a bit of a mismatch. In many cases > this notifier will do nothing, and will defer to the kmsg_dump() mechanism > to notify the hypervisor about the panic. Running the kmsg_dump() > mechanism is linked to the info_list, so I'm thinking the Hyper-V panic report > notifier should be on the info_list as well. That way the reporting behavior > is triggered at the same point in the panic path regardless of which > reporting mechanism is used. > Hi Michael, thanks for your feedback! I agree that your idea could work, but...there is one downside: imagine the kmsg_dump() approach is not set in some Hyper-V guest, then we would rely in the regular notification mechanism [hv_die_panic_notify_crash()], right? But...you want then to run this notifier in the informational list, which...won't execute *by default* before kdump if no kmsg_dump() is set. So, this logic is convoluted when you mix it with the default level concept + kdump. May I suggest something? If possible, take a run with this patch set + DEBUG_NOTIFIER=y, in *both* cases (with and without the kmsg_dump() set). I did that and they run almost at the same time...I've checked the notifiers called, it's like almost nothing runs in-between. I feel the panic notification mechanism does really fit with a hypervisor list, it's a good match with the nature of the list, which aims at informing the panic notification to the hypervisor/FW. Of course we can modify it if you prefer...but please take into account the kdump case and how it complicates the logic. Let me know your considerations, in case you can experiment with the patch set as-is. Cheers, Guilherme _______________________________________________ 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: "Guilherme G. Piccoli" Subject: Re: [PATCH 19/30] panic: Add the panic hypervisor notifier list Date: Fri, 29 Apr 2022 15:04:22 -0300 Message-ID: <0147d038-571b-0802-c210-ccd4d52cd5dd@igalia.com> References: <20220427224924.592546-1-gpiccoli@igalia.com> <20220427224924.592546-20-gpiccoli@igalia.com> Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Return-path: DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=igalia.com; s=20170329; h=Content-Transfer-Encoding:Content-Type:In-Reply-To:From: References:Cc:To:Subject:MIME-Version:Date:Message-ID:Sender:Reply-To: Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender: Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=CfgJSltNTETfm69VgWZeJ1xxMOJuVJJeVDaV1//hOM4=; b=slyIgXmeUq77MnrSUr5lBtJx9z m86cp15BT6gKICc3jCqWbY2B0tpa99u5bAed7DYYXL/rMf6XHJRMLSIrTGUcJCHpVtGl/+APLcmrh RXQpa9HZXWVydxhVU95+hp+ZiaA0uHFZXPx5zEc5qg8trUY5PT1GKrapnos8zeTpFOAoQJEAx9vvn efpqnWK3jhpJIbXy7DD12LOMiGnr6GupNgQCAiVUmjcrI2J6NMnifvEjcuVQUbIlOFxyQL8HlLyj5 ydadfwYuZkTNnyuPFseli5ffoVSkxD0MS5bvuilrJblEdywU2jKWaFEjPPnVXRJf3ScYrp3fr3KhT ij4JQKS Content-Language: en-US In-Reply-To: List-ID: Content-Type: text/plain; charset="us-ascii" To: "Michael Kelley (LINUX)" , "akpm@linux-foundation.org" , "bhe@redhat.com" , "pmladek@suse.com" , "kexec@lists.infradead.org" Cc: "linux-kernel@vger.kernel.org" , "bcm-kernel-feedback-list@broadcom.com" , "linuxppc-dev@lists.ozlabs.org" , "linux-alpha@vger.kernel.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"
  • On 29/04/2022 14:30, Michael Kelley (LINUX) wrote: > From: Guilherme G. Piccoli Sent: Wednesday, April 27, 2022 3:49 PM >> [...] >> >> @@ -2843,7 +2843,7 @@ static void __exit vmbus_exit(void) >> if (ms_hyperv.misc_features & HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE) { >> kmsg_dump_unregister(&hv_kmsg_dumper); >> unregister_die_notifier(&hyperv_die_report_block); >> - atomic_notifier_chain_unregister(&panic_notifier_list, >> + atomic_notifier_chain_unregister(&panic_hypervisor_list, >> &hyperv_panic_report_block); >> } >> > > Using the hypervisor_list here produces a bit of a mismatch. In many cases > this notifier will do nothing, and will defer to the kmsg_dump() mechanism > to notify the hypervisor about the panic. Running the kmsg_dump() > mechanism is linked to the info_list, so I'm thinking the Hyper-V panic report > notifier should be on the info_list as well. That way the reporting behavior > is triggered at the same point in the panic path regardless of which > reporting mechanism is used. > Hi Michael, thanks for your feedback! I agree that your idea could work, but...there is one downside: imagine the kmsg_dump() approach is not set in some Hyper-V guest, then we would rely in the regular notification mechanism [hv_die_panic_notify_crash()], right? But...you want then to run this notifier in the informational list, which...won't execute *by default* before kdump if no kmsg_dump() is set. So, this logic is convoluted when you mix it with the default level concept + kdump. May I suggest something? If possible, take a run with this patch set + DEBUG_NOTIFIER=y, in *both* cases (with and without the kmsg_dump() set). I did that and they run almost at the same time...I've checked the notifiers called, it's like almost nothing runs in-between. I feel the panic notification mechanism does really fit with a hypervisor list, it's a good match with the nature of the list, which aims at informing the panic notification to the hypervisor/FW. Of course we can modify it if you prefer...but please take into account the kdump case and how it complicates the logic. Let me know your considerations, in case you can experiment with the patch set as-is. Cheers, Guilherme