2020-09-13 19:51:29

by Michael Kelley (LINUX)

[permalink] [raw]
Subject: [PATCH 1/1] Drivers: hv: vmbus: Add timeout to vmbus_wait_for_unload

vmbus_wait_for_unload() looks for a CHANNELMSG_UNLOAD_RESPONSE message
coming from Hyper-V. But if the message isn't found for some reason,
the panic path gets hung forever. Add a timeout of 10 seconds to prevent
this.

Fixes: 415719160de3 ("Drivers: hv: vmbus: avoid scheduling in interrupt context in vmbus_initiate_unload()")
Signed-off-by: Michael Kelley <[email protected]>
---
drivers/hv/channel_mgmt.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index 591106c..1d44bb6 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -731,7 +731,7 @@ static void vmbus_wait_for_unload(void)
void *page_addr;
struct hv_message *msg;
struct vmbus_channel_message_header *hdr;
- u32 message_type;
+ u32 message_type, i;

/*
* CHANNELMSG_UNLOAD_RESPONSE is always delivered to the CPU which was
@@ -741,8 +741,11 @@ static void vmbus_wait_for_unload(void)
* functional and vmbus_unload_response() will complete
* vmbus_connection.unload_event. If not, the last thing we can do is
* read message pages for all CPUs directly.
+ *
+ * Wait no more than 10 seconds so that the panic path can't get
+ * hung forever in case the response message isn't seen.
*/
- while (1) {
+ for (i = 0; i < 1000; i++) {
if (completion_done(&vmbus_connection.unload_event))
break;

--
1.8.3.1


2020-09-13 19:59:42

by Dexuan Cui

[permalink] [raw]
Subject: RE: [PATCH 1/1] Drivers: hv: vmbus: Add timeout to vmbus_wait_for_unload

> From: [email protected]
> <[email protected]> On Behalf Of Michael Kelley
> Sent: Sunday, September 13, 2020 12:47 PM
>
> vmbus_wait_for_unload() looks for a CHANNELMSG_UNLOAD_RESPONSE
> message
> coming from Hyper-V. But if the message isn't found for some reason,
> the panic path gets hung forever. Add a timeout of 10 seconds to prevent
> this.
>
> Fixes: 415719160de3 ("Drivers: hv: vmbus: avoid scheduling in interrupt
> context in vmbus_initiate_unload()")
> Signed-off-by: Michael Kelley <[email protected]>

Reviewed-by: Dexuan Cui <[email protected]>

2020-09-14 09:04:25

by Wei Liu

[permalink] [raw]
Subject: Re: [PATCH 1/1] Drivers: hv: vmbus: Add timeout to vmbus_wait_for_unload

On Sun, Sep 13, 2020 at 07:56:30PM +0000, Dexuan Cui wrote:
> > From: [email protected]
> > <[email protected]> On Behalf Of Michael Kelley
> > Sent: Sunday, September 13, 2020 12:47 PM
> >
> > vmbus_wait_for_unload() looks for a CHANNELMSG_UNLOAD_RESPONSE
> > message
> > coming from Hyper-V. But if the message isn't found for some reason,
> > the panic path gets hung forever. Add a timeout of 10 seconds to prevent
> > this.
> >
> > Fixes: 415719160de3 ("Drivers: hv: vmbus: avoid scheduling in interrupt
> > context in vmbus_initiate_unload()")
> > Signed-off-by: Michael Kelley <[email protected]>
>
> Reviewed-by: Dexuan Cui <[email protected]>

LGTM.

Applied to hyperv-fixes. Thanks.

Wei.

2020-09-14 10:10:58

by Vitaly Kuznetsov

[permalink] [raw]
Subject: Re: ** POTENTIAL FRAUD ALERT - RED HAT ** [PATCH 1/1] Drivers: hv: vmbus: Add timeout to vmbus_wait_for_unload

Michael Kelley <[email protected]> writes:

> vmbus_wait_for_unload() looks for a CHANNELMSG_UNLOAD_RESPONSE message
> coming from Hyper-V. But if the message isn't found for some reason,
> the panic path gets hung forever. Add a timeout of 10 seconds to prevent
> this.

If I remember correctly, the problem I was observing back then was that
if CHANNELMSG_UNLOAD_RESPONSE is not delivered, Hyper-V won't respond to
the consequent CHANNELMSG_INITIATE_CONTACT/CHANNELMSG_REQUESTOFFERS
(don't remember exactly) so we either hang here or crash in the kdump
kernel because we can't find any devices. Maybe the problem was only
with some ancient Hyper-V versions or it was fixed.

>
> Fixes: 415719160de3 ("Drivers: hv: vmbus: avoid scheduling in interrupt context in vmbus_initiate_unload()")
> Signed-off-by: Michael Kelley <[email protected]>
> ---
> drivers/hv/channel_mgmt.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
> index 591106c..1d44bb6 100644
> --- a/drivers/hv/channel_mgmt.c
> +++ b/drivers/hv/channel_mgmt.c
> @@ -731,7 +731,7 @@ static void vmbus_wait_for_unload(void)
> void *page_addr;
> struct hv_message *msg;
> struct vmbus_channel_message_header *hdr;
> - u32 message_type;
> + u32 message_type, i;
>
> /*
> * CHANNELMSG_UNLOAD_RESPONSE is always delivered to the CPU which was
> @@ -741,8 +741,11 @@ static void vmbus_wait_for_unload(void)
> * functional and vmbus_unload_response() will complete
> * vmbus_connection.unload_event. If not, the last thing we can do is
> * read message pages for all CPUs directly.
> + *
> + * Wait no more than 10 seconds so that the panic path can't get
> + * hung forever in case the response message isn't seen.
> */
> - while (1) {
> + for (i = 0; i < 1000; i++) {
> if (completion_done(&vmbus_connection.unload_event))
> break;

LGTM,

Reviewed-by: Vitaly Kuznetsov <[email protected]>

--
Vitaly