search for: virtio_fs_remov

On Thu, Sep 05, 2019 at 03:48:57PM -0400, Vivek Goyal wrote: > It is possible that a mount is in progress and device is being removed at > the same time. Use virtio_fs_mutex to avoid races. > > This also takes care of bunch of races and removes some TODO items. > > Signed-off-by: Vivek Goyal <vgoyal at redhat.com> > --- > fs/fuse/virtio_fs.c | 32

On Thu, Sep 05, 2019 at 03:48:57PM -0400, Vivek Goyal wrote: > It is possible that a mount is in progress and device is being removed at > the same time. Use virtio_fs_mutex to avoid races. > > This also takes care of bunch of races and removes some TODO items. > > Signed-off-by: Vivek Goyal <vgoyal at redhat.com> > --- > fs/fuse/virtio_fs.c | 32

...x); } static void virtio_fs_put(struct fuse_iqueue *fiq) { struct virtio_fs *vfs = fiq->priv; + + mutex_lock(&virtio_fs_mutex); virtiofs_put(vfs); + mutex_unlock(&virtio_fs_mutex); } static void virtio_fs_drain_queue(struct virtio_fs_vq *fsvq) @@ -596,9 +600,8 @@ static void virtio_fs_remove(struct virtio_device *vdev) struct virtio_fs *fs = vdev->priv; mutex_lock(&virtio_fs_mutex); + /* This device is going away. No one should get new reference */ list_del_init(&fs->list); - mutex_unlock(&virtio_fs_mutex); - virtio_fs_stop_all_queues(fs); virtio_fs_drain...

...static void virtio_fs_fiq_release(struct fuse_iqueue *fiq) { struct virtio_fs *vfs = fiq->priv; + + mutex_lock(&virtio_fs_mutex); virtio_fs_put(vfs); + mutex_unlock(&virtio_fs_mutex); } static void virtio_fs_drain_queue(struct virtio_fs_vq *fsvq) @@ -596,9 +600,8 @@ static void virtio_fs_remove(struct virt struct virtio_fs *fs = vdev->priv; mutex_lock(&virtio_fs_mutex); + /* This device is going away. No one should get new reference */ list_del_init(&fs->list); - mutex_unlock(&virtio_fs_mutex); - virtio_fs_stop_all_queues(fs); virtio_fs_drain_all_queues(fs);...

Hi, Michael Tsirkin pointed out issues w.r.t various locking related TODO items and races w.r.t device removal. In this first round of cleanups, I have taken care of most pressing issues. These patches apply on top of following. git://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/fuse.git#virtiofs-v4 I have tested these patches with mount/umount and device removal using qemu monitor. For

...Signed-off-by: Vivek Goyal <vgoyal at redhat.com> --- fs/fuse/virtio_fs.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/fs/fuse/virtio_fs.c b/fs/fuse/virtio_fs.c index 5df97dfee37d..f68a25ca9e9d 100644 --- a/fs/fuse/virtio_fs.c +++ b/fs/fuse/virtio_fs.c @@ -497,8 +497,6 @@ static void virtio_fs_remove(struct virtio_device *vdev) { struct virtio_fs *fs = vdev->priv; - virtio_fs_free_devs(fs); - vdev->config->reset(vdev); virtio_fs_cleanup_vqs(vdev, fs); -- 2.20.1

...+static void virtio_fs_stop_all_queues(struct virtio_fs *fs) +{ + struct virtio_fs_vq *fsvq; + int i; + + for (i = 0; i < fs->nvqs; i++) { + fsvq = &fs->vqs[i]; + spin_lock(&fsvq->lock); + fsvq->connected = false; + spin_unlock(&fsvq->lock); + } +} + static void virtio_fs_remove(struct virtio_device *vdev) { struct virtio_fs *fs = vdev->priv; + virtio_fs_stop_all_queues(fs); vdev->config->reset(vdev); virtio_fs_cleanup_vqs(vdev, fs); -- 2.20.1

...{ + fsvq = &fs->vqs[i]; + if (i == VQ_HIPRIO) + drain_hiprio_queued_reqs(fsvq); + + virtio_fs_drain_queue(fsvq); + } +} + /* Add a new instance to the list or return -EEXIST if tag name exists*/ static int virtio_fs_add_instance(struct virtio_fs *fs) { @@ -511,6 +560,7 @@ static void virtio_fs_remove(struct virtio_device *vdev) struct virtio_fs *fs = vdev->priv; virtio_fs_stop_all_queues(fs); + virtio_fs_drain_all_queues(fs); vdev->config->reset(vdev); virtio_fs_cleanup_vqs(vdev, fs); @@ -865,37 +915,6 @@ __releases(fiq->waitq.lock) } } -static void virtio_fs_flush...

...->dev, sizeof(*fs), GFP_KERNEL); + fs = kzalloc(sizeof(*fs), GFP_KERNEL); if (!fs) return -ENOMEM; vdev->priv = fs; @@ -552,6 +552,7 @@ static int virtio_fs_probe(struct virtio_device *vdev) out: vdev->priv = NULL; + kfree(fs); return ret; } @@ -582,6 +583,8 @@ static void virtio_fs_remove(struct virtio_device *vdev) mutex_unlock(&virtio_fs_mutex); vdev->priv = NULL; + kfree(fs->vqs); + kfree(fs); } #ifdef CONFIG_PM_SLEEP -- 2.20.1

