[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <PH0PR21MB30257E4E6E16BB8FFDE8F312D7C09@PH0PR21MB3025.namprd21.prod.outlook.com>
Date: Tue, 3 May 2022 17:44:03 +0000
From: "Michael Kelley (LINUX)" <mikelley@...rosoft.com>
To: "Guilherme G. Piccoli" <gpiccoli@...lia.com>,
"akpm@...ux-foundation.org" <akpm@...ux-foundation.org>,
"bhe@...hat.com" <bhe@...hat.com>,
"pmladek@...e.com" <pmladek@...e.com>,
"kexec@...ts.infradead.org" <kexec@...ts.infradead.org>
CC: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"bcm-kernel-feedback-list@...adcom.com"
<bcm-kernel-feedback-list@...adcom.com>,
"linuxppc-dev@...ts.ozlabs.org" <linuxppc-dev@...ts.ozlabs.org>,
"linux-alpha@...r.kernel.org" <linux-alpha@...r.kernel.org>,
"linux-edac@...r.kernel.org" <linux-edac@...r.kernel.org>,
"linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>,
"linux-leds@...r.kernel.org" <linux-leds@...r.kernel.org>,
"linux-mips@...r.kernel.org" <linux-mips@...r.kernel.org>,
"linux-parisc@...r.kernel.org" <linux-parisc@...r.kernel.org>,
"linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>,
"linux-remoteproc@...r.kernel.org" <linux-remoteproc@...r.kernel.org>,
"linux-s390@...r.kernel.org" <linux-s390@...r.kernel.org>,
"linux-tegra@...r.kernel.org" <linux-tegra@...r.kernel.org>,
"linux-um@...ts.infradead.org" <linux-um@...ts.infradead.org>,
"linux-xtensa@...ux-xtensa.org" <linux-xtensa@...ux-xtensa.org>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
"openipmi-developer@...ts.sourceforge.net"
<openipmi-developer@...ts.sourceforge.net>,
"rcu@...r.kernel.org" <rcu@...r.kernel.org>,
"sparclinux@...r.kernel.org" <sparclinux@...r.kernel.org>,
"xen-devel@...ts.xenproject.org" <xen-devel@...ts.xenproject.org>,
"x86@...nel.org" <x86@...nel.org>,
"kernel-dev@...lia.com" <kernel-dev@...lia.com>,
"kernel@...ccoli.net" <kernel@...ccoli.net>,
"halves@...onical.com" <halves@...onical.com>,
"fabiomirmar@...il.com" <fabiomirmar@...il.com>,
"alejandro.j.jimenez@...cle.com" <alejandro.j.jimenez@...cle.com>,
"andriy.shevchenko@...ux.intel.com"
<andriy.shevchenko@...ux.intel.com>,
"arnd@...db.de" <arnd@...db.de>, "bp@...en8.de" <bp@...en8.de>,
"corbet@....net" <corbet@....net>,
"d.hatayama@...fujitsu.com" <d.hatayama@...fujitsu.com>,
"dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
"dyoung@...hat.com" <dyoung@...hat.com>,
"feng.tang@...el.com" <feng.tang@...el.com>,
"gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
"hidehiro.kawai.ez@...achi.com" <hidehiro.kawai.ez@...achi.com>,
"jgross@...e.com" <jgross@...e.com>,
"john.ogness@...utronix.de" <john.ogness@...utronix.de>,
"keescook@...omium.org" <keescook@...omium.org>,
"luto@...nel.org" <luto@...nel.org>,
"mhiramat@...nel.org" <mhiramat@...nel.org>,
"mingo@...hat.com" <mingo@...hat.com>,
"paulmck@...nel.org" <paulmck@...nel.org>,
"peterz@...radead.org" <peterz@...radead.org>,
"rostedt@...dmis.org" <rostedt@...dmis.org>,
"senozhatsky@...omium.org" <senozhatsky@...omium.org>,
"stern@...land.harvard.edu" <stern@...land.harvard.edu>,
"tglx@...utronix.de" <tglx@...utronix.de>,
"vgoyal@...hat.com" <vgoyal@...hat.com>,
vkuznets <vkuznets@...hat.com>,
"will@...nel.org" <will@...nel.org>,
Alexander Gordeev <agordeev@...ux.ibm.com>,
Andrea Parri <parri.andrea@...il.com>,
Ard Biesheuvel <ardb@...nel.org>,
Benjamin Herrenschmidt <benh@...nel.crashing.org>,
Brian Norris <computersforpeace@...il.com>,
Christian Borntraeger <borntraeger@...ux.ibm.com>,
Christophe JAILLET <christophe.jaillet@...adoo.fr>,
David Gow <davidgow@...gle.com>,
"David S. Miller" <davem@...emloft.net>,
Dexuan Cui <decui@...rosoft.com>,
Doug Berger <opendmb@...il.com>,
Evan Green <evgreen@...omium.org>,
Florian Fainelli <f.fainelli@...il.com>,
Haiyang Zhang <haiyangz@...rosoft.com>,
Hari Bathini <hbathini@...ux.ibm.com>,
Heiko Carstens <hca@...ux.ibm.com>,
Julius Werner <jwerner@...omium.org>,
Justin Chen <justinpopo6@...il.com>,
KY Srinivasan <kys@...rosoft.com>,
Lee Jones <lee.jones@...aro.org>,
Markus Mayer <mmayer@...adcom.com>,
Michael Ellerman <mpe@...erman.id.au>,
Mihai Carabas <mihai.carabas@...cle.com>,
Nicholas Piggin <npiggin@...il.com>,
Paul Mackerras <paulus@...ba.org>, Pavel Machek <pavel@....cz>,
Scott Branden <scott.branden@...adcom.com>,
Sebastian Reichel <sre@...nel.org>,
Shile Zhang <shile.zhang@...ux.alibaba.com>,
Stephen Hemminger <sthemmin@...rosoft.com>,
Sven Schnelle <svens@...ux.ibm.com>,
Thomas Bogendoerfer <tsbogend@...ha.franken.de>,
Tianyu Lan <Tianyu.Lan@...rosoft.com>,
Vasily Gorbik <gor@...ux.ibm.com>,
Wang ShaoBo <bobo.shaobowang@...wei.com>,
Wei Liu <wei.liu@...nel.org>,
zhenwei pi <pizhenwei@...edance.com>
Subject: RE: [PATCH 19/30] panic: Add the panic hypervisor notifier list
From: Guilherme G. Piccoli <gpiccoli@...lia.com> Sent: Friday, April 29, 2022 11:04 AM
>
> On 29/04/2022 14:30, Michael Kelley (LINUX) wrote:
> > From: Guilherme G. Piccoli <gpiccoli@...lia.com> 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.
Yes, you are right. But to me that speaks as much to the linkage
between the informational list and kmsg_dump() being the core
problem. But as I described in my reply to Patch 24, I can live with
the linkage as-is.
FWIW, guests on newer versions of Hyper-V will always register a
kmsg dumper. The flags that are tested to decide whether to
register provide compatibility with older versions of Hyper-V that
don’t support the 4K bytes of notification info.
>
> 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.
I agree that the runtime effect of one list vs. the other is nil. The
code works and can stay as you written it.
I was trying to align from a conceptual standpoint. It was a bit
unexpected that one path would be on the hypervisor list, and the
other path effectively on the informational list. When I see
conceptual mismatches like that, I tend to want to understand why,
and if there is something more fundamental that is out-of-whack.
>
> Let me know your considerations, in case you can experiment with the
> patch set as-is.
> Cheers,
>
>
> Guilherme
Powered by blists - more mailing lists