search for: vq_request

As of now we maintain this count only for VQ_HIPRIO. Maintain it for VQ_REQUEST as well so that later it can be used to drain VQ_REQUEST queue. Signed-off-by: Vivek Goyal <vgoyal at redhat.com> --- fs/fuse/virtio_fs.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/fs/fuse/virtio_fs.c b/fs/fuse/virtio_fs.c index c46dd4d284d6..5df97dfee37d 100644 --- a/fs/fuse/v...

Right now we are checking ->connected state only for VQ_HIPRIO. Now we want to make use of this method for all queues. So check it for VQ_REQUEST as well. This will be helpful if device has been removed and virtqueue is gone. In that case ->connected will be false and request can't be submitted anymore and user space will see error -ENOTCONN. Signed-off-by: Vivek Goyal <vgoyal at redhat.com> --- fs/fuse/virtio_fs.c | 6 ++++++...

...gs, out_sgs, in_sgs, req, GFP_ATOMIC); > > + if (ret < 0) { > > + /* TODO handle full virtqueue */ > > Indeed. What prevents this? So for forget requests (VQ_HIPRIO), I already handled this by retrying submitting the request with the help of a worker. For regular requests (VQ_REQUESTS), I have never run into virt queue being full so far. That's why never worried about it. So nothing prevents this. But we have not noticed it yet. So its a TODO item. It will be nice to retry if virtuqueue gets full (instead of returning error to caller). [..] > > +static void virtio_f...

...ontext.h> > +#include <linux/highmem.h> > +#include "fuse_i.h" > + > +/* List of virtio-fs device instances and a lock for the list */ > +static DEFINE_MUTEX(virtio_fs_mutex); > +static LIST_HEAD(virtio_fs_instances); > + > +enum { > + VQ_HIPRIO, > + VQ_REQUEST > +}; > + > +/* Per-virtqueue state */ > +struct virtio_fs_vq { > + spinlock_t lock; > + struct virtqueue *vq; /* protected by ->lock */ > + struct work_struct done_work; > + struct list_head queued_reqs; > + struct delayed_work dispatch_work; > + struct fuse_de...

...ontext.h> > +#include <linux/highmem.h> > +#include "fuse_i.h" > + > +/* List of virtio-fs device instances and a lock for the list */ > +static DEFINE_MUTEX(virtio_fs_mutex); > +static LIST_HEAD(virtio_fs_instances); > + > +enum { > + VQ_HIPRIO, > + VQ_REQUEST > +}; > + > +/* Per-virtqueue state */ > +struct virtio_fs_vq { > + spinlock_t lock; > + struct virtqueue *vq; /* protected by ->lock */ > + struct work_struct done_work; > + struct list_head queued_reqs; > + struct delayed_work dispatch_work; > + struct fuse_de...

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

...re welcome. Thanks Vivek Vivek Goyal (18): virtiofs: Remove request from processing list before calling end virtiofs: Check whether hiprio queue is connected at submission time virtiofs: Pass fsvq instead of vq as parameter to virtio_fs_enqueue_req virtiofs: Check connected state for VQ_REQUEST queue as well Maintain count of in flight requests for VQ_REQUEST queue virtiofs: ->remove should not clean virtiofs fuse devices virtiofs: Stop virtiofs queues when device is being removed virtiofs: Drain all pending requests during ->remove time virtiofs: Add an helper to start al...

...gt;priv = NULL; /* Put device reference on virtio_fs object */ virtiofs_put(fs); + mutex_unlock(&virtio_fs_mutex); } #ifdef CONFIG_PM_SLEEP @@ -978,10 +982,15 @@ static int virtio_fs_fill_super(struct super_block *sb) .no_force_umount = true, }; - /* TODO lock */ - if (fs->vqs[VQ_REQUEST].fud) { - pr_err("virtio-fs: device already in use\n"); - err = -EBUSY; + mutex_lock(&virtio_fs_mutex); + + /* After holding mutex, make sure virtiofs device is still there. + * Though we are holding a refernce to it, drive ->remove might + * still have cleaned up virtual queue...

...ot; > + > +/* List of virtio-fs device instances and a lock for the list. Also provides > + * mutual exclusion in device removal and mounting path > + */ > +static DEFINE_MUTEX(virtio_fs_mutex); > +static LIST_HEAD(virtio_fs_instances); > + > +enum { > + VQ_HIPRIO, > + VQ_REQUEST > +}; > + > +/* Per-virtqueue state */ > +struct virtio_fs_vq { > + spinlock_t lock; > + struct virtqueue *vq; /* protected by ->lock */ > + struct work_struct done_work; > + struct list_head queued_reqs; > + struct delayed_work dispatch_work; > + struct fuse_de...

...virt vdev->priv = NULL; /* Put device reference on virtio_fs object */ virtio_fs_put(fs); + mutex_unlock(&virtio_fs_mutex); } #ifdef CONFIG_PM_SLEEP @@ -978,10 +982,15 @@ static int virtio_fs_fill_super(struct s .no_force_umount = true, }; - /* TODO lock */ - if (fs->vqs[VQ_REQUEST].fud) { - pr_err("virtio-fs: device already in use\n"); - err = -EBUSY; + mutex_lock(&virtio_fs_mutex); + + /* After holding mutex, make sure virtiofs device is still there. + * Though we are holding a refernce to it, drive ->remove might + * still have cleaned up virtual queue...

