virtualization.lists.linux-foundation.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] staging: hv: Fix race condition in hv_utils module initialization.
@ 2010-05-19 15:54 Haiyang Zhang
  0 siblings, 0 replies; 6+ messages in thread
From: Haiyang Zhang @ 2010-05-19 15:54 UTC (permalink / raw)
  To: 'linux-kernel@vger.kernel.org',
	'devel@driverdev.osuosl.org',
	"'virtualization@lists.osdl.org'" <virtualiz>

[-- Attachment #1: Type: text/plain, Size: 4803 bytes --]

From: Haiyang Zhang <haiyangz@microsoft.com>

Subject: [PATCH 1/2] staging: hv: Fix race condition in hv_utils module initialization.
There is a possible race condition when hv_utils starts to load immediately
after hv_vmbus is loading - null pointer error could happen.
This patch added an atomic counter to ensure the hv_utils module initialization
happens after all vmbus IC channels are initialized.

Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: Hank Janssen <hjanssen@microsoft.com>

---
 drivers/staging/hv/channel_mgmt.c |   26 +++++++++++++++-----------
 drivers/staging/hv/hv_utils.c     |   11 ++++++++---
 drivers/staging/hv/utils.h        |    5 +++++
 3 files changed, 28 insertions(+), 14 deletions(-)

diff --git a/drivers/staging/hv/channel_mgmt.c b/drivers/staging/hv/channel_mgmt.c
index 3f53b4d..b5b6a70 100644
--- a/drivers/staging/hv/channel_mgmt.c
+++ b/drivers/staging/hv/channel_mgmt.c
@@ -33,7 +33,6 @@ struct vmbus_channel_message_table_entry {
 	void (*messageHandler)(struct vmbus_channel_message_header *msg);
 };
 
-#define MAX_MSG_TYPES                    3
 #define MAX_NUM_DEVICE_CLASSES_SUPPORTED 7
 
 static const struct hv_guid
@@ -233,6 +232,11 @@ struct hyperv_service_callback hv_cb_utils[MAX_MSG_TYPES] = {
 };
 EXPORT_SYMBOL(hv_cb_utils);
 
+/* Counter of IC channels initialized */
+atomic_t hv_utils_initcnt = ATOMIC_INIT(0);
+EXPORT_SYMBOL(hv_utils_initcnt);
+
+
 /*
  * AllocVmbusChannel - Allocate and initialize a vmbus channel object
  */
@@ -373,22 +377,22 @@ static void VmbusChannelProcessOffer(void *context)
 		 * can cleanup properly
 		 */
 		newChannel->State = CHANNEL_OPEN_STATE;
-		cnt = 0;
 
-		while (cnt != MAX_MSG_TYPES) {
+		/* Open IC channels */
+		for (cnt = 0; cnt < MAX_MSG_TYPES; cnt++) {
 			if (memcmp(&newChannel->OfferMsg.Offer.InterfaceType,
 				   &hv_cb_utils[cnt].data,
-				   sizeof(struct hv_guid)) == 0) {
+				   sizeof(struct hv_guid)) == 0 &&
+			    VmbusChannelOpen(newChannel, 2 * PAGE_SIZE,
+					     2 * PAGE_SIZE, NULL, 0,
+					     hv_cb_utils[cnt].callback,
+					     newChannel) == 0) {
+				hv_cb_utils[cnt].channel = newChannel;
+				mb();
 				DPRINT_INFO(VMBUS, "%s",
 					    hv_cb_utils[cnt].log_msg);
-
-				if (VmbusChannelOpen(newChannel, 2 * PAGE_SIZE,
-						    2 * PAGE_SIZE, NULL, 0,
-						    hv_cb_utils[cnt].callback,
-						    newChannel) == 0)
-					hv_cb_utils[cnt].channel = newChannel;
+				atomic_inc(&hv_utils_initcnt);
 			}
-			cnt++;
 		}
 	}
 	DPRINT_EXIT(VMBUS);
