Quantcast

[SRU][Xenial][Zesty][PATCH 0/1] Drivers: hv: util: move waiting for release to hv_utils_transport itself

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[SRU][Xenial][Zesty][PATCH 0/1] Drivers: hv: util: move waiting for release to hv_utils_transport itself

Joseph Salisbury-3
BugLink: http://bugs.launchpad.net/bugs/1682561

== SRU Justification ==
Microsoft is observing call traces with the current proposed kernels
that are shown in the bug description.  Commit e9c18ae6eb is the fix
to this issue.  This commit is needed in X, Y and Z.  However, the Yakkey SRU
will be sent separatly from X and Z.  This is because Y does not have commit
0d325d642c648 and X and Z do.  The cherry pick is clean in X, Y and Z, but the
Y resulting patch is different because it doesn not have 0d325d642c648.

This commit was included in mainline in 4.11-rc4.

== Fix ==
commit e9c18ae6eb2b312f16c63e34b43ea23926daa398
Author: Vitaly Kuznetsov <[hidden email]>
Date:   Sat Mar 4 18:13:59 2017 -0700

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


== Test Case ==
A test kernel was built with this patch and tested by the original bug reporter.
The bug reporter states the test kernel resolved the bug.

Vitaly Kuznetsov (1):
  Drivers: hv: util: move waiting for release to hv_utils_transport
    itself

 drivers/hv/hv_fcopy.c           |  4 ----
 drivers/hv/hv_kvp.c             |  4 ----
 drivers/hv/hv_snapshot.c        |  4 ----
 drivers/hv/hv_utils_transport.c | 12 ++++++++----
 drivers/hv/hv_utils_transport.h |  1 +
 5 files changed, 9 insertions(+), 16 deletions(-)

--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[SRU][Xenial][Zesty][PATCH 1/1] Drivers: hv: util: move waiting for release to hv_utils_transport itself

Joseph Salisbury-3
From: Vitaly Kuznetsov <[hidden email]>

BugLink: http://bugs.launchpad.net/bugs/1682561

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 <[hidden email]>
Tested-by: Dexuan Cui <[hidden email]>
Signed-off-by: Vitaly Kuznetsov <[hidden email]>
Signed-off-by: K. Y. Srinivasan <[hidden email]>
Signed-off-by: Greg Kroah-Hartman <[hidden email]>
(cherry picked from commit e9c18ae6eb2b312f16c63e34b43ea23926daa398)
Signed-off-by: Joseph Salisbury <[hidden email]>
---
 drivers/hv/hv_fcopy.c           |  4 ----
 drivers/hv/hv_kvp.c             |  4 ----
 drivers/hv/hv_snapshot.c        |  4 ----
 drivers/hv/hv_utils_transport.c | 12 ++++++++----
 drivers/hv/hv_utils_transport.h |  1 +
 5 files changed, 9 insertions(+), 16 deletions(-)

diff --git a/drivers/hv/hv_fcopy.c b/drivers/hv/hv_fcopy.c
index e47d8c9..8b2ba98 100644
--- a/drivers/hv/hv_fcopy.c
+++ b/drivers/hv/hv_fcopy.c
@@ -61,7 +61,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.
  */
@@ -318,7 +317,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)
@@ -326,7 +324,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.
@@ -348,5 +345,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 3abfc59..5e1fdc8 100644
--- a/drivers/hv/hv_kvp.c
+++ b/drivers/hv/hv_kvp.c
@@ -88,7 +88,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.
@@ -717,7 +716,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
@@ -726,7 +724,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.
@@ -750,5 +747,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 ab02966..c42ae82 100644
--- a/drivers/hv/hv_snapshot.c
+++ b/drivers/hv/hv_snapshot.c
@@ -69,7 +69,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);
@@ -335,13 +334,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");
@@ -372,5 +369,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 c235a95..4402a71 100644
--- 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 d98f522..79afb62 100644
--- 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,
--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

ACK: [SRU][Xenial][Zesty][PATCH 0/1] Drivers: hv: util: move waiting for release to hv_utils_transport itself

brad.figg
In reply to this post by Joseph Salisbury-3
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [SRU][Xenial][Zesty][PATCH 0/1] Drivers: hv: util: move waiting for release to hv_utils_transport itself

Colin Ian King-2
In reply to this post by Joseph Salisbury-3
On 13/04/17 22:09, Joseph Salisbury wrote:

> BugLink: http://bugs.launchpad.net/bugs/1682561
>
> == SRU Justification ==
> Microsoft is observing call traces with the current proposed kernels
> that are shown in the bug description.  Commit e9c18ae6eb is the fix
> to this issue.  This commit is needed in X, Y and Z.  However, the Yakkey SRU
> will be sent separatly from X and Z.  This is because Y does not have commit
> 0d325d642c648 and X and Z do.  The cherry pick is clean in X, Y and Z, but the
> Y resulting patch is different because it doesn not have 0d325d642c648.
>
> This commit was included in mainline in 4.11-rc4.
>
> == Fix ==
> commit e9c18ae6eb2b312f16c63e34b43ea23926daa398
> Author: Vitaly Kuznetsov <[hidden email]>
> Date:   Sat Mar 4 18:13:59 2017 -0700
>
>     Drivers: hv: util: move waiting for release to hv_utils_transport itself
>
>
> == Test Case ==
> A test kernel was built with this patch and tested by the original bug reporter.
> The bug reporter states the test kernel resolved the bug.
>
> Vitaly Kuznetsov (1):
>   Drivers: hv: util: move waiting for release to hv_utils_transport
>     itself
>
>  drivers/hv/hv_fcopy.c           |  4 ----
>  drivers/hv/hv_kvp.c             |  4 ----
>  drivers/hv/hv_snapshot.c        |  4 ----
>  drivers/hv/hv_utils_transport.c | 12 ++++++++----
>  drivers/hv/hv_utils_transport.h |  1 +
>  5 files changed, 9 insertions(+), 16 deletions(-)
>
The fix is an upstream cherry pick, just touches this specific driver
and hence has limited regression potential, plus it has positive test
results. So...

Acked-by: Colin Ian King <[hidden email]>

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

ACK/cmnt: [SRU][Xenial][Zesty][PATCH 1/1] Drivers: hv: util: move waiting for release to hv_utils_transport itself

Stefan Bader-2
In reply to this post by Joseph Salisbury-3
Same notes as for the yakkety patch.

-Stefan


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

signature.asc (836 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: ACK/cmnt: [SRU][Xenial][Zesty][PATCH 1/1] Drivers: hv: util: move waiting for release to hv_utils_transport itself

Joshua R. Poulson
Missing this fix will definitely cause a functionality regression once
this kernel exits proposed.

On Tue, Apr 18, 2017 at 2:07 AM, Stefan Bader
<[hidden email]> wrote:

> Same notes as for the yakkety patch.
>
> -Stefan
>
>
> --
> kernel-team mailing list
> [hidden email]
> https://lists.ubuntu.com/mailman/listinfo/kernel-team
>

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

APPLIED: [SRU][Xenial][Zesty][PATCH 1/1] Drivers: hv: util: move waiting for release to hv_utils_transport itself

Stefan Bader-2
In reply to this post by Joseph Salisbury-3
Applied to Zesty/Xenial when re-spinning


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

signature.asc (836 bytes) Download Attachment
Loading...