d/freebsd-skq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

hyperv: vmbus: run non-blocking message handlers in vmbus_msg_swintr()

Browse Source 
We'll remove the per-channel control_work_queue because it can't properly
do serialization of message handling, e.g., when there are 2 NIC devices,
vmbus_channel_on_offer() -> hv_queue_work_item() has a race condition:
for an SMP VM, vmbus_channel_process_offer() can run concurrently on
different CPUs and if the second NIC's
vmbus_channel_process_offer() -> hv_vmbus_child_device_register() runs
first, the second NIC's name will be hn0 and the first NIC's name will
be hn1!

We can fix the race condition by removing the per-channel control_work_queue
and run all the message handlers in the global
hv_vmbus_g_connection.work_queue -- we'll do this in the next patch.

With the coming next patch, we have to run the non-blocking handlers
directly in the kernel thread vmbus_msg_swintr(), because the special
handling of sub-channel: when a sub-channel (e.g., of the storvsc driver)
is received and being handled in vmbus_channel_on_offer() running on the
global hv_vmbus_g_connection.work_queue, vmbus_channel_process_offer()
invokes channel->sc_creation_callback, i.e., storvsc_handle_sc_creation,
and the callback will invoke hv_vmbus_channel_open() -> hv_vmbus_post_message
and expect a further reply from the host, but the handling of the further
messag can't be done because the current message's handling hasn't finished
yet; as result, hv_vmbus_channel_open() -> sema_timedwait() will time out
and th device can't work.

Also renamed the handler type from hv_pfn_channel_msg_handler to
vmbus_msg_handler: the 'pfn' and 'channel' in the old name make no sense.

Submitted by:	Dexuan Cui <decui microsoft com>
Reviewed by:	royger
MFC after:	2 weeks
Differential Revision:	https://reviews.freebsd.org/D4596
This commit is contained in:
Xin LI 2015-12-29 08:19:43 +00:00
parent 47f175b846
commit fcf8d36c46
3 changed files with 64 additions and 29 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

52
sys/dev/hyperv/vmbus/hv_channel_mgmt.c
View File

@ -31,13 +31,6 @@
#include "hv_vmbus_priv.h" #include "hv_vmbus_priv.h"
typedef void (*hv_pfn_channel_msg_handler)(hv_vmbus_channel_msg_header* msg);
typedef struct hv_vmbus_channel_msg_table_entry {
hv_vmbus_channel_msg_type messageType;
hv_pfn_channel_msg_handler messageHandler;
} hv_vmbus_channel_msg_table_entry;
/* /*
* Internal functions * Internal functions
*/ */
@ -55,29 +48,40 @@ static void vmbus_channel_on_version_response(hv_vmbus_channel_msg_header* hdr);
*/ */
hv_vmbus_channel_msg_table_entry hv_vmbus_channel_msg_table_entry
g_channel_message_table[HV_CHANNEL_MESSAGE_COUNT] = { g_channel_message_table[HV_CHANNEL_MESSAGE_COUNT] = {
{ HV_CHANNEL_MESSAGE_INVALID, NULL }, { HV_CHANNEL_MESSAGE_INVALID,
{ HV_CHANNEL_MESSAGE_OFFER_CHANNEL, vmbus_channel_on_offer }, 0, NULL },
{ HV_CHANNEL_MESSAGE_OFFER_CHANNEL,
0, vmbus_channel_on_offer },
{ HV_CHANNEL_MESSAGE_RESCIND_CHANNEL_OFFER, { HV_CHANNEL_MESSAGE_RESCIND_CHANNEL_OFFER,
vmbus_channel_on_offer_rescind }, 0, vmbus_channel_on_offer_rescind },
{ HV_CHANNEL_MESSAGE_REQUEST_OFFERS, NULL }, { HV_CHANNEL_MESSAGE_REQUEST_OFFERS,
0, NULL },
{ HV_CHANNEL_MESSAGE_ALL_OFFERS_DELIVERED, { HV_CHANNEL_MESSAGE_ALL_OFFERS_DELIVERED,
vmbus_channel_on_offers_delivered }, 1, vmbus_channel_on_offers_delivered },
{ HV_CHANNEL_MESSAGE_OPEN_CHANNEL, NULL }, { HV_CHANNEL_MESSAGE_OPEN_CHANNEL,
0, NULL },
{ HV_CHANNEL_MESSAGE_OPEN_CHANNEL_RESULT, { HV_CHANNEL_MESSAGE_OPEN_CHANNEL_RESULT,
vmbus_channel_on_open_result }, 1, vmbus_channel_on_open_result },
{ HV_CHANNEL_MESSAGE_CLOSE_CHANNEL, NULL }, { HV_CHANNEL_MESSAGE_CLOSE_CHANNEL,
{ HV_CHANNEL_MESSAGEL_GPADL_HEADER, NULL }, 0, NULL },
{ HV_CHANNEL_MESSAGE_GPADL_BODY, NULL }, { HV_CHANNEL_MESSAGEL_GPADL_HEADER,
0, NULL },
{ HV_CHANNEL_MESSAGE_GPADL_BODY,
0, NULL },
{ HV_CHANNEL_MESSAGE_GPADL_CREATED, { HV_CHANNEL_MESSAGE_GPADL_CREATED,
vmbus_channel_on_gpadl_created }, 1, vmbus_channel_on_gpadl_created },
{ HV_CHANNEL_MESSAGE_GPADL_TEARDOWN, NULL }, { HV_CHANNEL_MESSAGE_GPADL_TEARDOWN,
0, NULL },
{ HV_CHANNEL_MESSAGE_GPADL_TORNDOWN, { HV_CHANNEL_MESSAGE_GPADL_TORNDOWN,
vmbus_channel_on_gpadl_torndown }, 1, vmbus_channel_on_gpadl_torndown },
{ HV_CHANNEL_MESSAGE_REL_ID_RELEASED, NULL }, { HV_CHANNEL_MESSAGE_REL_ID_RELEASED,
{ HV_CHANNEL_MESSAGE_INITIATED_CONTACT, NULL }, 0, NULL },
{ HV_CHANNEL_MESSAGE_INITIATED_CONTACT,
0, NULL },
{ HV_CHANNEL_MESSAGE_VERSION_RESPONSE, { HV_CHANNEL_MESSAGE_VERSION_RESPONSE,
vmbus_channel_on_version_response }, 1, vmbus_channel_on_version_response },
{ HV_CHANNEL_MESSAGE_UNLOAD, NULL } { HV_CHANNEL_MESSAGE_UNLOAD,
0, NULL }
}; };

