diff mbox

[azure,12/14] UBUNTU: SAUCE: vmbus: fix hv_percpu_channel_deq/enq race

Message ID 1497979654-27251-13-git-send-email-marcelo.cerri@canonical.com
State New
Headers show

Commit Message

Marcelo Henrique Cerri June 20, 2017, 5:27 p.m. UTC
From: Dexuan Cui <decui@microsoft.com>

BugLink: http://bugs.launchpad.net/bugs/1698425

Signed-off-by: Dexuan Cui <decui@microsoft.com>
Signed-off-by: Marcelo Henrique Cerri <marcelo.cerri@canonical.com>
---
 drivers/hv/channel_mgmt.c | 32 +++++++++++++++++++++----
 drivers/hv/connection.c   | 11 +++++++++
 drivers/hv/hyperv_vmbus.h |  1 +
 drivers/hv/vmbus_drv.c    | 59 ++++++++++++++++++++++++++++++++++++++++++++---
 4 files changed, 95 insertions(+), 8 deletions(-)
diff mbox

Patch

diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index f5296548cdc9..19ad79099492 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -365,11 +365,16 @@  static void percpu_channel_enq(void *arg)
 
 void hv_percpu_channel_enq(struct vmbus_channel *channel)
 {
+	unsigned long flags;
+
 	if (channel->target_cpu != get_cpu())
 		smp_call_function_single(channel->target_cpu,
 					 percpu_channel_enq, channel, true);
-	else
+	else {
+		local_irq_save(flags);
 		percpu_channel_enq(channel);
+		local_irq_restore(flags);
+	}
 
 	put_cpu();
 }
@@ -383,11 +388,16 @@  static void percpu_channel_deq(void *arg)
 
 void hv_percpu_channel_deq(struct vmbus_channel *channel)
 {
+	unsigned long flags;
+
 	if (channel->target_cpu != get_cpu())
 		smp_call_function_single(channel->target_cpu,
 					 percpu_channel_deq, channel, true);
-	else
+	else {
+		local_irq_save(flags);
 		percpu_channel_deq(channel);
+		local_irq_restore(flags);
+	}
 
 	put_cpu();
 }
@@ -495,7 +505,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;
 		}
 	}
@@ -549,6 +558,7 @@  static void vmbus_process_offer(struct vmbus_channel *newchannel)
 	return;
 
 err_deq_chan:
+	atomic_dec(&vmbus_connection.offer_in_progress);
 	mutex_lock(&vmbus_connection.channel_mutex);
 	list_del(&newchannel->listentry);
 	mutex_unlock(&vmbus_connection.channel_mutex);
@@ -915,16 +925,28 @@  static void vmbus_onoffer_rescind(struct vmbus_channel_message_header *hdr)
 	}
 }
 
+static void vmbus_stop_rescind_handling_work(struct work_struct *work)
+{
+	atomic_inc(&vmbus_connection.offer_in_progress);
+}
+
 void vmbus_hvsock_device_unregister(struct vmbus_channel *channel)
 {
-	mutex_lock(&vmbus_connection.channel_mutex);
+	struct work_struct work;
 
 	BUG_ON(!is_hvsock_channel(channel));
 
+	/* Prevent chn_rescind_callback from running in the rescind path */
+	INIT_WORK(&work, vmbus_stop_rescind_handling_work);
+	queue_work_on(vmbus_connection.connect_cpu,
+		      vmbus_connection.work_queue_rescind, &work);
+	flush_work(&work);
+
 	channel->rescind = true;
 	vmbus_device_unregister(channel->device_obj);
 
-	mutex_unlock(&vmbus_connection.channel_mutex);
+	/* Unblock the rescind handling */
+	atomic_dec(&vmbus_connection.offer_in_progress);
 }
 EXPORT_SYMBOL_GPL(vmbus_hvsock_device_unregister);
 
diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
index 13e2e148067b..9e4a3d099836 100644
--- a/drivers/hv/connection.c
+++ b/drivers/hv/connection.c
@@ -156,6 +156,12 @@  int vmbus_connect(void)
 		goto cleanup;
 	}
 
+	vmbus_connection.work_queue_rescind = create_workqueue("hv_vmbus_rsd");
+	if (!vmbus_connection.work_queue_rescind) {
+		ret = -ENOMEM;
+		goto cleanup;
+	}
+
 	INIT_LIST_HEAD(&vmbus_connection.chn_msg_list);
 	spin_lock_init(&vmbus_connection.channelmsg_lock);
 
@@ -246,6 +252,11 @@  void vmbus_disconnect(void)
 	 */
 	vmbus_initiate_unload(false);
 
