Re: [PATCH V3 10/11] drivers/hv/vmbus, video/hyperv_fb: Untangle and refactor Hyper-V panic notifiers
On 11/11/2022 20:16, Wei Liu wrote: > On Fri, Nov 11, 2022 at 10:47:17PM +, Wei Liu wrote: >> On Thu, Nov 10, 2022 at 06:32:54PM -0300, Guilherme G. Piccoli wrote: >>> [Trimming long CC list] >>> >>> Hi Wei / Michael, just out of curiosity, did this (and patch 9) ended-up >>> being queued in hyperv-next? >>> >> >> No. They are not queued. >> >> I didn't notice Michael's email and thought they would be picked up by >> someone else while dealing with the whole series. >> >> I will pick them up later. > > They are now queued to hyperv-next. Thanks. Thanks a lot Wei and Michael, much appreciated =) ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH V3 10/11] drivers/hv/vmbus, video/hyperv_fb: Untangle and refactor Hyper-V panic notifiers
On Fri, Nov 11, 2022 at 10:47:17PM +, Wei Liu wrote: > On Thu, Nov 10, 2022 at 06:32:54PM -0300, Guilherme G. Piccoli wrote: > > [Trimming long CC list] > > > > Hi Wei / Michael, just out of curiosity, did this (and patch 9) ended-up > > being queued in hyperv-next? > > > > No. They are not queued. > > I didn't notice Michael's email and thought they would be picked up by > someone else while dealing with the whole series. > > I will pick them up later. They are now queued to hyperv-next. Thanks. ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH V3 10/11] drivers/hv/vmbus, video/hyperv_fb: Untangle and refactor Hyper-V panic notifiers
On Thu, Nov 10, 2022 at 06:32:54PM -0300, Guilherme G. Piccoli wrote: > [Trimming long CC list] > > Hi Wei / Michael, just out of curiosity, did this (and patch 9) ended-up > being queued in hyperv-next? > No. They are not queued. I didn't notice Michael's email and thought they would be picked up by someone else while dealing with the whole series. I will pick them up later. Thanks, Wei. > Thanks in advance, > > > Guilherme > > On 17/10/2022 12:26, Michael Kelley (LINUX) wrote: > > From: Guilherme G. Piccoli Sent: Tuesday, October 4, > > 2022 10:20 AM > >> > >> On 04/10/2022 13:24, Michael Kelley (LINUX) wrote: > >>> [...] > >>> > >>> Tested this patch in combination with Patch 9 in this series. Verified > >>> that both the panic and die paths work correctly with notification to > >>> Hyper-V via hyperv_report_panic() or via hv_kmsg_dump(). Hyper-V > >>> framebuffer is updated as expected, though I did not reproduce > >>> a case where the ring buffer lock is held. vmbus_initiate_unload() runs > >>> as expected. > >>> > >>> Tested-by: Michael Kelley > >>> > >> > >> Thanks a lot for the tests/review Michael! > >> > >> Do you think Hyper-V folks could add both patches in hv tree? If you > >> prefer, I can re-send them individually. > >> > > > > Wei Liu: Could you pick up Patch 9 and Patch 10 from this series in the > > hyperv-next tree? > > > > Michael > > ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH V3 10/11] drivers/hv/vmbus, video/hyperv_fb: Untangle and refactor Hyper-V panic notifiers
[Trimming long CC list] Hi Wei / Michael, just out of curiosity, did this (and patch 9) ended-up being queued in hyperv-next? Thanks in advance, Guilherme On 17/10/2022 12:26, Michael Kelley (LINUX) wrote: > From: Guilherme G. Piccoli Sent: Tuesday, October 4, > 2022 10:20 AM >> >> On 04/10/2022 13:24, Michael Kelley (LINUX) wrote: >>> [...] >>> >>> Tested this patch in combination with Patch 9 in this series. Verified >>> that both the panic and die paths work correctly with notification to >>> Hyper-V via hyperv_report_panic() or via hv_kmsg_dump(). Hyper-V >>> framebuffer is updated as expected, though I did not reproduce >>> a case where the ring buffer lock is held. vmbus_initiate_unload() runs >>> as expected. >>> >>> Tested-by: Michael Kelley >>> >> >> Thanks a lot for the tests/review Michael! >> >> Do you think Hyper-V folks could add both patches in hv tree? If you >> prefer, I can re-send them individually. >> > > Wei Liu: Could you pick up Patch 9 and Patch 10 from this series in the > hyperv-next tree? > > Michael > ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
RE: [PATCH V3 10/11] drivers/hv/vmbus, video/hyperv_fb: Untangle and refactor Hyper-V panic notifiers
From: Guilherme G. Piccoli Sent: Tuesday, October 4, 2022 10:20 AM > > On 04/10/2022 13:24, Michael Kelley (LINUX) wrote: > > [...] > > > > Tested this patch in combination with Patch 9 in this series. Verified > > that both the panic and die paths work correctly with notification to > > Hyper-V via hyperv_report_panic() or via hv_kmsg_dump(). Hyper-V > > framebuffer is updated as expected, though I did not reproduce > > a case where the ring buffer lock is held. vmbus_initiate_unload() runs > > as expected. > > > > Tested-by: Michael Kelley > > > > Thanks a lot for the tests/review Michael! > > Do you think Hyper-V folks could add both patches in hv tree? If you > prefer, I can re-send them individually. > Wei Liu: Could you pick up Patch 9 and Patch 10 from this series in the hyperv-next tree? Michael ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH V3 10/11] drivers/hv/vmbus, video/hyperv_fb: Untangle and refactor Hyper-V panic notifiers
On 04/10/2022 13:24, Michael Kelley (LINUX) wrote: > [...] > > Tested this patch in combination with Patch 9 in this series. Verified > that both the panic and die paths work correctly with notification to > Hyper-V via hyperv_report_panic() or via hv_kmsg_dump(). Hyper-V > framebuffer is updated as expected, though I did not reproduce > a case where the ring buffer lock is held. vmbus_initiate_unload() runs > as expected. > > Tested-by: Michael Kelley > Thanks a lot for the tests/review Michael! Do you think Hyper-V folks could add both patches in hv tree? If you prefer, I can re-send them individually. Cheers, Guilherme ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
RE: [PATCH V3 10/11] drivers/hv/vmbus, video/hyperv_fb: Untangle and refactor Hyper-V panic notifiers
From: Guilherme G. Piccoli Sent: Friday, August 19, 2022
3:18 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)
> Cc: Dexuan Cui
> Cc: Haiyang Zhang
> Cc: "K. Y. Srinivasan"
> Cc: Petr Mladek
> Cc: Stephen Hemminger
> Cc: Tianyu Lan
> Cc: Wei Liu
> Reviewed-by: Michael Kelley
> Tested-by: Fabio A M Martins
> Signed-off-by: Guilherme G. Piccoli
>
> ---
>
> V3:
> - Added Michael's review tag - thanks!
>
> 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-4a15d7070...@igalia.com/
>
>
> drivers/hv/vmbus_drv.c | 109 +++-
> drivers/video/fbdev/hyperv_fb.c | 8 +++
> 2 files changed, 74 insertions(+), 43 deletions(-)
>
Tested this patch in combination with Patch 9 in this series. Verified
that both the panic and die paths work correctly with notification to
Hyper-V via hyperv_report_panic() or via hv_kmsg_dump(). Hyper-V
framebuffer is updated as expected, though I did not reproduce
a case where the ring buffer lock is held. vmbus_initiate_unload() runs
as expected.
Tested-by: Michael Kelley
> diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> index 23c680d1a0f5..0a77e2bb0b70 100644
> --- a/drivers/hv/vmbus_drv.c
> +++ b/drivers/hv/vmbus_drv.c
> @@ -25,7 +25,6 @@
> #include
>
> #include
> -#include
> #include
> #include
> #include
> @@ -69,53 +68,74 @@ static int hyperv_report_reg(void)
> return !sysctl_record_panic_msg || !hv_panic_page;
> }
>
> -static int hyperv_panic_event(struct notifier_block *nb, unsigned long val,
> +/*
> + * The panic notifier below is responsible solely for unloading the
> + * vmbus connection, which is necessary in a panic event.
> + *
> + * Notice an intrincate relation of this notifier with Hyper-V
> + * framebuffer panic notifier exists - we need vmbus connection alive
> + * there in order to succeed, so we need to order both with each other
> + * [see hvfb_on_panic()] - this is done using notifiers' priorities.
> + */
> +static int hv_panic_vmbus_unload(struct notifier_block *nb, unsigned long
> val,
> void *args)
> +{
> + vmbus_initiate_unload(true);
> + return NOTIFY_DONE;
> +}
> +static struct notifier_block hyperv_panic_vmbus_unload_block = {
> + .notifier_call = hv_panic_vmbus_unload,
> + .priority = INT_MIN + 1, /* almost the latest one to execute */
> +};
> +
> +static int hv_die_panic_notify_crash(struct notifier_block *self,
> + unsigned long
