diff mbox series

[1/2] vhost: remove mutex ops in vhost_set_backend_features

Message ID 20200907105220.27776-2-lingshan.zhu@intel.com
State New
Headers show
Series [1/2] vhost: remove mutex ops in vhost_set_backend_features | expand

Commit Message

Zhu Lingshan Sept. 7, 2020, 10:52 a.m. UTC
In vhost_vdpa ioctl SET_BACKEND_FEATURES path, currect code
would try to acquire vhost dev mutex twice
(first shown in vhost_vdpa_unlocked_ioctl), which can lead
to a dead lock issue.
This commit removed mutex operations in vhost_set_backend_features.
As a compensation for vhost_net, a followinig commit will add
needed mutex lock/unlock operations in a new function
vhost_net_set_backend_features() which is a wrap of
vhost_set_backend_features().

Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>
---
 drivers/vhost/vhost.c | 2 --
 1 file changed, 2 deletions(-)

Comments

Michael S. Tsirkin Sept. 21, 2020, 12:35 p.m. UTC | #1
On Tue, Sep 08, 2020 at 09:00:19PM +0800, Zhu, Lingshan wrote:
> 

> On 9/8/2020 8:05 PM, Michael S. Tsirkin wrote:

> 

>     On Mon, Sep 07, 2020 at 06:52:19PM +0800, Zhu Lingshan wrote:

> 

>         In vhost_vdpa ioctl SET_BACKEND_FEATURES path, currect code

>         would try to acquire vhost dev mutex twice

>         (first shown in vhost_vdpa_unlocked_ioctl), which can lead

>         to a dead lock issue.

>         This commit removed mutex operations in vhost_set_backend_features.

>         As a compensation for vhost_net, a followinig commit will add

>         needed mutex lock/unlock operations in a new function

>         vhost_net_set_backend_features() which is a wrap of

>         vhost_set_backend_features().

> 

>         Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>

> 

>     I think you need to squash these two or reorder, we can't first

>     make code racy then fix it up.

> 

> OK, I will send a V2 series with Jason's fixes tomorrow (handle SET/GET_BACKEND_FEATURES in vhost_vdpa ioctl than vring ioctl).

> 

> Thanks,

> BR

> Zhu Lingshan



this never materialized ...

> 

> 

>         ---

>          drivers/vhost/vhost.c | 2 --

>          1 file changed, 2 deletions(-)

> 

>         diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c

>         index b45519ca66a7..e03c9e6f058f 100644

>         --- a/drivers/vhost/vhost.c

>         +++ b/drivers/vhost/vhost.c

>         @@ -2591,14 +2591,12 @@ void vhost_set_backend_features(struct vhost_dev *dev, u64 features)

>                 struct vhost_virtqueue *vq;

>                 int i;

> 

>         -       mutex_lock(&dev->mutex);

>                 for (i = 0; i < dev->nvqs; ++i) {

>                         vq = dev->vqs[i];

>                         mutex_lock(&vq->mutex);

>                         vq->acked_backend_features = features;

>                         mutex_unlock(&vq->mutex);

>                 }

>         -       mutex_unlock(&dev->mutex);

>          }

>          EXPORT_SYMBOL_GPL(vhost_set_backend_features);

> 

>         --

>         2.18.4

>
diff mbox series

Patch

diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index b45519ca66a7..e03c9e6f058f 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -2591,14 +2591,12 @@  void vhost_set_backend_features(struct vhost_dev *dev, u64 features)
 	struct vhost_virtqueue *vq;
 	int i;
 
-	mutex_lock(&dev->mutex);
 	for (i = 0; i < dev->nvqs; ++i) {
 		vq = dev->vqs[i];
 		mutex_lock(&vq->mutex);
 		vq->acked_backend_features = features;
 		mutex_unlock(&vq->mutex);
 	}
-	mutex_unlock(&dev->mutex);
 }
 EXPORT_SYMBOL_GPL(vhost_set_backend_features);