diff --git a/drivers/staging/hv/hv_utils.c b/drivers/staging/hv/hv_utils.c
index 8a49aaf..f9826c7 100644
--- a/drivers/staging/hv/hv_utils.c
+++ b/drivers/staging/hv/hv_utils.c
@@ -253,7 +253,11 @@ static void heartbeat_onchannelcallback(void *context)
 
 static int __init init_hyperv_utils(void)
 {
-	printk(KERN_INFO "Registering HyperV Utility Driver\n");
+	printk(KERN_INFO "Registering HyperV Utility Driver...\n");
+
+	/* Wait until all IC channels are initialized */
+	while (atomic_read(&hv_utils_initcnt) < MAX_MSG_TYPES)
+		msleep(100);
 
 	hv_cb_utils[HV_SHUTDOWN_MSG].channel->OnChannelCallback =
 		&shutdown_onchannelcallback;
@@ -267,13 +271,12 @@ static int __init init_hyperv_utils(void)
 		&heartbeat_onchannelcallback;
 	hv_cb_utils[HV_HEARTBEAT_MSG].callback = &heartbeat_onchannelcallback;
 
+	printk(KERN_INFO "Registered HyperV Utility Driver.\n");
 	return 0;
 }
 
 static void exit_hyperv_utils(void)
 {
-	printk(KERN_INFO "De-Registered HyperV Utility Driver\n");
-
 	hv_cb_utils[HV_SHUTDOWN_MSG].channel->OnChannelCallback =
 		&chn_cb_negotiate;
 	hv_cb_utils[HV_SHUTDOWN_MSG].callback = &chn_cb_negotiate;
@@ -285,6 +288,8 @@ static void exit_hyperv_utils(void)
 	hv_cb_utils[HV_HEARTBEAT_MSG].channel->OnChannelCallback =
 		&chn_cb_negotiate;
 	hv_cb_utils[HV_HEARTBEAT_MSG].callback = &chn_cb_negotiate;
+
+	printk(KERN_INFO "De-Registered HyperV Utility Driver.\n");
 }
 
 module_init(init_hyperv_utils);
diff --git a/drivers/staging/hv/utils.h b/drivers/staging/hv/utils.h
index 7c07499..3291ab4 100644
--- a/drivers/staging/hv/utils.h
+++ b/drivers/staging/hv/utils.h
@@ -98,6 +98,10 @@ struct ictimesync_data{
 	u8 flags;
 } __attribute__((packed));
 
+
+/* Number of IC types supported */
+#define MAX_MSG_TYPES	3
+
 /* Index for each IC struct in array hv_cb_utils[] */
 #define HV_SHUTDOWN_MSG		0
 #define HV_TIMESYNC_MSG		1
@@ -115,5 +119,6 @@ extern void prep_negotiate_resp(struct icmsg_hdr *,
 				struct icmsg_negotiate *, u8 *);
 extern void chn_cb_negotiate(void *);
 extern struct hyperv_service_callback hv_cb_utils[];
+extern atomic_t hv_utils_initcnt;
 
 #endif /* __HV_UTILS_H_ */
-- 
1.6.3.2


[-- Attachment #2: 0518-1-Fix-race-condition-in-hv_utils-module-initialization.patch --]
[-- Type: application/octet-stream, Size: 4663 bytes --]

From: Haiyang Zhang <haiyangz@microsoft.com>

Subject: [PATCH 1/2] staging: hv: Fix race condition in hv_utils module initialization.
There is a possible race condition when hv_utils starts to load immediately
after hv_vmbus is loading - null pointer error could happen.
This patch added an atomic counter to ensure the hv_utils module initialization
happens after all vmbus IC channels are initialized.

Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: Hank Janssen <hjanssen@microsoft.com>

---
 drivers/staging/hv/channel_mgmt.c |   26 +++++++++++++++-----------
 drivers/staging/hv/hv_utils.c     |   11 ++++++++---
 drivers/staging/hv/utils.h        |    5 +++++
 3 files changed, 28 insertions(+), 14 deletions(-)

diff --git a/drivers/staging/hv/channel_mgmt.c b/drivers/staging/hv/channel_mgmt.c
index 3f53b4d..b5b6a70 100644
--- a/drivers/staging/hv/channel_mgmt.c
+++ b/drivers/staging/hv/channel_mgmt.c
@@ -33,7 +33,6 @@ struct vmbus_channel_message_table_entry {
 	void (*messageHandler)(struct vmbus_channel_message_header *msg);
 };
 
-#define MAX_MSG_TYPES                    3
 #define MAX_NUM_DEVICE_CLASSES_SUPPORTED 7
 
 static const struct hv_guid
@@ -233,6 +232,11 @@ struct hyperv_service_callback hv_cb_utils[MAX_MSG_TYPES] = {
 };
 EXPORT_SYMBOL(hv_cb_utils);
 
+/* Counter of IC channels initialized */
+atomic_t hv_utils_initcnt = ATOMIC_INIT(0);
+EXPORT_SYMBOL(hv_utils_initcnt);
+
+
 /*
  * AllocVmbusChannel - Allocate and initialize a vmbus channel object
  */
@@ -373,22 +377,22 @@ static void VmbusChannelProcessOffer(void *context)
 		 * can cleanup properly
 		 */
 		newChannel->State = CHANNEL_OPEN_STATE;
-		cnt = 0;
 
-		while (cnt != MAX_MSG_TYPES) {
+		/* Open IC channels */
+		for (cnt = 0; cnt < MAX_MSG_TYPES; cnt++) {
 			if (memcmp(&newChannel->OfferMsg.Offer.InterfaceType,
 				   &hv_cb_utils[cnt].data,
-				   sizeof(struct hv_guid)) == 0) {
+				   sizeof(struct hv_guid)) == 0 &&
+			    VmbusChannelOpen(newChannel, 2 * PAGE_SIZE,
+					     2 * PAGE_SIZE, NULL, 0,
+					     hv_cb_utils[cnt].callback,
+					     newChannel) == 0) {
+				hv_cb_utils[cnt].channel = newChannel;
+				mb();
 				DPRINT_INFO(VMBUS, "%s",
 					    hv_cb_utils[cnt].log_msg);
-
-				if (VmbusChannelOpen(newChannel, 2 * PAGE_SIZE,
-						    2 * PAGE_SIZE, NULL, 0,
-						    hv_cb_utils[cnt].callback,
-						    newChannel) == 0)
-					hv_cb_utils[cnt].channel = newChannel;
+				atomic_inc(&hv_utils_initcnt);
 			}
-			cnt++;
 		}
 	}
 	DPRINT_EXIT(VMBUS);