+	if (vmbus_connection.work_queue_rescind) {
+		drain_workqueue(vmbus_connection.work_queue_rescind);
+		destroy_workqueue(vmbus_connection.work_queue_rescind);
+	}
+
 	if (vmbus_connection.work_queue) {
 		drain_workqueue(vmbus_connection.work_queue);
 		destroy_workqueue(vmbus_connection.work_queue);
diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index 8ce4ae1c78d0..7b8603a00555 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -339,6 +339,7 @@  struct vmbus_connection {
 	struct mutex channel_mutex;
 
 	struct workqueue_struct *work_queue;
+	struct workqueue_struct *work_queue_rescind;
 };
 
 
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 59bb3efa6e10..fd221cffeb4d 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -837,6 +837,52 @@  static void vmbus_onmessage_work(struct work_struct *work)
 	kfree(ctx);
 }
 
+static void vmbus_dispatch_msg_work(struct work_struct *work)
+{
+	struct vmbus_channel_message_header *hdr;
+	struct onmessage_work_context *ctx, *context;
+
+	ctx = container_of(work, struct onmessage_work_context, work);
+	hdr = (struct vmbus_channel_message_header *)ctx->msg.u.payload;
+
+	context = kmalloc(sizeof(*context), GFP_KERNEL | __GFP_NOFAIL);
+	INIT_WORK(&context->work, vmbus_onmessage_work);
+	memcpy(&context->msg, &ctx->msg, sizeof(struct hv_message));
+
+	/*
+	 * The host can generate a rescind message while we
+	 * may still be handling the original offer. We deal with
+	 * this condition by ensuring the processing is done on the
+	 * same CPU.
+	 */
+	switch (hdr->msgtype) {
+	case CHANNELMSG_RESCIND_CHANNELOFFER:
+		/*
+		 * If we are handling the rescind message;
+		 * schedule the work on the global work queue.
+		 */
+		queue_work_on(vmbus_connection.connect_cpu,
+			      vmbus_connection.work_queue_rescind,
+			      &context->work);
+		break;
+
+	case CHANNELMSG_OFFERCHANNEL:
+		/* XXX */
+		flush_workqueue(vmbus_connection.work_queue_rescind);
+
+		atomic_inc(&vmbus_connection.offer_in_progress);
+		queue_work_on(vmbus_connection.connect_cpu,
+			      vmbus_connection.work_queue,
+			      &context->work);
+		break;
+
+	default:
+		queue_work(vmbus_connection.work_queue, &context->work);
+	}
+
+	kfree(ctx);
+}
+
 static void hv_process_timer_expiration(struct hv_message *msg,
 					struct hv_per_cpu_context *hv_cpu)
 {
@@ -876,9 +922,10 @@  void vmbus_on_msg_dpc(unsigned long data)
 		if (ctx == NULL)
 			return;
 
-		INIT_WORK(&ctx->work, vmbus_onmessage_work);
+		INIT_WORK(&ctx->work, vmbus_dispatch_msg_work);
 		memcpy(&ctx->msg, msg, sizeof(*msg));
 
+#if 0
 		/*
 		 * The host can generate a rescind message while we
 		 * may still be handling the original offer. We deal with
@@ -891,8 +938,9 @@  void vmbus_on_msg_dpc(unsigned long data)
 			 * If we are handling the rescind message;
 			 * schedule the work on the global work queue.
 			 */
-			schedule_work_on(vmbus_connection.connect_cpu,
-					 &ctx->work);
+			queue_work_on(vmbus_connection.connect_cpu,
+				      vmbus_connection.work_queue_rescind,
+				      &ctx->work);
 			break;
 
 		case CHANNELMSG_OFFERCHANNEL:
@@ -905,6 +953,9 @@  void vmbus_on_msg_dpc(unsigned long data)
 		default:
 			queue_work(vmbus_connection.work_queue, &ctx->work);
 		}
+#else
+		schedule_work(&ctx->work);
+#endif
 	} else
 		entry->message_handler(hdr);
 
@@ -1202,6 +1253,8 @@  int vmbus_device_register(struct hv_device *child_device_obj)
 	child_device_obj->device.parent = &hv_acpi_dev->dev;
 	child_device_obj->device.release = vmbus_device_release;
 
+	if (is_hvsock_channel(child_device_obj->channel))
+		dev_set_uevent_suppress(&child_device_obj->device, 1);
 	/*
 	 * Register with the LDM. This will kick off the driver/device
 	 * binding...which will eventually call vmbus_match() and vmbus_probe()