search for: virtio_fs_enqueue_req

Pass fsvq instead of vq as parameter to virtio_fs_enqueue_req(). We will retrieve vq from fsvq under spin lock. Later in the patch series we will retrieve vq only if fsvq is still connected other vq might have been cleaned up by device ->remove code and we will return error. Signed-off-by: Vivek Goyal <vgoyal at redhat.com> --- fs/fuse/virtio_fs.c...

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

...e *fpq; > > + struct virtio_fs_vq *fsvq; > > int ret; > > > > WARN_ON(list_empty(&fiq->pending)); > > @@ -951,7 +974,8 @@ __releases(fiq->lock) > > smp_mb__after_atomic(); > > > > retry: > > - ret = virtio_fs_enqueue_req(&fs->vqs[queue_id], req); > > + fsvq = &fs->vqs[queue_id]; > > + ret = virtio_fs_enqueue_req(fsvq, req); > > if (ret < 0) { > > if (ret == -ENOMEM || ret == -ENOSPC) { > > /* Virtqueue full. R...

...parate so that review is easier. Any feedback or comments are 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 ->...

...e_interrupt_and_unlock(struct fuse_iqueue *fiq) > > +__releases(fiq->waitq.lock) > > +{ > > + /* TODO */ > > what's needed? We have not implemented this interrupt thing. It interrupts the existing pending requests. Its not a must to have. [..] > > +static int virtio_fs_enqueue_req(struct virtqueue *vq, struct fuse_req *req) > > +{ > > + /* requests need at least 4 elements */ > > + struct scatterlist *stack_sgs[6]; > > + struct scatterlist stack_sg[ARRAY_SIZE(stack_sgs)]; > > + struct scatterlist **sgs = stack_sgs; > > + struct scatterlist...

...++++ 1 file changed, 4 insertions(+) diff --git a/fs/fuse/virtio_fs.c b/fs/fuse/virtio_fs.c index 197e79e536f9..a708ccb65662 100644 --- a/fs/fuse/virtio_fs.c +++ b/fs/fuse/virtio_fs.c @@ -826,6 +826,10 @@ __releases(fiq->waitq.lock) } req->out.h.error = ret; pr_err("virtio-fs: virtio_fs_enqueue_req() failed %d\n", ret); + spin_lock(&fpq->lock); + clear_bit(FR_SENT, &req->flags); + list_del_init(&req->list); + spin_unlock(&fpq->lock); fuse_request_end(fc, req); return; } -- 2.20.1

...d-off-by: Vivek Goyal <vgoyal at redhat.com> --- fs/fuse/virtio_fs.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/fs/fuse/virtio_fs.c b/fs/fuse/virtio_fs.c index 9d30530e3ca9..c46dd4d284d6 100644 --- a/fs/fuse/virtio_fs.c +++ b/fs/fuse/virtio_fs.c @@ -755,6 +755,12 @@ static int virtio_fs_enqueue_req(struct virtio_fs_vq *fsvq, spin_lock(&fsvq->lock); + if (!fsvq->connected) { + spin_unlock(&fsvq->lock); + ret = -ENOTCONN; + goto out; + } + vq = fsvq->vq; ret = virtqueue_add_sgs(vq, sgs, out_sgs, in_sgs, req, GFP_ATOMIC); if (ret < 0) { -- 2.20.1

...irtio_fs.c @@ -360,6 +360,9 @@ static void virtio_fs_requests_done_work(struct work_struct *work) spin_unlock(&fpq->lock); fuse_request_end(fc, req); + spin_lock(&fsvq->lock); + fsvq->in_flight--; + spin_unlock(&fsvq->lock); } } @@ -769,6 +772,7 @@ static int virtio_fs_enqueue_req(struct virtio_fs_vq *fsvq, goto out; } + fsvq->in_flight++; notify = virtqueue_kick_prepare(vq); spin_unlock(&fsvq->lock); -- 2.20.1

...e series from Vivek and fixed a couple of TODO comments > myself. AFAICS two issues remain that need to be resolved in the short > term, one way or the other: freeze/restore and full virtqueue. Hi Miklos, We are already handling full virtqueue by waiting a bit and retrying. I think TODO in virtio_fs_enqueue_req() is stale. Caller already handles virtqueue full situation by retrying. Havind said that, this could be improved by using some sort of wait queue or completion privimitve. Thanks Vivek

