[PATCH 0000/0004] Staging: hv: Driver cleanup

virtualization.lists.linux-foundation.org archive mirror
 help / color / mirror / Atom feed

* [PATCH 0000/0004] Staging: hv: Driver cleanup
@ 2011-08-31 21:35 K. Y. Srinivasan
  2011-08-31 21:35 ` [PATCH 1/4] Staging: hv: util: Deal with driver register failures K. Y. Srinivasan
  0 siblings, 1 reply; 5+ messages in thread
From: K. Y. Srinivasan @ 2011-08-31 21:35 UTC (permalink / raw)
  To: gregkh, linux-kernel, devel, virtualization; +Cc: K. Y. Srinivasan

Address the comments on the last couple of patch sets.


Regards,

K. Y

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/4] Staging: hv: util: Deal with driver register failures
  2011-08-31 21:35 [PATCH 0000/0004] Staging: hv: Driver cleanup K. Y. Srinivasan
@ 2011-08-31 21:35 ` K. Y. Srinivasan
  2011-08-31 21:35   ` [PATCH 2/4] Staging: hv: vmbus: Fix a bug in error handling in vmbus_bus_init() K. Y. Srinivasan
                     ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: K. Y. Srinivasan @ 2011-08-31 21:35 UTC (permalink / raw)
  To: gregkh, linux-kernel, devel, virtualization
  Cc: K. Y. Srinivasan, Haiyang Zhang

Properly deal with vmbus_driver_register() failures.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
---
 drivers/staging/hv/hv_util.c |   21 ++++++++++++++++-----
 1 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/hv/hv_util.c b/drivers/staging/hv/hv_util.c
index e29a2a2..6039217 100644
--- a/drivers/staging/hv/hv_util.c
+++ b/drivers/staging/hv/hv_util.c
@@ -277,6 +277,7 @@ static  struct hv_driver util_drv = {
 
 static int __init init_hyperv_utils(void)
 {
+	int ret;
 	pr_info("Registering HyperV Utility Driver\n");
 
 	if (hv_kvp_init())
@@ -289,12 +290,15 @@ static int __init init_hyperv_utils(void)
 
 	if (!shut_txf_buf || !time_txf_buf || !hbeat_txf_buf) {
 		pr_info("Unable to allocate memory for receive buffer\n");
-		kfree(shut_txf_buf);
-		kfree(time_txf_buf);
-		kfree(hbeat_txf_buf);
-		return -ENOMEM;
+		ret = -ENOMEM;
+		goto err;
 	}
 
+	ret = vmbus_driver_register(&util_drv);
+
+	if (ret != 0)
+		goto err;
+
 	hv_cb_utils[HV_SHUTDOWN_MSG].callback = &shutdown_onchannelcallback;
 
 	hv_cb_utils[HV_TIMESYNC_MSG].callback = &timesync_onchannelcallback;
@@ -303,7 +307,14 @@ static int __init init_hyperv_utils(void)
 
 	hv_cb_utils[HV_KVP_MSG].callback = &hv_kvp_onchannelcallback;
 
-	return vmbus_driver_register(&util_drv);
+	return 0;
+
+err:
+	kfree(shut_txf_buf);
+	kfree(time_txf_buf);
+	kfree(hbeat_txf_buf);
+
+	return ret;
 }
 
 static void exit_hyperv_utils(void)
-- 
1.7.4.1

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 2/4] Staging: hv: vmbus: Fix a bug in error handling in vmbus_bus_init()
  2011-08-31 21:35 ` [PATCH 1/4] Staging: hv: util: Deal with driver register failures K. Y. Srinivasan
