[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 19/30] panic: Add the panic hypervisor notifier list
- To: "Michael Kelley (LINUX)" <mikelley@xxxxxxxxxxxxx>, "akpm@xxxxxxxxxxxxxxxxxxxx" <akpm@xxxxxxxxxxxxxxxxxxxx>, "bhe@xxxxxxxxxx" <bhe@xxxxxxxxxx>, "pmladek@xxxxxxxx" <pmladek@xxxxxxxx>, "kexec@xxxxxxxxxxxxxxxxxxx" <kexec@xxxxxxxxxxxxxxxxxxx>
- From: "Guilherme G. Piccoli" <gpiccoli@xxxxxxxxxx>
- Date: Tue, 3 May 2022 14:56:27 -0300
- Cc: "linux-kernel@xxxxxxxxxxxxxxx" <linux-kernel@xxxxxxxxxxxxxxx>, "bcm-kernel-feedback-list@xxxxxxxxxxxx" <bcm-kernel-feedback-list@xxxxxxxxxxxx>, "linuxppc-dev@xxxxxxxxxxxxxxxx" <linuxppc-dev@xxxxxxxxxxxxxxxx>, "linux-alpha@xxxxxxxxxxxxxxx" <linux-alpha@xxxxxxxxxxxxxxx>, "linux-edac@xxxxxxxxxxxxxxx" <linux-edac@xxxxxxxxxxxxxxx>, "linux-hyperv@xxxxxxxxxxxxxxx" <linux-hyperv@xxxxxxxxxxxxxxx>, "linux-leds@xxxxxxxxxxxxxxx" <linux-leds@xxxxxxxxxxxxxxx>, "linux-mips@xxxxxxxxxxxxxxx" <linux-mips@xxxxxxxxxxxxxxx>, "linux-parisc@xxxxxxxxxxxxxxx" <linux-parisc@xxxxxxxxxxxxxxx>, "linux-pm@xxxxxxxxxxxxxxx" <linux-pm@xxxxxxxxxxxxxxx>, "linux-remoteproc@xxxxxxxxxxxxxxx" <linux-remoteproc@xxxxxxxxxxxxxxx>, "linux-s390@xxxxxxxxxxxxxxx" <linux-s390@xxxxxxxxxxxxxxx>, "linux-tegra@xxxxxxxxxxxxxxx" <linux-tegra@xxxxxxxxxxxxxxx>, "linux-um@xxxxxxxxxxxxxxxxxxx" <linux-um@xxxxxxxxxxxxxxxxxxx>, "linux-xtensa@xxxxxxxxxxxxxxxx" <linux-xtensa@xxxxxxxxxxxxxxxx>, "netdev@xxxxxxxxxxxxxxx" <netdev@xxxxxxxxxxxxxxx>, "openipmi-developer@xxxxxxxxxxxxxxxxxxxxx" <openipmi-developer@xxxxxxxxxxxxxxxxxxxxx>, "rcu@xxxxxxxxxxxxxxx" <rcu@xxxxxxxxxxxxxxx>, "sparclinux@xxxxxxxxxxxxxxx" <sparclinux@xxxxxxxxxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>, "x86@xxxxxxxxxx" <x86@xxxxxxxxxx>, "kernel-dev@xxxxxxxxxx" <kernel-dev@xxxxxxxxxx>, "kernel@xxxxxxxxxxxx" <kernel@xxxxxxxxxxxx>, "halves@xxxxxxxxxxxxx" <halves@xxxxxxxxxxxxx>, "fabiomirmar@xxxxxxxxx" <fabiomirmar@xxxxxxxxx>, "alejandro.j.jimenez@xxxxxxxxxx" <alejandro.j.jimenez@xxxxxxxxxx>, "andriy.shevchenko@xxxxxxxxxxxxxxx" <andriy.shevchenko@xxxxxxxxxxxxxxx>, "arnd@xxxxxxxx" <arnd@xxxxxxxx>, "bp@xxxxxxxxx" <bp@xxxxxxxxx>, "corbet@xxxxxxx" <corbet@xxxxxxx>, "d.hatayama@xxxxxxxxxxxxxx" <d.hatayama@xxxxxxxxxxxxxx>, "dave.hansen@xxxxxxxxxxxxxxx" <dave.hansen@xxxxxxxxxxxxxxx>, "dyoung@xxxxxxxxxx" <dyoung@xxxxxxxxxx>, "feng.tang@xxxxxxxxx" <feng.tang@xxxxxxxxx>, "gregkh@xxxxxxxxxxxxxxxxxxx" <gregkh@xxxxxxxxxxxxxxxxxxx>, "hidehiro.kawai.ez@xxxxxxxxxxx" <hidehiro.kawai.ez@xxxxxxxxxxx>, "jgross@xxxxxxxx" <jgross@xxxxxxxx>, "john.ogness@xxxxxxxxxxxxx" <john.ogness@xxxxxxxxxxxxx>, "keescook@xxxxxxxxxxxx" <keescook@xxxxxxxxxxxx>, "luto@xxxxxxxxxx" <luto@xxxxxxxxxx>, "mhiramat@xxxxxxxxxx" <mhiramat@xxxxxxxxxx>, "mingo@xxxxxxxxxx" <mingo@xxxxxxxxxx>, "paulmck@xxxxxxxxxx" <paulmck@xxxxxxxxxx>, "peterz@xxxxxxxxxxxxx" <peterz@xxxxxxxxxxxxx>, "rostedt@xxxxxxxxxxx" <rostedt@xxxxxxxxxxx>, "senozhatsky@xxxxxxxxxxxx" <senozhatsky@xxxxxxxxxxxx>, "stern@xxxxxxxxxxxxxxxxxxx" <stern@xxxxxxxxxxxxxxxxxxx>, "tglx@xxxxxxxxxxxxx" <tglx@xxxxxxxxxxxxx>, "vgoyal@xxxxxxxxxx" <vgoyal@xxxxxxxxxx>, vkuznets <vkuznets@xxxxxxxxxx>, "will@xxxxxxxxxx" <will@xxxxxxxxxx>, Alexander Gordeev <agordeev@xxxxxxxxxxxxx>, Andrea Parri <parri.andrea@xxxxxxxxx>, Ard Biesheuvel <ardb@xxxxxxxxxx>, Benjamin Herrenschmidt <benh@xxxxxxxxxxxxxxxxxxx>, Brian Norris <computersforpeace@xxxxxxxxx>, Christian Borntraeger <borntraeger@xxxxxxxxxxxxx>, Christophe JAILLET <christophe.jaillet@xxxxxxxxxx>, David Gow <davidgow@xxxxxxxxxx>, "David S. Miller" <davem@xxxxxxxxxxxxx>, Dexuan Cui <decui@xxxxxxxxxxxxx>, Doug Berger <opendmb@xxxxxxxxx>, Evan Green <evgreen@xxxxxxxxxxxx>, Florian Fainelli <f.fainelli@xxxxxxxxx>, Haiyang Zhang <haiyangz@xxxxxxxxxxxxx>, Hari Bathini <hbathini@xxxxxxxxxxxxx>, Heiko Carstens <hca@xxxxxxxxxxxxx>, Julius Werner <jwerner@xxxxxxxxxxxx>, Justin Chen <justinpopo6@xxxxxxxxx>, KY Srinivasan <kys@xxxxxxxxxxxxx>, Lee Jones <lee.jones@xxxxxxxxxx>, Markus Mayer <mmayer@xxxxxxxxxxxx>, Michael Ellerman <mpe@xxxxxxxxxxxxxx>, Mihai Carabas <mihai.carabas@xxxxxxxxxx>, Nicholas Piggin <npiggin@xxxxxxxxx>, Paul Mackerras <paulus@xxxxxxxxx>, Pavel Machek <pavel@xxxxxx>, Scott Branden <scott.branden@xxxxxxxxxxxx>, Sebastian Reichel <sre@xxxxxxxxxx>, Shile Zhang <shile.zhang@xxxxxxxxxxxxxxxxx>, Stephen Hemminger <sthemmin@xxxxxxxxxxxxx>, Sven Schnelle <svens@xxxxxxxxxxxxx>, Thomas Bogendoerfer <tsbogend@xxxxxxxxxxxxxxxx>, Tianyu Lan <Tianyu.Lan@xxxxxxxxxxxxx>, Vasily Gorbik <gor@xxxxxxxxxxxxx>, Wang ShaoBo <bobo.shaobowang@xxxxxxxxxx>, Wei Liu <wei.liu@xxxxxxxxxx>, zhenwei pi <pizhenwei@xxxxxxxxxxxxx>
- Delivery-date: Tue, 03 May 2022 17:58:38 +0000
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
On 03/05/2022 14:44, Michael Kelley (LINUX) wrote:
> [...]
>>
>> 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.
Thanks for the feedback Michael!
> [...]
>> 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.
>
Totally agree with you here, I am like that as well - try to really
understand the details, this is very important specially in this patch
set, since it's a refactor and affects every user of the notifiers
infrastructure.
Again, just to double-say it: feel free to suggest any change for the
Hyper-V portion (might as well for any patch in the series, indeed) -
you and the other Hyper-V maintainers own this code and I'd be glad to
align with your needs, you are honor citizens in the panic notifiers
area, being one the most heavy users for that =)
Cheers,
Guilherme
|