diff --git a/drivers/staging/hv/hv_utils.c b/drivers/staging/hv/hv_utils.c
index 8a49aaf..f9826c7 100644
--- a/drivers/staging/hv/hv_utils.c
+++ b/drivers/staging/hv/hv_utils.c
@@ -253,7 +253,11 @@ static void heartbeat_onchannelcallback(void *context)
 
 static int __init init_hyperv_utils(void)
 {
-	printk(KERN_INFO "Registering HyperV Utility Driver\n");
+	printk(KERN_INFO "Registering HyperV Utility Driver...\n");
+
+	/* Wait until all IC channels are initialized */
+	while (atomic_read(&hv_utils_initcnt) < MAX_MSG_TYPES)
+		msleep(100);
 
 	hv_cb_utils[HV_SHUTDOWN_MSG].channel->OnChannelCallback =
 		&shutdown_onchannelcallback;
@@ -267,13 +271,12 @@ static int __init init_hyperv_utils(void)
 		&heartbeat_onchannelcallback;
 	hv_cb_utils[HV_HEARTBEAT_MSG].callback = &heartbeat_onchannelcallback;
 
+	printk(KERN_INFO "Registered HyperV Utility Driver.\n");
 	return 0;
 }
 
 static void exit_hyperv_utils(void)
 {
-	printk(KERN_INFO "De-Registered HyperV Utility Driver\n");
-
 	hv_cb_utils[HV_SHUTDOWN_MSG].channel->OnChannelCallback =
 		&chn_cb_negotiate;
 	hv_cb_utils[HV_SHUTDOWN_MSG].callback = &chn_cb_negotiate;
@@ -285,6 +288,8 @@ static void exit_hyperv_utils(void)
 	hv_cb_utils[HV_HEARTBEAT_MSG].channel->OnChannelCallback =
 		&chn_cb_negotiate;
 	hv_cb_utils[HV_HEARTBEAT_MSG].callback = &chn_cb_negotiate;
+
+	printk(KERN_INFO "De-Registered HyperV Utility Driver.\n");
 }
 
 module_init(init_hyperv_utils);
diff --git a/drivers/staging/hv/utils.h b/drivers/staging/hv/utils.h
index 7c07499..3291ab4 100644
--- a/drivers/staging/hv/utils.h
+++ b/drivers/staging/hv/utils.h
@@ -98,6 +98,10 @@ struct ictimesync_data{
 	u8 flags;
 } __attribute__((packed));
 
+
+/* Number of IC types supported */
+#define MAX_MSG_TYPES	3
+
 /* Index for each IC struct in array hv_cb_utils[] */
 #define HV_SHUTDOWN_MSG		0
 #define HV_TIMESYNC_MSG		1
@@ -115,5 +119,6 @@ extern void prep_negotiate_resp(struct icmsg_hdr *,
 				struct icmsg_negotiate *, u8 *);
 extern void chn_cb_negotiate(void *);
 extern struct hyperv_service_callback hv_cb_utils[];
+extern atomic_t hv_utils_initcnt;
 
 #endif /* __HV_UTILS_H_ */
-- 
1.6.3.2


[-- Attachment #3: Type: text/plain, Size: 184 bytes --]

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization

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

end of thread, other threads:[~2010-05-19 22:21 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <1FB5E1D5CA062146B38059374562DF7266B8930E@TK5EX14MBXC128.redmond.corp.microsoft.com>
2010-05-19 16:10 ` [PATCH 1/2] staging: hv: Fix race condition in hv_utils module initialization Greg KH
2010-05-19 20:30   ` Haiyang Zhang
2010-05-19 20:39     ` Greg KH
2010-05-19 22:12       ` Haiyang Zhang
2010-05-19 22:21         ` Greg KH
2010-05-19 15:54 Haiyang Zhang

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).