@ 2011-08-31 21:35   ` K. Y. Srinivasan
  2011-08-31 21:35   ` [PATCH 3/4] Staging: hv: vmbus: Check for events before messages K. Y. Srinivasan
  2011-08-31 21:35   ` [PATCH 4/4] Staging: hv: vmbus: Cleanup the code in process_chn_event() K. Y. Srinivasan
  2 siblings, 0 replies; 5+ messages in thread
From: K. Y. Srinivasan @ 2011-08-31 21:35 UTC (permalink / raw)
  To: gregkh, linux-kernel, devel, virtualization
  Cc: K. Y. Srinivasan, Haiyang Zhang

Fix a bug in error handling in vmbus_bus_init().

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
---
 drivers/staging/hv/vmbus_drv.c |   26 +++++++++++++++-----------
 1 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/hv/vmbus_drv.c b/drivers/staging/hv/vmbus_drv.c
index 757943b..b5e06d6 100644
--- a/drivers/staging/hv/vmbus_drv.c
+++ b/drivers/staging/hv/vmbus_drv.c
@@ -492,7 +492,7 @@ static int vmbus_bus_init(int irq)
 
 	ret = bus_register(&hv_bus);
 	if (ret)
-		return ret;
+		goto err_cleanup;
 
 	ret = request_irq(irq, vmbus_isr, IRQF_SAMPLE_RANDOM,
 			driver_name, hv_acpi_dev);
@@ -500,10 +500,7 @@ static int vmbus_bus_init(int irq)
 	if (ret != 0) {
 		pr_err("Unable to request IRQ %d\n",
 			   irq);
-
-		bus_unregister(&hv_bus);
-
-		return ret;
+		goto err_unregister;
 	}
 
 	vector = IRQ0_VECTOR + irq;
@@ -514,16 +511,23 @@ static int vmbus_bus_init(int irq)
 	 */
 	on_each_cpu(hv_synic_init, (void *)&vector, 1);
 	ret = vmbus_connect();
-	if (ret) {
-		free_irq(irq, hv_acpi_dev);
-		bus_unregister(&hv_bus);
-		return ret;
-	}
-
+	if (ret)
+		goto err_irq;
 
 	vmbus_request_offers();
 
 	return 0;
+
+err_irq:
+	free_irq(irq, hv_acpi_dev);
+
+err_unregister:
+	bus_unregister(&hv_bus);
+
+err_cleanup:
+	hv_cleanup();
+
+	return ret;
 }
 
 /**
-- 
1.7.4.1

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 3/4] Staging: hv: vmbus: Check for events before messages
  2011-08-31 21:35 ` [PATCH 1/4] Staging: hv: util: Deal with driver register failures K. Y. Srinivasan
  2011-08-31 21:35   ` [PATCH 2/4] Staging: hv: vmbus: Fix a bug in error handling in vmbus_bus_init() K. Y. Srinivasan
@ 2011-08-31 21:35   ` K. Y. Srinivasan
  2011-08-31 21:35   ` [PATCH 4/4] Staging: hv: vmbus: Cleanup the code in process_chn_event() K. Y. Srinivasan
  2 siblings, 0 replies; 5+ messages in thread
From: K. Y. Srinivasan @ 2011-08-31 21:35 UTC (permalink / raw)
  To: gregkh, linux-kernel, devel, virtualization; +Cc: Haiyang Zhang

The Windows team has informed us that on Windows guests on Hyper-V,
they check for events before messages. They also recommended that we do
the same. This patch addresses this.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
---
 drivers/staging/hv/vmbus_drv.c |   22 ++++++++++++++--------
 1 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/hv/vmbus_drv.c b/drivers/staging/hv/vmbus_drv.c
index b5e06d6..cd43ddd 100644
--- a/drivers/staging/hv/vmbus_drv.c
+++ b/drivers/staging/hv/vmbus_drv.c
@@ -442,14 +442,11 @@ static irqreturn_t vmbus_isr(int irq, void *dev_id)
 	union hv_synic_event_flags *event;
 	bool handled = false;
 
-	page_addr = hv_context.synic_message_page[cpu];
-	msg = (struct hv_message *)page_addr + VMBUS_MESSAGE_SINT;
-
-	/* Check if there are actual msgs to be process */
-	if (msg->header.message_type != HVMSG_NONE) {
-		handled = true;
-		tasklet_schedule(&msg_dpc);
-	}
+	/*
+	 * Check for events before checking for messages. This is the order
+	 * in which events and messages are checked in Windows guests on
+	 * Hyper-V, and the Windows team suggested we do the same.
+	 */
 
 	page_addr = hv_context.synic_event_page[cpu];
 	event = (union hv_synic_event_flags *)page_addr + VMBUS_MESSAGE_SINT;
@@ -460,6 +457,15 @@ static irqreturn_t vmbus_isr(int irq, void *dev_id)
 		tasklet_schedule(&event_dpc);
 	}
 
+	page_addr = hv_context.synic_message_page[cpu];
+	msg = (struct hv_message *)page_addr + VMBUS_MESSAGE_SINT;
+
+	/* Check if there are actual msgs to be processed */
+	if (msg->header.message_type != HVMSG_NONE) {
+		handled = true;
+		tasklet_schedule(&msg_dpc);
+	}
+
 	if (handled)
 		return IRQ_HANDLED;
 	else
-- 
1.7.4.1

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 4/4] Staging: hv: vmbus: Cleanup the code in process_chn_event()
  2011-08-31 21:35 ` [PATCH 1/4] Staging: hv: util: Deal with driver register failures K. Y. Srinivasan
  2011-08-31 21:35   ` [PATCH 2/4] Staging: hv: vmbus: Fix a bug in error handling in vmbus_bus_init() K. Y. Srinivasan
  2011-08-31 21:35   ` [PATCH 3/4] Staging: hv: vmbus: Check for events before messages K. Y. Srinivasan
