Drivers: hv: util: move waiting for release to hv_utils_transport itself

Waiting for release_event in all three drivers introduced issues on release
as on_reset() hook is not always called. E.g. if the device was never
opened we will never get the completion.

Move the waiting code to hvutil_transport_destroy() and make sure it is
only called when the device is open. hvt->lock serialization should
guarantee the absence of races.

Fixes: 5a66fecbf6aa ("Drivers: hv: util: kvp: Fix a rescind processing issue")
Fixes: 20951c7535b5 ("Drivers: hv: util: Fcopy: Fix a rescind processing issue")
Fixes: d77044d142e9 ("Drivers: hv: util: Backup: Fix a rescind processing issue")
Reported-by: Dexuan Cui <decui@microsoft.com>
Tested-by: Dexuan Cui <decui@microsoft.com>
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/drivers/hv/hv_fcopy.c b/drivers/hv/hv_fcopy.c
index 9aee6014339dffc8627d173446e7bfe0dc06247c..a5596a642ed06b31beb5b2e19347e10b9abb0ed2 100644 (file)
--- a/drivers/hv/hv_fcopy.c
+++ b/drivers/hv/hv_fcopy.c
@@ -71,7 +71,6 @@ static DECLARE_WORK(fcopy_send_work, fcopy_send_data);
 static const char fcopy_devname[] = "vmbus/hv_fcopy";
 static u8 *recv_buffer;
 static struct hvutil_transport *hvt;
-static struct completion release_event;
 /*
  * This state maintains the version number registered by the daemon.
  */
@@ -331,7 +330,6 @@ static void fcopy_on_reset(void)
 
        if (cancel_delayed_work_sync(&fcopy_timeout_work))
                fcopy_respond_to_host(HV_E_FAIL);
-       complete(&release_event);
 }
 
 int hv_fcopy_init(struct hv_util_service *srv)
@@ -339,7 +337,6 @@ int hv_fcopy_init(struct hv_util_service *srv)
        recv_buffer = srv->recv_buffer;
        fcopy_transaction.recv_channel = srv->channel;
 
