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 <decui@microsoft.com> for observing this problem.

The current char-next branch is broken and this patch fixes
the bug.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index 611789139f9b799d1ea612a534c02ed3ddd8188a..5f8e47bf5ccca41098fe794c478c0638b3f7e7b8 100644 (file)
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -23,6 +23,7 @@
 #include <linux/kernel.h>
 #include <linux/sched.h>
 #include <linux/wait.h>
+#include <linux/delay.h>
 #include <linux/mm.h>
 #include <linux/slab.h>
 #include <linux/list.h>
@@ -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 @@ fw_error:
 
 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 583d7d42b46d84916615f3e7de2c17e28a0ce813..8bcd3071c84ffa7de1bc6a1fba4c93a03c5c11fd 100644 (file)
--- 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 88af4ec559c42c4d0fd4472783771446b800b922..63395896baf9b627e62892c3bbe72557c25e4179 100644 (file)
--- 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);