Skip to content

Commit

Permalink
Staging: hv: Re-implement the synchronization for util channels
Browse files Browse the repository at this point in the history
The util module expects that the util channels are fully initialized
when the module loads. To deal with the race condition which can result
in a NULL pointer dereferencing if the util module were to load before
all the util channels are fully initialized, in commit:

	commit: 8b5d6d3
	Author: Haiyang Zhang <haiyangz@microsoft.com>
	Date:   Fri May 28 23:22:44 2010 +000

code was introduced in the vmbus driver to ensure that all the
util channels were fully initialized before returning from the load
of the vmbus driver. This solution has several problems: if for whatever
reason, any util channel were to fail to initialize, vmbus driver would
wait indefinitely. We deal with this synchronization issue very differently
in this patch.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: Abhishek Kane <v-abkane@microsoft.com>
Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
  • Loading branch information
K. Y. Srinivasan authored and Greg Kroah-Hartman committed May 17, 2011
1 parent 31bceb8 commit b9d8e35
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 26 deletions.
24 changes: 19 additions & 5 deletions drivers/staging/hv/channel_mgmt.c
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,24 @@ void chn_cb_negotiate(void *context)
struct icmsg_hdr *icmsghdrp;
struct icmsg_negotiate *negop = NULL;

if (channel->util_index >= 0) {
/*
* This is a properly initialized util channel.
* Route this callback appropriately and setup state
* so that we don't need to reroute again.
*/
if (hv_cb_utils[channel->util_index].callback != NULL) {
/*
* The util driver has established a handler for
* this service; do the magic.
*/
channel->onchannel_callback =
hv_cb_utils[channel->util_index].callback;
(hv_cb_utils[channel->util_index].callback)(channel);
return;
}
}

buflen = PAGE_SIZE;
buf = kmalloc(buflen, GFP_ATOMIC);

Expand Down Expand Up @@ -217,7 +235,6 @@ struct hyperv_service_callback hv_cb_utils[MAX_MSG_TYPES] = {
0x31, 0x60, 0x0B, 0X0E, 0x13, 0x52, 0x34, 0x49,
0x81, 0x8B, 0x38, 0XD9, 0x0C, 0xED, 0x39, 0xDB
},
.callback = chn_cb_negotiate,
.log_msg = "Shutdown channel functionality initialized"
},

Expand All @@ -229,7 +246,6 @@ struct hyperv_service_callback hv_cb_utils[MAX_MSG_TYPES] = {
0x30, 0xe6, 0x27, 0x95, 0xae, 0xd0, 0x7b, 0x49,
0xad, 0xce, 0xe8, 0x0a, 0xb0, 0x17, 0x5c, 0xaf
},
.callback = chn_cb_negotiate,
.log_msg = "Timesync channel functionality initialized"
},
/* {57164f39-9115-4e78-ab55-382f3bd5422d} */
Expand All @@ -240,7 +256,6 @@ struct hyperv_service_callback hv_cb_utils[MAX_MSG_TYPES] = {
0x39, 0x4f, 0x16, 0x57, 0x15, 0x91, 0x78, 0x4e,
0xab, 0x55, 0x38, 0x2f, 0x3b, 0xd5, 0x42, 0x2d
},
.callback = chn_cb_negotiate,
.log_msg = "Heartbeat channel functionality initialized"
},
/* {A9A0F4E7-5A45-4d96-B827-8A841E8C03E6} */
Expand All @@ -250,7 +265,6 @@ struct hyperv_service_callback hv_cb_utils[MAX_MSG_TYPES] = {
0xe7, 0xf4, 0xa0, 0xa9, 0x45, 0x5a, 0x96, 0x4d,
0xb8, 0x27, 0x8a, 0x84, 0x1e, 0x8c, 0x3, 0xe6
},
.callback = chn_cb_negotiate,
.log_msg = "KVP channel functionality initialized"
},
};
Expand Down Expand Up @@ -428,7 +442,7 @@ static void vmbus_process_offer(struct work_struct *work)
sizeof(struct hv_guid)) == 0 &&
vmbus_open(newchannel, 2 * PAGE_SIZE,
2 * PAGE_SIZE, NULL, 0,
hv_cb_utils[cnt].callback,
chn_cb_negotiate,
newchannel) == 0) {
hv_cb_utils[cnt].channel = newchannel;
newchannel->util_index = cnt;
Expand Down
39 changes: 18 additions & 21 deletions drivers/staging/hv/hv_util.c
Original file line number Diff line number Diff line change
Expand Up @@ -256,22 +256,13 @@ static int __init init_hyperv_utils(void)
return -ENOMEM;
}

