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

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

[Michael Kelley]

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 <mikelley@microsoft.com>
---
 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

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

[Dexuan Cui]

> From: linux-hyperv-owner@vger.kernel.org
> <linux-hyperv-owner@vger.kernel.org> 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 <mikelley@microsoft.com>

Reviewed-by: Dexuan Cui <decui@microsoft.com>

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

[Wei Liu]

On Sun, Sep 13, 2020 at 07:56:30PM +0000, Dexuan Cui wrote:
> > From: linux-hyperv-owner@vger.kernel.org
> > <linux-hyperv-owner@vger.kernel.org> 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 <mikelley@microsoft.com>
> 
> Reviewed-by: Dexuan Cui <decui@microsoft.com>

LGTM.

Applied to hyperv-fixes. Thanks.

Wei.

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

[Vitaly Kuznetsov]

Michael Kelley <mikelley@microsoft.com> 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 <mikelley@microsoft.com>
> ---
>  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 <vkuznets@redhat.com>

-- 
Vitaly