@ 2011-08-31 21:35   ` K. Y. Srinivasan
  2 siblings, 0 replies; 5+ messages in thread
From: K. Y. Srinivasan @ 2011-08-31 21:35 UTC (permalink / raw)
  To: gregkh, linux-kernel, devel, virtualization
  Cc: K. Y. Srinivasan, Haiyang Zhang

A channel in Hyper-V is equivalent to a device. Thus, a channel is
persistent once it is presented to the guest, even if the driver
managing this device is unloaded. By checking and invoking the driver
specific callback function under the protection of the channel
inbound_lock, we can properly deal with racing driver unloads since an
unloading driver sets the callback to NULL under the protection of this
inbound_lock.



Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
---
 drivers/staging/hv/connection.c |   18 ++++++++++++++++--
 1 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/hv/connection.c b/drivers/staging/hv/connection.c
index 9e99c04..649b91b 100644
--- a/drivers/staging/hv/connection.c
+++ b/drivers/staging/hv/connection.c
@@ -219,11 +219,25 @@ static void process_chn_event(u32 relid)
 	 */
 	channel = relid2channel(relid);
 
+	if (!channel) {
+		pr_err("channel not found for relid - %u\n", relid);
+		return;
+	}
+
+	/*
+	 * A channel once created is persistent even when there
+	 * is no driver handling the device. An unloading driver
+	 * sets the onchannel_callback to NULL under the
+	 * protection of the channel inbound_lock. Thus, checking
+	 * and invoking the driver specific callback takes care of
+	 * orderly unloading of the driver.
+	 */
+
 	spin_lock_irqsave(&channel->inbound_lock, flags);
-	if (channel && (channel->onchannel_callback != NULL))
+	if (channel->onchannel_callback != NULL)
 		channel->onchannel_callback(channel->channel_callback_context);
 	else
-		pr_err("channel not found for relid - %u\n", relid);
+		pr_err("no channel callback for relid - %u\n", relid);
 
 	spin_unlock_irqrestore(&channel->inbound_lock, flags);
 }
-- 
1.7.4.1

^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2011-08-31 21:35 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-08-31 21:35 [PATCH 0000/0004] Staging: hv: Driver cleanup K. Y. Srinivasan
2011-08-31 21:35 ` [PATCH 1/4] Staging: hv: util: Deal with driver register failures K. Y. Srinivasan
2011-08-31 21:35   ` [PATCH 2/4] Staging: hv: vmbus: Fix a bug in error handling in vmbus_bus_init() K. Y. Srinivasan
2011-08-31 21:35   ` [PATCH 3/4] Staging: hv: vmbus: Check for events before messages K. Y. Srinivasan
2011-08-31 21:35   ` [PATCH 4/4] Staging: hv: vmbus: Cleanup the code in process_chn_event() K. Y. Srinivasan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).