[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <1FB5E1D5CA062146B38059374562DF7266B8AE7C@TK5EX14MBXC128.redmond.corp.microsoft.com>
Date: Fri, 21 May 2010 19:58:26 +0000
From: Haiyang Zhang <haiyangz@...rosoft.com>
To: "'linux-kernel@...r.kernel.org'" <linux-kernel@...r.kernel.org>,
"'devel@...verdev.osuosl.org'" <devel@...verdev.osuosl.org>,
"'virtualization@...ts.osdl.org'" <virtualization@...ts.osdl.org>,
"'gregkh@...e.de'" <gregkh@...e.de>
CC: Hank Janssen <hjanssen@...rosoft.com>
Subject: [PATCH 1/1] staging: hv: Fix race condition on IC channel
initialization
From: Haiyang Zhang <haiyangz@...rosoft.com>
Subject: staging: hv: Fix race condition on IC channel 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 all channels are ready before
vmbus_init() returns. So another module won't have any uninitialized channel.
Signed-off-by: Haiyang Zhang <haiyangz@...rosoft.com>
Signed-off-by: Hank Janssen <hjanssen@...rosoft.com>
---
drivers/staging/hv/channel_mgmt.c | 25 ++++++++++++++-----------
drivers/staging/hv/hv_utils.c | 7 ++++---
drivers/staging/hv/utils.h | 5 +++++
drivers/staging/hv/vmbus_drv.c | 5 +++++
4 files changed, 28 insertions(+), 14 deletions(-)
diff --git a/drivers/staging/hv/channel_mgmt.c b/drivers/staging/hv/channel_mgmt.c
index 3f53b4d..68dfa0f 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,10 @@ 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);
+
+
/*
* AllocVmbusChannel - Allocate and initialize a vmbus channel object
*/
@@ -373,22 +376,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..22f6f4a 100644
--- a/drivers/staging/hv/hv_utils.c
+++ b/drivers/staging/hv/hv_utils.c
@@ -253,7 +253,7 @@ 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");
hv_cb_utils[HV_SHUTDOWN_MSG].channel->OnChannelCallback =
&shutdown_onchannelcallback;
@@ -267,13 +267,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 +284,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_ */
diff --git a/drivers/staging/hv/vmbus_drv.c b/drivers/staging/hv/vmbus_drv.c
index c21731a..160ee92 100644
--- a/drivers/staging/hv/vmbus_drv.c
+++ b/drivers/staging/hv/vmbus_drv.c
@@ -31,6 +31,7 @@
#include "osd.h"
#include "logging.h"
#include "vmbus.h"
+#include "utils.h"
/* FIXME! We need to do this dynamically for PIC and APIC system */
@@ -1005,6 +1006,10 @@ static int __init vmbus_init(void)
ret = vmbus_bus_init(VmbusInitialize);
+ /* Wait until all IC channels are initialized */
+ while (atomic_read(&hv_utils_initcnt) < MAX_MSG_TYPES)
+ msleep(100);
+
DPRINT_EXIT(VMBUS_DRV);
return ret;
}
--
1.6.3.2
Download attachment "0521-Fix-race-condition-on-IC-channel-initialization.patch" of type "application/octet-stream" (5166 bytes)
Powered by blists - more mailing lists