Skip to content

Commit a9bfd9d

Browse files
rhvgoyalMiklos Szeredi
authored andcommitted
virtiofs: Retry request submission from worker context
If regular request queue gets full, currently we sleep for a bit and retrying submission in submitter's context. This assumes submitter is not holding any spin lock. But this assumption is not true for background requests. For background requests, we are called with fc->bg_lock held. This can lead to deadlock where one thread is trying submission with fc->bg_lock held while request completion thread has called fuse_request_end() which tries to acquire fc->bg_lock and gets blocked. As request completion thread gets blocked, it does not make further progress and that means queue does not get empty and submitter can't submit more requests. To solve this issue, retry submission with the help of a worker, instead of retrying in submitter's context. We already do this for hiprio/forget requests. Reported-by: Chirantan Ekbote <[email protected]> Signed-off-by: Vivek Goyal <[email protected]> Signed-off-by: Miklos Szeredi <[email protected]>
1 parent c17ea00 commit a9bfd9d

File tree

1 file changed

+52
-9
lines changed

1 file changed

+52
-9
lines changed

fs/fuse/virtio_fs.c

Lines changed: 52 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,9 @@ struct virtio_fs_forget {
5555
struct list_head list;
5656
};
5757

58+
static int virtio_fs_enqueue_req(struct virtio_fs_vq *fsvq,
59+
struct fuse_req *req, bool in_flight);
60+
5861
static inline struct virtio_fs_vq *vq_to_fsvq(struct virtqueue *vq)
5962
{
6063
struct virtio_fs *fs = vq->vdev->priv;
@@ -260,6 +263,7 @@ static void virtio_fs_request_dispatch_work(struct work_struct *work)
260263
struct virtio_fs_vq *fsvq = container_of(work, struct virtio_fs_vq,
261264
dispatch_work.work);
262265
struct fuse_conn *fc = fsvq->fud->fc;
266+
int ret;
263267

264268
pr_debug("virtio-fs: worker %s called.\n", __func__);
265269
while (1) {
@@ -268,13 +272,45 @@ static void virtio_fs_request_dispatch_work(struct work_struct *work)
268272
list);
269273
if (!req) {
270274
spin_unlock(&fsvq->lock);
271-
return;
275+
break;
272276
}
273277

274278
list_del_init(&req->list);
275279
spin_unlock(&fsvq->lock);
276280
fuse_request_end(fc, req);
277281
}
282+
283+
/* Dispatch pending requests */
284+
while (1) {
285+
spin_lock(&fsvq->lock);
286+
req = list_first_entry_or_null(&fsvq->queued_reqs,
287+
struct fuse_req, list);
288+
if (!req) {
289+
spin_unlock(&fsvq->lock);
290+
return;
291+
}
292+
list_del_init(&req->list);
293+
spin_unlock(&fsvq->lock);
294+
295+
ret = virtio_fs_enqueue_req(fsvq, req, true);
296+
if (ret < 0) {
297+
if (ret == -ENOMEM || ret == -ENOSPC) {
298+
spin_lock(&fsvq->lock);
299+
list_add_tail(&req->list, &fsvq->queued_reqs);
300+
schedule_delayed_work(&fsvq->dispatch_work,
301+
msecs_to_jiffies(1));
302+
spin_unlock(&fsvq->lock);
303+
return;
304+
}
305+
req->out.h.error = ret;
306+
spin_lock(&fsvq->lock);
307+
dec_in_flight_req(fsvq);
308+
spin_unlock(&fsvq->lock);
309+
pr_err("virtio-fs: virtio_fs_enqueue_req() failed %d\n",
310+
ret);
311+
fuse_request_end(fc, req);
312+
}
313+
}
278314
}
279315

280316
static void virtio_fs_hiprio_dispatch_work(struct work_struct *work)
@@ -837,7 +873,7 @@ static unsigned int sg_init_fuse_args(struct scatterlist *sg,
837873

838874
/* Add a request to a virtqueue and kick the device */
839875
static int virtio_fs_enqueue_req(struct virtio_fs_vq *fsvq,
840-
struct fuse_req *req)
876+
struct fuse_req *req, bool in_flight)
841877
{
842878
/* requests need at least 4 elements */
843879
struct scatterlist *stack_sgs[6];
@@ -917,7 +953,8 @@ static int virtio_fs_enqueue_req(struct virtio_fs_vq *fsvq,
917953
/* matches barrier in request_wait_answer() */
918954
smp_mb__after_atomic();
919955

920-
inc_in_flight_req(fsvq);
956+
if (!in_flight)
957+
inc_in_flight_req(fsvq);
921958
notify = virtqueue_kick_prepare(vq);
922959

923960
spin_unlock(&fsvq->lock);
@@ -963,15 +1000,21 @@ __releases(fiq->lock)
9631000
req->in.h.nodeid, req->in.h.len,
9641001
fuse_len_args(req->args->out_numargs, req->args->out_args));
9651002

966-
retry:
9671003
fsvq = &fs->vqs[queue_id];
968-
ret = virtio_fs_enqueue_req(fsvq, req);
1004+
ret = virtio_fs_enqueue_req(fsvq, req, false);
9691005
if (ret < 0) {
9701006
if (ret == -ENOMEM || ret == -ENOSPC) {
971-
/* Virtqueue full. Retry submission */
972-
/* TODO use completion instead of timeout */
973-
usleep_range(20, 30);
974-
goto retry;
1007+
/*
1008+
* Virtqueue full. Retry submission from worker
1009+
* context as we might be holding fc->bg_lock.
1010+
*/
1011+
spin_lock(&fsvq->lock);
1012+
list_add_tail(&req->list, &fsvq->queued_reqs);
1013+
inc_in_flight_req(fsvq);
1014+
schedule_delayed_work(&fsvq->dispatch_work,
1015+
msecs_to_jiffies(1));
1016+
spin_unlock(&fsvq->lock);
1017+
return;
9751018
}
9761019
req->out.h.error = ret;
9771020
pr_err("virtio-fs: virtio_fs_enqueue_req() failed %d\n", ret);

0 commit comments

Comments
 (0)