...(&fsvq->lock); + if (!list_empty(&fsvq->queued_reqs)) + schedule_work(&fsvq->dispatch_work); + spin_unlock(&fsvq->lock); } /* Virtqueue interrupt handler */ @@ -670,12 +674,12 @@ static void virtio_fs_init_vq(struct virtio_fs_vq *fsvq, char *name, if (vq_type == VQ_REQUEST) { INIT_WORK(&fsvq->done_work, virtio_fs_requests_done_work); - INIT_DELAYED_WORK(&fsvq->dispatch_work, - virtio_fs_request_dispatch_work); + INIT_WORK(&fsvq->dispatch_work, + virtio_fs_request_dispatch_work); } else { INIT_WORK(&fsvq->done_work, virti...

...(&fsvq->lock); + if (!list_empty(&fsvq->queued_reqs)) + schedule_work(&fsvq->dispatch_work); + spin_unlock(&fsvq->lock); } /* Virtqueue interrupt handler */ @@ -670,12 +674,12 @@ static void virtio_fs_init_vq(struct virtio_fs_vq *fsvq, char *name, if (vq_type == VQ_REQUEST) { INIT_WORK(&fsvq->done_work, virtio_fs_requests_done_work); - INIT_DELAYED_WORK(&fsvq->dispatch_work, - virtio_fs_request_dispatch_work); + INIT_WORK(&fsvq->dispatch_work, + virtio_fs_request_dispatch_work); } else { INIT_WORK(&fsvq->done_work, virti...

...c void virtio_fs_free_devs(struct virtio_fs *fs) if (!fsvq->fud) continue; - /* TODO need to quiesce/end_requests/decrement dev_count */ fuse_dev_free(fsvq->fud); fsvq->fud = NULL; } @@ -1022,7 +1021,6 @@ static int virtio_fs_fill_super(struct super_block *sb) if (i == VQ_REQUEST) continue; /* already initialized */ fuse_dev_install(fsvq->fud, fc); - atomic_inc(&fc->dev_count); } /* Previous unmount will stop all queues. Start these again */ -- 2.20.1

..._work); > > INIT_LIST_HEAD(&fs->vqs[i].queued_reqs); > > + INIT_LIST_HEAD(&fs->vqs[i].end_reqs); > > snprintf(fs->vqs[i].name, sizeof(fs->vqs[i].name), > > "requests.%u", i - VQ_REQUEST); > > callbacks[i] = virtio_fs_vq_done; > > @@ -918,6 +940,7 @@ __releases(fiq->lock) > > struct fuse_conn *fc; > > struct fuse_req *req; > > struct fuse_pqueue *fpq; > > + struct virtio_fs_vq *fsvq; > >...

...ibing at huawei.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 2de8fc0..a5c8604 100644 --- a/fs/fuse/virtio_fs.c +++ b/fs/fuse/virtio_fs.c @@ -980,7 +980,6 @@ __releases(fiq->lock) { unsigned int queue_id = VQ_REQUEST; /* TODO multiqueue */ struct virtio_fs *fs; - struct fuse_conn *fc; struct fuse_req *req; struct virtio_fs_vq *fsvq; int ret; @@ -993,7 +992,6 @@ __releases(fiq->lock) spin_unlock(&fiq->lock); fs = fiq->priv; - fc = fs->vqs[queue_id].fud->fc; pr_debug("%s:...

...ice *vdev, virtio_fs_request_dispatch_work); INIT_LIST_HEAD(&fs->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_dr...

...u remember why did you put this comment? If not, > I will get rid of it. This comment can be removed. > > > +static void virtio_fs_wake_pending_and_unlock(struct fuse_iqueue *fiq) > > > +__releases(fiq->waitq.lock) > > > +{ > > > + unsigned int queue_id = VQ_REQUEST; /* TODO multiqueue */ > > > + struct virtio_fs *fs; > > > + struct fuse_conn *fc; > > > + struct fuse_req *req; > > > + struct fuse_pqueue *fpq; > > > + int ret; > > > + > > > + WARN_ON(list_empty(&fiq->pending)); > > >...

Hi, We have couple of places which can result in deadlock. This patch series fixes these. We can be called with fc->bg_lock (for background requests) while submitting a request. This leads to two constraints. - We can't end requests in submitter's context and call fuse_end_request() as it tries to take fc->bg_lock as well. So queue these requests on a list and use a worker to

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