hv_cb_utils[HV_SHUTDOWN_MSG].channel->onchannel_callback =
&shutdown_onchannelcallback;
hv_cb_utils[HV_SHUTDOWN_MSG].callback = &shutdown_onchannelcallback;

hv_cb_utils[HV_TIMESYNC_MSG].channel->onchannel_callback =
&timesync_onchannelcallback;
hv_cb_utils[HV_TIMESYNC_MSG].callback = &timesync_onchannelcallback;

hv_cb_utils[HV_HEARTBEAT_MSG].channel->onchannel_callback =
&heartbeat_onchannelcallback;
hv_cb_utils[HV_HEARTBEAT_MSG].callback = &heartbeat_onchannelcallback;

hv_cb_utils[HV_KVP_MSG].channel->onchannel_callback =
&hv_kvp_onchannelcallback;


hv_cb_utils[HV_KVP_MSG].callback = &hv_kvp_onchannelcallback;

return 0;
}
Expand All @@ -280,20 +271,26 @@ static void exit_hyperv_utils(void)
{
pr_info("De-Registered HyperV Utility Driver\n");

hv_cb_utils[HV_SHUTDOWN_MSG].channel->onchannel_callback =
&chn_cb_negotiate;
hv_cb_utils[HV_SHUTDOWN_MSG].callback = &chn_cb_negotiate;
if (hv_cb_utils[HV_SHUTDOWN_MSG].channel != NULL)
hv_cb_utils[HV_SHUTDOWN_MSG].channel->onchannel_callback =
&chn_cb_negotiate;
hv_cb_utils[HV_SHUTDOWN_MSG].callback = NULL;

if (hv_cb_utils[HV_TIMESYNC_MSG].channel != NULL)
hv_cb_utils[HV_TIMESYNC_MSG].channel->onchannel_callback =
&chn_cb_negotiate;
hv_cb_utils[HV_TIMESYNC_MSG].callback = NULL;

hv_cb_utils[HV_TIMESYNC_MSG].channel->onchannel_callback =
&chn_cb_negotiate;
hv_cb_utils[HV_TIMESYNC_MSG].callback = &chn_cb_negotiate;
if (hv_cb_utils[HV_HEARTBEAT_MSG].channel != NULL)
hv_cb_utils[HV_HEARTBEAT_MSG].channel->onchannel_callback =
&chn_cb_negotiate;
hv_cb_utils[HV_HEARTBEAT_MSG].callback = NULL;

hv_cb_utils[HV_HEARTBEAT_MSG].channel->onchannel_callback =
&chn_cb_negotiate;
hv_cb_utils[HV_HEARTBEAT_MSG].callback = &chn_cb_negotiate;
if (hv_cb_utils[HV_KVP_MSG].channel != NULL)
hv_cb_utils[HV_KVP_MSG].channel->onchannel_callback =
&chn_cb_negotiate;
hv_cb_utils[HV_KVP_MSG].callback = NULL;

hv_cb_utils[HV_KVP_MSG].channel->onchannel_callback =
&chn_cb_negotiate;
hv_kvp_deinit();

kfree(shut_txf_buf);
Expand Down

0 comments on commit b9d8e35

Please sign in to comment.