31
sys/dev/hyperv/vmbus/hv_vmbus_drv_freebsd.c
View File

@ -76,8 +76,12 @@ vmbus_msg_swintr(void *arg)
{ {
int cpu; int cpu;
void* page_addr; void* page_addr;
hv_vmbus_channel_msg_header *hdr;
hv_vmbus_channel_msg_table_entry *entry;
hv_vmbus_channel_msg_type msg_type;
hv_vmbus_message* msg; hv_vmbus_message* msg;
hv_vmbus_message* copied; hv_vmbus_message* copied;
static bool warned = false;
cpu = (int)(long)arg; cpu = (int)(long)arg;
KASSERT(cpu <= mp_maxid, ("VMBUS: vmbus_msg_swintr: " KASSERT(cpu <= mp_maxid, ("VMBUS: vmbus_msg_swintr: "
@ -87,9 +91,24 @@ vmbus_msg_swintr(void *arg)
msg = (hv_vmbus_message*) page_addr + HV_VMBUS_MESSAGE_SINT; msg = (hv_vmbus_message*) page_addr + HV_VMBUS_MESSAGE_SINT;
for (;;) { for (;;) {
if (msg->header.message_type == HV_MESSAGE_TYPE_NONE) { if (msg->header.message_type == HV_MESSAGE_TYPE_NONE)
break; /* no message */ break; /* no message */
} else {
hdr = (hv_vmbus_channel_msg_header *)msg->u.payload;
msg_type = hdr->message_type;
if (msg_type >= HV_CHANNEL_MESSAGE_COUNT && !warned) {
warned = true;
printf("VMBUS: unknown message type = %d\n", msg_type);
goto handled;
}
entry = &g_channel_message_table[msg_type];
if (entry->handler_no_sleep)
entry->messageHandler(hdr);
else {
copied = malloc(sizeof(hv_vmbus_message), copied = malloc(sizeof(hv_vmbus_message),
M_DEVBUF, M_NOWAIT); M_DEVBUF, M_NOWAIT);
KASSERT(copied != NULL, KASSERT(copied != NULL,
@ -97,11 +116,13 @@ vmbus_msg_swintr(void *arg)
" hv_vmbus_message!")); " hv_vmbus_message!"));
if (copied == NULL) if (copied == NULL)
continue; continue;
memcpy(copied, msg, sizeof(hv_vmbus_message)); memcpy(copied, msg, sizeof(hv_vmbus_message));
hv_queue_work_item(hv_vmbus_g_connection.work_queue, hv_queue_work_item(hv_vmbus_g_connection.work_queue,
hv_vmbus_on_channel_message, copied); hv_vmbus_on_channel_message,
} copied);
}
handled:
msg->header.message_type = HV_MESSAGE_TYPE_NONE; msg->header.message_type = HV_MESSAGE_TYPE_NONE;
/* /*

10
sys/dev/hyperv/vmbus/hv_vmbus_priv.h
View File

@ -586,6 +586,16 @@ typedef enum {
extern hv_vmbus_context hv_vmbus_g_context; extern hv_vmbus_context hv_vmbus_g_context;
extern hv_vmbus_connection hv_vmbus_g_connection; extern hv_vmbus_connection hv_vmbus_g_connection;
typedef void (*vmbus_msg_handler)(hv_vmbus_channel_msg_header *msg);
typedef struct hv_vmbus_channel_msg_table_entry {
hv_vmbus_channel_msg_type messageType;
bool handler_no_sleep; /* true: the handler doesn't sleep */
vmbus_msg_handler messageHandler;
} hv_vmbus_channel_msg_table_entry;
extern hv_vmbus_channel_msg_table_entry g_channel_message_table[];
/* /*
* Private, VM Bus functions * Private, VM Bus functions