Messages in this thread | | | From | "Michael Kelley (LINUX)" <> | Subject | RE: [PATCH v2 12/13] drivers/hv/vmbus, video/hyperv_fb: Untangle and refactor Hyper-V panic notifiers | Date | Mon, 25 Jul 2022 18:55:20 +0000 |
| |
From: Guilherme G. Piccoli <gpiccoli@igalia.com> Sent: Tuesday, July 19, 2022 12:53 PM > > Currently Hyper-V guests are among the most relevant users of the panic > infrastructure, like panic notifiers, kmsg dumpers, etc. The reasons rely > both in cleaning-up procedures (closing hypervisor <-> guest connection, > disabling some paravirtualized timer) as well as to data collection > (sending panic information to the hypervisor) and framebuffer management. > > The thing is: some notifiers are related to others, ordering matters, some > functionalities are duplicated and there are lots of conditionals behind > sending panic information to the hypervisor. As part of an effort to > clean-up the panic notifiers mechanism and better document things, we > hereby address some of the issues/complexities of Hyper-V panic handling > through the following changes: > > (a) We have die and panic notifiers on vmbus_drv.c and both have goals of > sending panic information to the hypervisor, though the panic notifier is > also responsible for a cleaning-up procedure. > > This commit clears the code by splitting the panic notifier in two, one > for closing the vmbus connection whereas the other is only for sending > panic info to hypervisor. With that, it was possible to merge the die and > panic notifiers in a single/well-documented function, and clear some > conditional complexities on sending such information to the hypervisor. > > (b) There is a Hyper-V framebuffer panic notifier, which relies in doing > a vmbus operation that demands a valid connection. So, we must order this > notifier with the panic notifier from vmbus_drv.c, to guarantee that the > framebuffer code executes before the vmbus connection is unloaded. > > Also, this commit removes a useless header. > > Although there is code rework and re-ordering, we expect that this change > has no functional regressions but instead optimize the path and increase > panic reliability on Hyper-V. This was tested on Hyper-V with success. > > Cc: Andrea Parri (Microsoft) <parri.andrea@gmail.com> > Cc: Dexuan Cui <decui@microsoft.com> > Cc: Haiyang Zhang <haiyangz@microsoft.com> > Cc: "K. Y. Srinivasan" <kys@microsoft.com> > Cc: Michael Kelley <mikelley@microsoft.com> > Cc: Petr Mladek <pmladek@suse.com> > Cc: Stephen Hemminger <sthemmin@microsoft.com> > Cc: Tianyu Lan <Tianyu.Lan@microsoft.com> > Cc: Wei Liu <wei.liu@kernel.org> > Tested-by: Fabio A M Martins <fabiomirmar@gmail.com> > Signed-off-by: Guilherme G. Piccoli <gpiccoli@igalia.com> > > --- > > > V2: > - Unfortunately we cannot rely in the crash shutdown (custom) handler > to perform the vmbus unload - arm64 architecture doesn't have this > "feature" [0]. So, in V2 we kept the notifier behavior and always > unload the vmbus connection, no matter what - thanks Michael for > pointing that; > > - Removed the Fixes tags as per Michael suggestion; > > - As per Petr suggestion, we abandoned the idea of distinguish among > notifiers using an id - so, in V2 we rely in the old and good address > comparison for that. Thanks Petr for the enriching discussion! > > [0] > https://lore.kernel.org/lkml/427a8277-49f0-4317-d6c3-4a15d7070e55@igalia.com/ > > > drivers/hv/vmbus_drv.c | 109 +++++++++++++++++++------------- > drivers/video/fbdev/hyperv_fb.c | 8 +++ > 2 files changed, 74 insertions(+), 43 deletions(-) >
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
| |