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

Groups > linux.kernel > #1709866 > unrolled thread

[PATCH 1/1] Drivers: hv: vmbus: Fix rescind handling issues

Back to article view | Back to linux.kernel

Contents

  [PATCH 1/1] Drivers: hv: vmbus: Fix rescind handling issues kys@exchange.microsoft.com - 2017-08-11 19:10 +0200
    Re: [PATCH 1/1] Drivers: hv: vmbus: Fix rescind handling issues Stephen Hemminger <stephen@networkplumber.org> - 2017-08-25 00:50 +0200

#1709866 — [PATCH 1/1] Drivers: hv: vmbus: Fix rescind handling issues

From: K. Y. Srinivasan <kys@microsoft.com>

This patch handles the following issues that were observed when we are
handling racing channel offer message and rescind message for the same
offer:

1. Since the host does not respond to messages on a rescinded channel,
in the current code, we could be indefinitely blocked on the vmbus_open() call.

2. When a rescinded channel is being closed, if there is a pending interrupt on the
channel, we could end up freeing the channel that the interrupt handler would run on.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Reviewed-by: Dexuan Cui <decui@microsoft.com>
Tested-by: Dexuan Cui <decui@microsoft.com>
---
 drivers/hv/channel.c      |   14 ++++++++++++++
 drivers/hv/channel_mgmt.c |   29 ++++++++++++++++++++++++++---
 drivers/hv/vmbus_drv.c    |    3 +++
 include/linux/hyperv.h    |    2 ++
 4 files changed, 45 insertions(+), 3 deletions(-)

diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
index e9bf0bb..966a823 100644
--- a/drivers/hv/channel.c
+++ b/drivers/hv/channel.c
@@ -177,6 +177,11 @@ int vmbus_open(struct vmbus_channel *newchannel, u32 send_ringbuffer_size,
 		      &vmbus_connection.chn_msg_list);
 	spin_unlock_irqrestore(&vmbus_connection.channelmsg_lock, flags);
 
+	if (newchannel->rescind) {
+		err = -ENODEV;
+		goto error_free_gpadl;
+	}
+
 	ret = vmbus_post_msg(open_msg,
 			     sizeof(struct vmbus_channel_open_channel), true);
 
@@ -421,6 +426,11 @@ int vmbus_establish_gpadl(struct vmbus_channel *channel, void *kbuffer,
 
 	spin_unlock_irqrestore(&vmbus_connection.channelmsg_lock, flags);
 
+	if (channel->rescind) {
+		ret = -ENODEV;
+		goto cleanup;
+	}
+
 	ret = vmbus_post_msg(gpadlmsg, msginfo->msgsize -
 			     sizeof(*msginfo), true);
 	if (ret != 0)
@@ -494,6 +504,10 @@ int vmbus_teardown_gpadl(struct vmbus_channel *channel, u32 gpadl_handle)
 	list_add_tail(&info->msglistentry,
 		      &vmbus_connection.chn_msg_list);
 	spin_unlock_irqrestore(&vmbus_connection.channelmsg_lock, flags);
+
+	if (channel->rescind)
+		goto post_msg_err;
+
 	ret = vmbus_post_msg(msg, sizeof(struct vmbus_channel_gpadl_teardown),
 			     true);
 
diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index 4bbb8de..968af17 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -451,6 +451,12 @@ static void vmbus_process_offer(struct vmbus_channel *newchannel)
 	/* Make sure this is a new offer */
 	mutex_lock(&vmbus_connection.channel_mutex);
 
+	/*
+	 * Now that we have acquired the channel_mutex,
+	 * we can release the potentially racing rescind thread.
+	 */
+	atomic_dec(&vmbus_connection.offer_in_progress);
+
 	list_for_each_entry(channel, &vmbus_connection.chn_list, listentry) {
 		if (!uuid_le_cmp(channel->offermsg.offer.if_type,
 			newchannel->offermsg.offer.if_type) &&
@@ -481,7 +487,6 @@ static void vmbus_process_offer(struct vmbus_channel *newchannel)
 			channel->num_sc++;
 			spin_unlock_irqrestore(&channel->lock, flags);
 		} else {
-			atomic_dec(&vmbus_connection.offer_in_progress);
 			goto err_free_chan;
 		}
 	}
@@ -510,7 +515,6 @@ static void vmbus_process_offer(struct vmbus_channel *newchannel)
 	if (!fnew) {
 		if (channel->sc_creation_callback != NULL)
 			channel->sc_creation_callback(newchannel);
-		atomic_dec(&vmbus_connection.offer_in_progress);
 		return;
 	}
 
@@ -541,7 +545,7 @@ static void vmbus_process_offer(struct vmbus_channel *newchannel)
 		goto err_deq_chan;
 	}
 
-	atomic_dec(&vmbus_connection.offer_in_progress);
+	newchannel->probe_done = true;
 	return;
 
 err_deq_chan:
@@ -882,8 +886,27 @@ static void vmbus_onoffer_rescind(struct vmbus_channel_message_header *hdr)
 	channel->rescind = true;
 	spin_unlock_irqrestore(&channel->lock, flags);
 
+	/*
+	 * Now that we have posted the rescind state, perform
+	 * rescind related cleanup.
+	 */
 	vmbus_rescind_cleanup(channel);
 
+	/*
+	 * Now wait for offer handling to complete.
+	 */
+	while (READ_ONCE(channel->probe_done) == false) {
+		/*
+		 * We wait here until any channel offer is currently
+		 * being processed.
+		 */
+		msleep(1);
+	}
+
+	/*
+	 * At this point, the rescind handling can proceed safely.
+	 */
+
 	if (channel->device_obj) {
 		if (channel->chn_rescind_callback) {
 			channel->chn_rescind_callback(channel);
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index ed84e96..43160a2 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -940,6 +940,9 @@ static void vmbus_chan_sched(struct hv_per_cpu_context *hv_cpu)
 			if (channel->offermsg.child_relid != relid)
 				continue;
 
+			if (channel->rescind)
+				continue;
+
 			switch (channel->callback_mode) {
 			case HV_CALL_ISR:
 				vmbus_channel_isr(channel);
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index 27db4e6..07650d0 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -879,6 +879,8 @@ struct vmbus_channel {
 	 */
 	enum hv_numa_policy affinity_policy;
 
+	bool probe_done;
+
 };
 
 static inline bool is_hvsock_channel(const struct vmbus_channel *c)
-- 
1.7.1

[toc] | [next] | [standalone]

#1719627

On Fri, 11 Aug 2017 10:03:59 -0700
kys@exchange.microsoft.com wrote:

> From: K. Y. Srinivasan <kys@microsoft.com>
> 
> This patch handles the following issues that were observed when we are
> handling racing channel offer message and rescind message for the same
> offer:
> 
> 1. Since the host does not respond to messages on a rescinded channel,
> in the current code, we could be indefinitely blocked on the vmbus_open() call.
> 
> 2. When a rescinded channel is being closed, if there is a pending interrupt on the
> channel, we could end up freeing the channel that the interrupt handler would run on.
> 
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> Reviewed-by: Dexuan Cui <decui@microsoft.com>
> Tested-by: Dexuan Cui <decui@microsoft.com>

This patch breaks re-initialization of the network device on MTU changes.

Doing:
	# ip li set dev eth1 mtu 9000

will hang in rndis_filter_add waiting for subchannel notification.
This is likely because when the vmbus device is reopened the sub channels
are not correctly created.  Not sure what is wrong with the patch, but my
suspicion is that the close/rescind events are no longer being sent to the
host.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web