-       init_completion(&release_event);
        /*
         * When this driver loads, the user level daemon that
         * processes the host requests may not yet be running.
@@ -361,5 +358,4 @@ void hv_fcopy_deinit(void)
        fcopy_transaction.state = HVUTIL_DEVICE_DYING;
        cancel_delayed_work_sync(&fcopy_timeout_work);
        hvutil_transport_destroy(hvt);
-       wait_for_completion(&release_event);
 }

diff --git a/drivers/hv/hv_kvp.c b/drivers/hv/hv_kvp.c
index de263712e247c2b8c47f450f3d1b7eb387f1e073..a1adfe2cfb34244e8f86a48cc1143c0f439be709 100644 (file)
--- a/drivers/hv/hv_kvp.c
+++ b/drivers/hv/hv_kvp.c
@@ -101,7 +101,6 @@ static DECLARE_WORK(kvp_sendkey_work, kvp_send_key);
 static const char kvp_devname[] = "vmbus/hv_kvp";
 static u8 *recv_buffer;
 static struct hvutil_transport *hvt;
-static struct completion release_event;
 /*
  * Register the kernel component with the user-level daemon.
  * As part of this registration, pass the LIC version number.
@@ -714,7 +713,6 @@ static void kvp_on_reset(void)
        if (cancel_delayed_work_sync(&kvp_timeout_work))
                kvp_respond_to_host(NULL, HV_E_FAIL);
        kvp_transaction.state = HVUTIL_DEVICE_INIT;
-       complete(&release_event);
 }
 
 int
@@ -723,7 +721,6 @@ hv_kvp_init(struct hv_util_service *srv)
        recv_buffer = srv->recv_buffer;
        kvp_transaction.recv_channel = srv->channel;
 
-       init_completion(&release_event);
        /*
         * When this driver loads, the user level daemon that
         * processes the host requests may not yet be running.
@@ -747,5 +744,4 @@ void hv_kvp_deinit(void)
        cancel_delayed_work_sync(&kvp_timeout_work);
        cancel_work_sync(&kvp_sendkey_work);
        hvutil_transport_destroy(hvt);
-       wait_for_completion(&release_event);
 }

diff --git a/drivers/hv/hv_snapshot.c b/drivers/hv/hv_snapshot.c
index bcc03f0748d61cd5a8c9f31447b77fd8b2e7e13b..e659d1b94a5794f11272086374e45b5fc99a3df6 100644 (file)
--- a/drivers/hv/hv_snapshot.c
+++ b/drivers/hv/hv_snapshot.c
@@ -79,7 +79,6 @@ static int dm_reg_value;
 static const char vss_devname[] = "vmbus/hv_vss";
 static __u8 *recv_buffer;
 static struct hvutil_transport *hvt;
-static struct completion release_event;
 
 static void vss_timeout_func(struct work_struct *dummy);
 static void vss_handle_request(struct work_struct *dummy);
@@ -361,13 +360,11 @@ static void vss_on_reset(void)
        if (cancel_delayed_work_sync(&vss_timeout_work))
                vss_respond_to_host(HV_E_FAIL);
        vss_transaction.state = HVUTIL_DEVICE_INIT;
-       complete(&release_event);
 }
 
 int
 hv_vss_init(struct hv_util_service *srv)
 {
-       init_completion(&release_event);
        if (vmbus_proto_version < VERSION_WIN8_1) {
                pr_warn("Integration service 'Backup (volume snapshot)'"
                        " not supported on this host version.\n");
@@ -400,5 +397,4 @@ void hv_vss_deinit(void)
        cancel_delayed_work_sync(&vss_timeout_work);
        cancel_work_sync(&vss_handle_request_work);
        hvutil_transport_destroy(hvt);
-       wait_for_completion(&release_event);
 }

diff --git a/drivers/hv/hv_utils_transport.c b/drivers/hv/hv_utils_transport.c
index c235a95152671104cd7042a451b16ccd0d0d06ab..4402a71e23f7f7277c6d1561c4a8db5d46489b8d 100644 (file)
--- a/drivers/hv/hv_utils_transport.c
+++ b/drivers/hv/hv_utils_transport.c
@@ -182,10 +182,11 @@ static int hvt_op_release(struct inode *inode, struct file *file)
         * connects back.
         */
        hvt_reset(hvt);
-       mutex_unlock(&hvt->lock);
 
        if (mode_old == HVUTIL_TRANSPORT_DESTROY)
-               hvt_transport_free(hvt);
+               complete(&hvt->release);
+
+       mutex_unlock(&hvt->lock);
 
        return 0;
 }
@@ -304,6 +305,7 @@ struct hvutil_transport *hvutil_transport_init(const char *name,
 
        init_waitqueue_head(&hvt->outmsg_q);
        mutex_init(&hvt->lock);
+       init_completion(&hvt->release);
 
        spin_lock(&hvt_list_lock);
        list_add(&hvt->list, &hvt_list);
@@ -351,6 +353,8 @@ void hvutil_transport_destroy(struct hvutil_transport *hvt)
        if (hvt->cn_id.idx > 0 && hvt->cn_id.val > 0)
                cn_del_callback(&hvt->cn_id);
 
-       if (mode_old != HVUTIL_TRANSPORT_CHARDEV)
-               hvt_transport_free(hvt);
+       if (mode_old == HVUTIL_TRANSPORT_CHARDEV)
+               wait_for_completion(&hvt->release);
+
+       hvt_transport_free(hvt);
 }

diff --git a/drivers/hv/hv_utils_transport.h b/drivers/hv/hv_utils_transport.h
index d98f5225c3e693468fdb27ccc03de8d17e2a8576..79afb626e1668981beaa740b4e7d618f22ab194d 100644 (file)
--- a/drivers/hv/hv_utils_transport.h
+++ b/drivers/hv/hv_utils_transport.h
@@ -41,6 +41,7 @@ struct hvutil_transport {
        int outmsg_len;                     /* its length */
        wait_queue_head_t outmsg_q;         /* poll/read wait queue */
        struct mutex lock;                  /* protects struct members */
+       struct completion release;          /* synchronize with fd release */
 };
 
 struct hvutil_transport *hvutil_transport_init(const char *name,