Messages in this thread | | | From | KY Srinivasan <> | Subject | RE: [PATCH 1/1] Drivers: hv: util: on deinit, don't wait the release event, if we shouldn't | Date | Sun, 5 Mar 2017 00:03:21 +0000 |
| |
> -----Original Message----- > From: kys@exchange.microsoft.com [mailto:kys@exchange.microsoft.com] > Sent: Monday, February 27, 2017 3:18 PM > To: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org; > devel@linuxdriverproject.org; olaf@aepfle.de; apw@canonical.com; > vkuznets@redhat.com; jasowang@redhat.com; > leann.ogasawara@canonical.com > Cc: Dexuan Cui <decui@microsoft.com>; KY Srinivasan > <kys@microsoft.com>; Haiyang Zhang <haiyangz@microsoft.com>; Stephen > Hemminger <sthemmin@microsoft.com> > Subject: [PATCH 1/1] Drivers: hv: util: on deinit, don't wait the release event, > if we shouldn't > > This sender failed our fraud detection&nbs > p;checks and may not be who they& > nbsp;appear to be. Learn about <a > href="http://aka.ms/LearnAboutSpoofing">spoofing</a> > > From: Dexuan Cui <decui@microsoft.com> > > If the daemon is NOT running at all, when we disable the util device from > Hyper-V Manager (or sometimes the host can rescind a util device and then > re-offer it), we'll hang in util_remove -> hv_kvp_deinit -> > wait_for_completion(&release_event), because this code path doesn't run: > hvt_op_release -> ... -> kvp_on_reset -> complete(&release_event). > > Due to this, we even can't reboot the VM properly. > > The patch tracks if the dev file is opened or not, and we only need to > wait if it's opened. > > Fixes: 5a66fecbf6aa ("Drivers: hv: util: kvp: Fix a rescind processing issue") > > Signed-off-by: Dexuan Cui <decui@microsoft.com> > Cc: Vitaly Kuznetsov <vkuznets@redhat.com> > Cc: "K. Y. Srinivasan" <kys@microsoft.com> > Cc: Haiyang Zhang <haiyangz@microsoft.com> > Cc: Stephen Hemminger <sthemmin@microsoft.com> > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com> > --- > drivers/hv/hv_fcopy.c | 5 ++++- > drivers/hv/hv_kvp.c | 6 +++++- > drivers/hv/hv_snapshot.c | 5 ++++- > drivers/hv/hv_utils_transport.c | 2 ++ > drivers/hv/hv_utils_transport.h | 1 + > 5 files changed, 16 insertions(+), 3 deletions(-) > > diff --git a/drivers/hv/hv_fcopy.c b/drivers/hv/hv_fcopy.c > index 9aee601..545cf43 100644 > --- a/drivers/hv/hv_fcopy.c > +++ b/drivers/hv/hv_fcopy.c > @@ -358,8 +358,11 @@ int hv_fcopy_init(struct hv_util_service *srv) > > void hv_fcopy_deinit(void) > { > + bool wait = hvt->dev_opened; > + > fcopy_transaction.state = HVUTIL_DEVICE_DYING; > cancel_delayed_work_sync(&fcopy_timeout_work); > hvutil_transport_destroy(hvt); > - wait_for_completion(&release_event); > + if (wait) > + wait_for_completion(&release_event); > } > diff --git a/drivers/hv/hv_kvp.c b/drivers/hv/hv_kvp.c > index de26371..15c7873 100644 > --- a/drivers/hv/hv_kvp.c > +++ b/drivers/hv/hv_kvp.c > @@ -742,10 +742,14 @@ static void kvp_on_reset(void) > > void hv_kvp_deinit(void) > { > + bool wait = hvt->dev_opened; > + > kvp_transaction.state = HVUTIL_DEVICE_DYING; > cancel_delayed_work_sync(&kvp_host_handshake_work); > cancel_delayed_work_sync(&kvp_timeout_work); > cancel_work_sync(&kvp_sendkey_work); > hvutil_transport_destroy(hvt); > - wait_for_completion(&release_event); > + > + if (wait) > + wait_for_completion(&release_event); > } > diff --git a/drivers/hv/hv_snapshot.c b/drivers/hv/hv_snapshot.c > index bcc03f0..3847f19 100644 > --- a/drivers/hv/hv_snapshot.c > +++ b/drivers/hv/hv_snapshot.c > @@ -396,9 +396,12 @@ static void vss_on_reset(void) > > void hv_vss_deinit(void) > { > + bool wait = hvt->dev_opened; > + > vss_transaction.state = HVUTIL_DEVICE_DYING; > cancel_delayed_work_sync(&vss_timeout_work); > cancel_work_sync(&vss_handle_request_work); > hvutil_transport_destroy(hvt); > - wait_for_completion(&release_event); > + if (wait) > + wait_for_completion(&release_event); > } > diff --git a/drivers/hv/hv_utils_transport.c b/drivers/hv/hv_utils_transport.c > index c235a95..05e0648 100644 > --- a/drivers/hv/hv_utils_transport.c > +++ b/drivers/hv/hv_utils_transport.c > @@ -153,6 +153,7 @@ static int hvt_op_open(struct inode *inode, struct file > *file) > > if (issue_reset) > hvt_reset(hvt); > + hvt->dev_opened = (hvt->mode == HVUTIL_TRANSPORT_CHARDEV) > && !ret; > > mutex_unlock(&hvt->lock); > > @@ -182,6 +183,7 @@ static int hvt_op_release(struct inode *inode, struct > file *file) > * connects back. > */ > hvt_reset(hvt); > + hvt->dev_opened = false; > mutex_unlock(&hvt->lock); > > if (mode_old == HVUTIL_TRANSPORT_DESTROY) > diff --git a/drivers/hv/hv_utils_transport.h b/drivers/hv/hv_utils_transport.h > index d98f522..9871283 100644 > --- a/drivers/hv/hv_utils_transport.h > +++ b/drivers/hv/hv_utils_transport.h > @@ -32,6 +32,7 @@ struct hvutil_transport { > int mode; /* hvutil_transport_mode */ > struct file_operations fops; /* file operations */ > struct miscdevice mdev; /* misc device */ > + bool dev_opened; /* Is the device opened? */ > struct cb_id cn_id; /* CN_*_IDX/CN_*_VAL */ > struct list_head list; /* hvt_list */ > int (*on_msg)(void *, int); /* callback on new user message */ > -- > 1.7.1
Greg,
Please drop this patch.
K. Y
| |