Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1413027 > unrolled thread

[PATCH] Drivers: hv: fix infinite wait when channel open timeouts

Started byVitaly Kuznetsov <vkuznets@redhat.com>
First post2016-06-03 12:20 +0200
Last post2016-06-03 22:10 +0200
Articles 2 — 2 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH] Drivers: hv: fix infinite wait when channel open timeouts Vitaly Kuznetsov <vkuznets@redhat.com> - 2016-06-03 12:20 +0200
    RE: [PATCH] Drivers: hv: fix infinite wait when channel open timeouts KY Srinivasan <kys@microsoft.com> - 2016-06-03 22:10 +0200

#1413027 — [PATCH] Drivers: hv: fix infinite wait when channel open timeouts

FromVitaly Kuznetsov <vkuznets@redhat.com>
Date2016-06-03 12:20 +0200
Subject[PATCH] Drivers: hv: fix infinite wait when channel open timeouts
Message-ID<rFUmR-1Sf-7@gated-at.bofh.it>
vmbus_teardown_gpadl() can result in infinite wait when it is called
on 5 second timeout in vmbus_open(). The issue is caused by the fact
that gpadl teardown operation won't ever succeed for an opened channel
and the timeout isn't always enough. Increase the timeout to 15 seconds
and add sending CHANNELMSG_CLOSECHANNEL message on timeout to make sure
the channel is closed.

The other possible solution for the issue would be to remove the timeout
in vmbus_open() making our wait infinite.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
 drivers/hv/channel.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
index 6a8660d..becb9e8 100644
--- a/drivers/hv/channel.c
+++ b/drivers/hv/channel.c
@@ -69,6 +69,7 @@ int vmbus_open(struct vmbus_channel *newchannel, u32 send_ringbuffer_size,
 		     void (*onchannelcallback)(void *context), void *context)
 {
 	struct vmbus_channel_open_channel *open_msg;
+	struct vmbus_channel_close_channel *close_msg;
 	struct vmbus_channel_msginfo *open_info = NULL;
 	void *in, *out;
 	unsigned long flags;
@@ -183,8 +184,19 @@ int vmbus_open(struct vmbus_channel *newchannel, u32 send_ringbuffer_size,
 		goto error1;
 	}
 
-	t = wait_for_completion_timeout(&open_info->waitevent, 5*HZ);
+	t = wait_for_completion_timeout(&open_info->waitevent, 15*HZ);
 	if (t == 0) {
+		/*
+		 * We won't be able to tear down the gpadl handle if the
+		 * channel finally gets openned, send channel close message
+		 * to be on the safe side.
+		 */
+		close_msg = &newchannel->close_msg.msg;
+		close_msg->header.msgtype = CHANNELMSG_CLOSECHANNEL;
+		close_msg->child_relid = newchannel->offermsg.child_relid;
+		vmbus_post_msg(close_msg,
+			       sizeof(struct vmbus_channel_close_channel));
+
 		err = -ETIMEDOUT;
 		goto error1;
 	}
-- 
2.5.5

[toc] | [next] | [standalone]


#1413438

FromKY Srinivasan <kys@microsoft.com>
Date2016-06-03 22:10 +0200
Message-ID<rG3zQ-7MZ-19@gated-at.bofh.it>
In reply to#1413027

> -----Original Message-----
> From: Vitaly Kuznetsov [mailto:vkuznets@redhat.com]
> Sent: Friday, June 3, 2016 3:15 AM
> To: devel@linuxdriverproject.org
> Cc: linux-kernel@vger.kernel.org; KY Srinivasan <kys@microsoft.com>; Haiyang
> Zhang <haiyangz@microsoft.com>
> Subject: [PATCH] Drivers: hv: fix infinite wait when channel open timeouts
> 
> vmbus_teardown_gpadl() can result in infinite wait when it is called
> on 5 second timeout in vmbus_open(). The issue is caused by the fact
> that gpadl teardown operation won't ever succeed for an opened channel
> and the timeout isn't always enough. Increase the timeout to 15 seconds
> and add sending CHANNELMSG_CLOSECHANNEL message on timeout to make
> sure
> the channel is closed.
> 
> The other possible solution for the issue would be to remove the timeout
> in vmbus_open() making our wait infinite.

I think removing the timeout in vmbus_open() maybe the safest solution since
anything else is still racy. A timeout in vmbus_open() tells us nothing about the
state of the channel. As a guest, we can trust the host to respond to our request.

Regards,

K. Y

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web