...vqs[i].queued_reqs); INIT_LIST_HEAD(&fs->vqs[i].end_reqs); + init_completion(&fs->vqs[i].in_flight_zero); snprintf(fs->vqs[i].name, sizeof(fs->vqs[i].name), "requests.%u", i - VQ_REQUEST); callbacks[i] = virtio_fs_vq_done; @@ -684,7 +703,7 @@ static void virtio_fs_remove(struct virtio_device *vdev) /* This device is going away. No one should get new reference */ list_del_init(&fs->list); virtio_fs_stop_all_queues(fs); - virtio_fs_drain_all_queues(fs); + virtio_fs_drain_all_queues_locked(fs); vdev->config->reset(vdev); virtio_fs_cleanup_vqs(v...

...NULL; /* not found */ @@ -519,6 +543,7 @@ static int virtio_fs_probe(struct virtio_device *vdev) fs = kzalloc(sizeof(*fs), GFP_KERNEL); if (!fs) return -ENOMEM; + kref_init(&fs->refcount); vdev->priv = fs; ret = virtio_fs_read_tag(vdev, fs); @@ -570,18 +595,18 @@ static void virtio_fs_remove(struct virtio_device *vdev) { struct virtio_fs *fs = vdev->priv; + mutex_lock(&virtio_fs_mutex); + list_del_init(&fs->list); + mutex_unlock(&virtio_fs_mutex); + virtio_fs_stop_all_queues(fs); virtio_fs_drain_all_queues(fs); vdev->config->reset(vdev); virtio_fs_...

On Thu, Sep 05, 2019 at 03:48:56PM -0400, Vivek Goyal wrote: > This object is used both by fuse_connection as well virt device. So make > this object reference counted and that makes it easy to define life cycle > of the object. > > Now deivce can be removed while filesystem is still mounted. This will > cleanup all the virtqueues but virtio_fs object will still be around and

On Thu, Sep 05, 2019 at 03:48:56PM -0400, Vivek Goyal wrote: > This object is used both by fuse_connection as well virt device. So make > this object reference counted and that makes it easy to define life cycle > of the object. > > Now deivce can be removed while filesystem is still mounted. This will > cleanup all the virtqueues but virtio_fs object will still be around and

...} } fs = NULL; /* not found */ @@ -519,6 +543,7 @@ static int virtio_fs_probe(struct virtio fs = kzalloc(sizeof(*fs), GFP_KERNEL); if (!fs) return -ENOMEM; + kref_init(&fs->refcount); vdev->priv = fs; ret = virtio_fs_read_tag(vdev, fs); @@ -570,18 +595,18 @@ static void virtio_fs_remove(struct virt { struct virtio_fs *fs = vdev->priv; + mutex_lock(&virtio_fs_mutex); + list_del_init(&fs->list); + mutex_unlock(&virtio_fs_mutex); + virtio_fs_stop_all_queues(fs); virtio_fs_drain_all_queues(fs); vdev->config->reset(vdev); virtio_fs_cleanup_vqs(vdev...

Hi Miklos, Here are few small cleanups for virtiofs for 5.5. I had received some comments from Michael Tsirkin on original virtiofs patches and these cleanups are result of these comments. Thanks Vivek Vivek Goyal (3): virtiofs: Use a common function to send forget virtiofs: Do not send forget request "struct list_head" element virtiofs: Use completions while waiting for queue

...t; +{ > + struct virtio_fs_vq *fsvq; > + int i; > + > + for (i = 0; i < fs->nvqs; i++) { > + fsvq = &fs->vqs[i]; > + spin_lock(&fsvq->lock); > + fsvq->connected = false; > + spin_unlock(&fsvq->lock); > + } > +} > + > +static void virtio_fs_remove(struct virtio_device *vdev) > +{ > + struct virtio_fs *fs = vdev->priv; > + > + mutex_lock(&virtio_fs_mutex); > + /* This device is going away. No one should get new reference */ > + list_del_init(&fs->list); > + virtio_fs_stop_all_queues(fs); > + virtio_fs_dr...

...> > + .feature_table_size = ARRAY_SIZE(feature_table), > > + /* TODO validate config_get != NULL */ > > Why? Don't know. Stefan, do you remember why did you put this comment? If not, I will get rid of it. > > > + .probe = virtio_fs_probe, > > + .remove = virtio_fs_remove, > > +#ifdef CONFIG_PM_SLEEP > > + .freeze = virtio_fs_freeze, > > + .restore = virtio_fs_restore, > > +#endif > > +}; > > + > > +static void virtio_fs_wake_forget_and_unlock(struct fuse_iqueue *fiq) > > +__releases(fiq->waitq.lock) > > +...

...= virtio_fs_add_instance(fs); > + if (ret < 0) > + goto out_vqs; > + > + return 0; > + > +out_vqs: > + vdev->config->reset(vdev); > + virtio_fs_cleanup_vqs(vdev, fs); > + > +out: > + vdev->priv = NULL; > + return ret; > +} > + > +static void virtio_fs_remove(struct virtio_device *vdev) > +{ > + struct virtio_fs *fs = vdev->priv; > + > + virtio_fs_free_devs(fs); > + > + vdev->config->reset(vdev); > + virtio_fs_cleanup_vqs(vdev, fs); > + > + mutex_lock(&virtio_fs_mutex); > + list_del(&fs->list); > + m...

...= virtio_fs_add_instance(fs); > + if (ret < 0) > + goto out_vqs; > + > + return 0; > + > +out_vqs: > + vdev->config->reset(vdev); > + virtio_fs_cleanup_vqs(vdev, fs); > + > +out: > + vdev->priv = NULL; > + return ret; > +} > + > +static void virtio_fs_remove(struct virtio_device *vdev) > +{ > + struct virtio_fs *fs = vdev->priv; > + > + virtio_fs_free_devs(fs); > + > + vdev->config->reset(vdev); > + virtio_fs_cleanup_vqs(vdev, fs); > + > + mutex_lock(&virtio_fs_mutex); > + list_del(&fs->list); > + m...