From fde25d25dbd997067058f7d4c2ff31600157e6f2 Mon Sep 17 00:00:00 2001 From: "K. Y. Srinivasan" Date: Wed, 18 Mar 2015 12:29:21 -0700 Subject: [PATCH] Drivers: hv: vmbus: Perform device register in the per-channel work element This patch is a continuation of the rescind handling cleanup work. We cannot block in the global message handling work context especially if we are blocking waiting for the host to wake us up. I would like to thank Dexuan Cui for observing this problem. The current char-next branch is broken and this patch fixes the bug. Signed-off-by: K. Y. Srinivasan Signed-off-by: Greg Kroah-Hartman --- drivers/hv/channel_mgmt.c | 143 +++++++++++++++++++++++++++----------- drivers/hv/connection.c | 6 +- drivers/hv/hyperv_vmbus.h | 2 +- 3 files changed, 107 insertions(+), 44 deletions(-) diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c index 611789139f9b..5f8e47bf5ccc 100644 --- a/drivers/hv/channel_mgmt.c +++ b/drivers/hv/channel_mgmt.c @@ -23,6 +23,7 @@ #include #include #include +#include #include #include #include @@ -37,6 +38,10 @@ struct vmbus_channel_message_table_entry { void (*message_handler)(struct vmbus_channel_message_header *msg); }; +struct vmbus_rescind_work { + struct work_struct work; + struct vmbus_channel *channel; +}; /** * vmbus_prep_negotiate_resp() - Create default response for Hyper-V Negotiate message @@ -134,20 +139,6 @@ bool vmbus_prep_negotiate_resp(struct icmsg_hdr *icmsghdrp, EXPORT_SYMBOL_GPL(vmbus_prep_negotiate_resp); -static void vmbus_process_device_unregister(struct work_struct *work) -{ - struct device *dev; - struct vmbus_channel *channel = container_of(work, - struct vmbus_channel, - work); - - dev = get_device(&channel->device_obj->device); - if (dev) { - vmbus_device_unregister(channel->device_obj); - put_device(dev); - } -} - static void vmbus_sc_creation_cb(struct work_struct *work) { struct vmbus_channel *newchannel = container_of(work, @@ -220,6 +211,40 @@ static void free_channel(struct vmbus_channel *channel) queue_work(vmbus_connection.work_queue, &channel->work); } +static void process_rescind_fn(struct work_struct *work) +{ + struct vmbus_rescind_work *rc_work; + struct vmbus_channel *channel; + struct device *dev; + + rc_work = container_of(work, struct vmbus_rescind_work, work); + channel = rc_work->channel; + + /* + * We have already acquired a reference on the channel + * and so it cannot vanish underneath us. + * It is possible (while very unlikely) that we may + * get here while the processing of the initial offer + * is still not complete. Deal with this situation by + * just waiting until the channel is in the correct state. + */ + + while (channel->work.func != release_channel) + msleep(1000); + + if (channel->device_obj) { + dev = get_device(&channel->device_obj->device); + if (dev) { + vmbus_device_unregister(channel->device_obj); + put_device(dev); + } + } else { + hv_process_channel_removal(channel, + channel->offermsg.child_relid); + } + kfree(work); +} + static void percpu_channel_enq(void *arg) { struct vmbus_channel *channel = arg; @@ -282,6 +307,46 @@ void vmbus_free_channels(void) } } +static void vmbus_do_device_register(struct work_struct *work) +{ + struct hv_device *device_obj; + int ret; + unsigned long flags; + struct vmbus_channel *newchannel = container_of(work, + struct vmbus_channel, + work); + + ret = vmbus_device_register(newchannel->device_obj); + if (ret != 0) { + pr_err("unable to add child device object (relid %d)\n", + newchannel->offermsg.child_relid); + spin_lock_irqsave(&vmbus_connection.channel_lock, flags); + list_del(&newchannel->listentry); + device_obj = newchannel->device_obj; + newchannel->device_obj = NULL; + spin_unlock_irqrestore(&vmbus_connection.channel_lock, flags); + + if (newchannel->target_cpu != get_cpu()) { + put_cpu(); + smp_call_function_single(newchannel->target_cpu, + percpu_channel_deq, newchannel, true); + } else { + percpu_channel_deq(newchannel); + put_cpu(); + } + + kfree(device_obj); + if (!newchannel->rescind) { + free_channel(newchannel); + return; + } + } + /* + * The next state for this channel is to be freed. + */ + INIT_WORK(&newchannel->work, release_channel); +} + /* * vmbus_process_offer - Process the offer by creating a channel/device * associated with this offer @@ -291,7 +356,6 @@ static void vmbus_process_offer(struct vmbus_channel *newchannel) struct vmbus_channel *channel; bool fnew = true; bool enq = false; - int ret; unsigned long flags; /* Make sure this is a new offer */ @@ -393,16 +457,13 @@ static void vmbus_process_offer(struct vmbus_channel *newchannel) * Add the new device to the bus. This will kick off device-driver * binding which eventually invokes the device driver's AddDevice() * method. + * Invoke this call on the per-channel work context. + * Until we return from this function, rescind offer message + * cannot be processed as we are running on the global message + * handling work. */ - ret = vmbus_device_register(newchannel->device_obj); - if (ret != 0) { - pr_err("unable to add child device object (relid %d)\n", - newchannel->offermsg.child_relid); - - kfree(newchannel->device_obj); - goto err_deq_chan; - } - + INIT_WORK(&newchannel->work, vmbus_do_device_register); + queue_work(newchannel->controlwq, &newchannel->work); return; err_deq_chan: @@ -556,33 +617,31 @@ static void vmbus_onoffer_rescind(struct vmbus_channel_message_header *hdr) { struct vmbus_channel_rescind_offer *rescind; struct vmbus_channel *channel; - unsigned long flags; + struct vmbus_rescind_work *rc_work; rescind = (struct vmbus_channel_rescind_offer *)hdr; - channel = relid2channel(rescind->child_relid); + channel = relid2channel(rescind->child_relid, true); if (channel == NULL) { hv_process_channel_removal(NULL, rescind->child_relid); return; } - spin_lock_irqsave(&channel->lock, flags); - channel->rescind = true; - spin_unlock_irqrestore(&channel->lock, flags); - - if (channel->device_obj) { - /* - * We will have to unregister this device from the - * driver core. Do this in the per-channel work context. - * Note that we are currently executing on the global - * workq for handling messages from the host. - */ - INIT_WORK(&channel->work, vmbus_process_device_unregister); - queue_work(channel->controlwq, &channel->work); - } else { - hv_process_channel_removal(channel, - channel->offermsg.child_relid); + /* + * We have acquired a reference on the channel and have posted + * the rescind state. Perform further cleanup in a work context + * that is different from the global work context in which + * we process messages from the host (we are currently executing + * on that global context. + */ + rc_work = kzalloc(sizeof(struct vmbus_rescind_work), GFP_KERNEL); + if (!rc_work) { + pr_err("Unable to allocate memory for rescind processing "); + return; } + rc_work->channel = channel; + INIT_WORK(&rc_work->work, process_rescind_fn); + schedule_work(&rc_work->work); } /* diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c index 583d7d42b46d..8bcd3071c84f 100644 --- a/drivers/hv/connection.c +++ b/drivers/hv/connection.c @@ -270,7 +270,7 @@ static struct vmbus_channel *pcpu_relid2channel(u32 relid) * relid2channel - Get the channel object given its * child relative id (ie channel id) */ -struct vmbus_channel *relid2channel(u32 relid) +struct vmbus_channel *relid2channel(u32 relid, bool rescind) { struct vmbus_channel *channel; struct vmbus_channel *found_channel = NULL; @@ -282,6 +282,8 @@ struct vmbus_channel *relid2channel(u32 relid) list_for_each_entry(channel, &vmbus_connection.chn_list, listentry) { if (channel->offermsg.child_relid == relid) { found_channel = channel; + if (rescind) + found_channel->rescind = true; break; } else if (!list_empty(&channel->sc_list)) { /* @@ -292,6 +294,8 @@ struct vmbus_channel *relid2channel(u32 relid) sc_list); if (cur_sc->offermsg.child_relid == relid) { found_channel = cur_sc; + if (rescind) + found_channel->rescind = true; break; } } diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h index 88af4ec559c4..63395896baf9 100644 --- a/drivers/hv/hyperv_vmbus.h +++ b/drivers/hv/hyperv_vmbus.h @@ -698,7 +698,7 @@ void vmbus_device_unregister(struct hv_device *device_obj); /* VmbusChildDeviceDestroy( */ /* struct hv_device *); */ -struct vmbus_channel *relid2channel(u32 relid); +struct vmbus_channel *relid2channel(u32 relid, bool rescind); void vmbus_free_channels(void);