...> > + spin_unlock(&fsvq->lock); > > + return; > > + } > > + list_del_init(&req->list); > > + spin_unlock(&fsvq->lock); > > + > > + ret = virtio_fs_enqueue_req(fsvq, req, true); > > + if (ret < 0) { > > + if (ret == -ENOMEM || ret == -ENOSPC) { > > + spin_lock(&fsvq->lock); > > + list_add_tail(&req->list, &fsvq->q...

...ues */ }; -struct virtio_fs_forget { +struct virtio_fs_forget_req { struct fuse_in_header ih; struct fuse_forget_in arg; +}; + +struct virtio_fs_forget { /* This request can be temporarily queued on virt queue */ struct list_head list; + struct virtio_fs_forget_req req; }; static int virtio_fs_enqueue_req(struct virtio_fs_vq *fsvq, @@ -325,6 +329,7 @@ static int send_forget_request(struct virtio_fs_vq *fsvq, struct virtqueue *vq; int ret = 0; bool notify; + struct virtio_fs_forget_req *req = &forget->req; spin_lock(&fsvq->lock); if (!fsvq->connected) { @@ -334,7 +339,7 @...

...+ req->pages, > + req->page_descs, > + req->num_pages, > + args[numargs - 1].size); > + > + if (len_used) > + *len_used = len; > + > + return total_sgs; > +} > + > +/* Add a request to a virtqueue and kick the device */ > +static int virtio_fs_enqueue_req(struct virtqueue *vq, struct fuse_req *req) > +{ > + /* requests need at least 4 elements */ > + struct scatterlist *stack_sgs[6]; > + struct scatterlist stack_sg[ARRAY_SIZE(stack_sgs)]; > + struct scatterlist **sgs = stack_sgs; > + struct scatterlist *sg = stack_sg; > + struct...

...+ req->pages, > + req->page_descs, > + req->num_pages, > + args[numargs - 1].size); > + > + if (len_used) > + *len_used = len; > + > + return total_sgs; > +} > + > +/* Add a request to a virtqueue and kick the device */ > +static int virtio_fs_enqueue_req(struct virtqueue *vq, struct fuse_req *req) > +{ > + /* requests need at least 4 elements */ > + struct scatterlist *stack_sgs[6]; > + struct scatterlist stack_sg[ARRAY_SIZE(stack_sgs)]; > + struct scatterlist **sgs = stack_sgs; > + struct scatterlist *sg = stack_sg; > + struct...

...;virtio-fs: Could not queue FORGET: err=%d. Dropping it.\n", ret); @@ -739,7 +735,7 @@ __releases(fiq->lock) goto out; } - fsvq->in_flight++; + inc_in_flight_req(fsvq); notify = virtqueue_kick_prepare(vq); spin_unlock(&fsvq->lock); @@ -921,7 +917,7 @@ static int virtio_fs_enqueue_req(struct virtio_fs_vq *fsvq, /* matches barrier in request_wait_answer() */ smp_mb__after_atomic(); - fsvq->in_flight++; + inc_in_flight_req(fsvq); notify = virtqueue_kick_prepare(vq); spin_unlock(&fsvq->lock); -- 2.20.1

...index a5c86048b96e..7505f8102762 100644 --- a/fs/fuse/virtio_fs.c +++ b/fs/fuse/virtio_fs.c @@ -55,6 +55,12 @@ struct virtio_fs_forget { struct list_head list; }; +struct virtio_fs_req_work { + struct fuse_req *req; + struct virtio_fs_vq *fsvq; + struct work_struct done_work; +}; + static int virtio_fs_enqueue_req(struct virtio_fs_vq *fsvq, struct fuse_req *req, bool in_flight); @@ -443,19 +449,67 @@ static void copy_args_from_argbuf(struct fuse_args *args, struct fuse_req *req) } /* Work function for request completion */ +static void virtio_fs_request_complete(struct fuse_req *req, +...

...74768903..0c6ef5d3c6ab 100644 --- a/fs/fuse/virtio_fs.c +++ b/fs/fuse/virtio_fs.c @@ -60,6 +60,12 @@ struct virtio_fs_forget { struct virtio_fs_forget_req req; }; +struct virtio_fs_req_work { + struct fuse_req *req; + struct virtio_fs_vq *fsvq; + struct work_struct done_work; +}; + static int virtio_fs_enqueue_req(struct virtio_fs_vq *fsvq, struct fuse_req *req, bool in_flight); @@ -485,19 +491,67 @@ static void copy_args_from_argbuf(struct fuse_args *args, struct fuse_req *req) } /* Work function for request completion */ +static void virtio_fs_request_complete(struct fuse_req *req, +...

...sg[total_sgs], > + ap->pages, ap->descs, > + ap->num_pages, > + args[numargs - 1].size); > + > + if (len_used) > + *len_used = len; > + > + return total_sgs; > +} > + > +/* Add a request to a virtqueue and kick the device */ > +static int virtio_fs_enqueue_req(struct virtio_fs_vq *fsvq, > + struct fuse_req *req) > +{ > + /* requests need at least 4 elements */ > + struct scatterlist *stack_sgs[6]; > + struct scatterlist stack_sg[ARRAY_SIZE(stack_sgs)]; > + struct scatterlist **sgs = stack_sgs; > + struct scatterlist *sg = stack_s...

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