* [PATCH v3] fuse: virtio-fs: stop new requests before draining queues
@ 2026-11-09 2:39 Qihang
2026-11-09 3:16 ` sashiko-bot
2026-11-12 3:09 ` Li Wang
0 siblings, 2 replies; 3+ messages in thread
From: Qihang @ 2026-11-09 2:39 UTC (permalink / raw)
To: German Maglione, Vivek Goyal, Stefan Hajnoczi, Miklos Szeredi
Cc: Eugenio Perez, virtualization, linux-fsdevel, fuse-devel,
linux-kernel
virtio_fs_conn_destroy() drains the virtqueues and then calls
fuse_conn_destroy(), which aborts the connection. The abort ends every
request the fuse device still owns, and each ended request is freed as
soon as its waiter wakes up.
The drain only covers the requests that were already counted in
fsvq->in_flight when it started. A request submitted after it returns is
not covered, but is still handed to the device before the abort runs:
CPU 0 (umount) CPU 1
--------------------------------- --------------------------------
virtio_fs_drain_all_queues()
/* in_flight drops to 0 and
the drain returns */
virtio_fs_enqueue_req()
virtqueue_add_sgs()
inc_in_flight_req() -> 1
fuse_conn_destroy()
fuse_send_destroy()
fuse_chan_abort()
/* ends and frees the
request above */
fuse_chan_wait_aborted()
/* the device still owns the
virtqueue descriptors and
DMA-writes into the freed
request and its argbuf */
Stop admitting requests on each queue before draining begins. The flag
is set per queue, but that is sufficient: request admission,
virtqueue_add_sgs() and, on the submission path, the in_flight
increment are serialized by the same fsvq->lock. A request is
therefore either rejected before the device can see it, or already
counted in fsvq->in_flight and covered by the drain. There is no
window in between.
Rejected requests use the existing -ENOTCONN error path and are ended by
virtio_fs_request_dispatch_work() through end_reqs, so no new request
completion or ownership state is introduced. Nothing has to be
discarded in the completion path, and fsvq->in_flight remains the only
per-queue accounting state.
The -ENOSPC path needs the same admission check. virtio_fs_enqueue_req()
drops fsvq->lock before returning -ENOSPC, and the caller queues the
request in a second critical section; teardown can finish in between and
leave the request stranded on queued_reqs, which nothing drains.
Re-check connected/aborting before queueing, in both the submission path
and the dispatch worker. After aborting is set, no request can newly
enter queued_reqs or the virtqueue, except the explicitly permitted
FUSE_DESTROY path.
Rejected requests are ended by virtio_fs_request_dispatch_work(). If
their end callback may block (a DAX release), ending them inline can
deadlock: the callback may issue a synchronous request that needs the
same worker. Offload those endings to a separate work item, like the
device completion path already does.
The new state is not folded into fsvq->connected because the request
queues have to stay usable: fuse_conn_destroy() sends FUSE_DESTROY
through them, with args.force set so that fuse_simple_request() waits
for the reply. Clearing fsvq->connected would make that request fail
with -ENOTCONN and the server would never see the destroy.
FUSE_DESTROY is the sole exception while aborting is set. It is sent
only after the first drain, and its completion is waited for before
fuse_chan_abort() can end requests, so the destroy is the only request
the device sees after the drain. virtio-fs exposes no userspace
/dev/fuse fd (and no_control disables the fusectl interface), so
userspace cannot inject another request.
virtio_fs_start_all_queues() clears the flag, so a mount that follows an
unmount starts with admission enabled again.
Fixes: a62a8ef9d97d ("virtio-fs: add virtiofs filesystem")
Cc: stable@vger.kernel.org
Signed-off-by: Qihang <q.h.hack.winter@gmail.com>
---
V2 -> V3:
- re-check admission before queueing on -ENOSPC, in virtio_fs_send_req()
and in the dispatch worker, so teardown cannot strand a request on
queued_reqs
- offload may_block request endings in the dispatch worker to avoid a
self-deadlock through fuse_simple_request()
- add Fixes: and Cc: stable
V1 -> V2:
- prevent new requests from reaching the virtqueue at teardown time
instead of skipping completions of aborted requests
- keep FUSE_DESTROY working: it is the teardown request and is waited
for before fuse_chan_abort()
- drop the per-vq aborted flag and the completion-skip branch
fs/fuse/virtio_fs.c | 104 +++++++++++++++++++++++++++++++++++++++-----
1 file changed, 94 insertions(+), 10 deletions(-)
diff --git a/fs/fuse/virtio_fs.c b/fs/fuse/virtio_fs.c
index ba8d79c4bcac6..a46536c9fdc8e 100644
--- a/fs/fuse/virtio_fs.c
+++ b/fs/fuse/virtio_fs.c
@@ -55,6 +55,7 @@ struct virtio_fs_vq {
struct work_struct dispatch_work;
struct fuse_dev *fud;
bool connected;
+ bool aborting; /* reject new requests except FUSE_DESTROY */
long in_flight;
struct completion in_flight_zero; /* No inflight requests */
struct kobject *kobj;
@@ -97,6 +98,11 @@ struct virtio_fs_req_work {
struct work_struct done_work;
};
+struct virtio_fs_end_work {
+ struct work_struct work;
+ struct fuse_req *req;
+};
+
static int virtio_fs_enqueue_req(struct virtio_fs_vq *fsvq,
struct fuse_req *req, bool in_flight,
gfp_t gfp);
@@ -348,6 +354,7 @@ static void virtio_fs_start_all_queues(struct virtio_fs *fs)
fsvq = &fs->vqs[i];
spin_lock(&fsvq->lock);
fsvq->connected = true;
+ fsvq->aborting = false;
spin_unlock(&fsvq->lock);
}
}
@@ -551,6 +558,39 @@ static void virtio_fs_hiprio_done_work(struct work_struct *work)
spin_unlock(&fsvq->lock);
}
+static void virtio_fs_end_req_work(struct work_struct *work)
+{
+ struct virtio_fs_end_work *w =
+ container_of(work, typeof(*w), work);
+
+ fuse_request_end(w->req);
+ kfree(w);
+}
+
+/*
+ * A request rejected during teardown can have an end callback that may
+ * block (DAX release). Ending it inline in virtio_fs_request_dispatch_work()
+ * can deadlock: the callback may issue a synchronous request whose
+ * completion needs the same dispatch_work that is currently blocked.
+ * Offload it, like the device completion path does.
+ *
+ * virtio_fs_request_complete() cannot be reused here: it also consumes
+ * argbuf and drops in_flight, while a rejected request has neither.
+ */
+static void virtio_fs_end_req(struct fuse_req *req)
+{
+ if (req->args->may_block) {
+ struct virtio_fs_end_work *w;
+
+ w = kzalloc_obj(*w, GFP_NOFS | __GFP_NOFAIL);
+ INIT_WORK(&w->work, virtio_fs_end_req_work);
+ w->req = req;
+ schedule_work(&w->work);
+ } else {
+ fuse_request_end(req);
+ }
+}
+
static void virtio_fs_request_dispatch_work(struct work_struct *work)
{
struct fuse_req *req;
@@ -570,7 +610,7 @@ static void virtio_fs_request_dispatch_work(struct work_struct *work)
list_del_init(&req->list);
spin_unlock(&fsvq->lock);
- fuse_request_end(req);
+ virtio_fs_end_req(req);
}
/* Dispatch pending requests */
@@ -593,9 +633,17 @@ static void virtio_fs_request_dispatch_work(struct work_struct *work)
if (ret < 0) {
if (ret == -ENOSPC) {
spin_lock(&fsvq->lock);
- list_add_tail(&req->list, &fsvq->queued_reqs);
- spin_unlock(&fsvq->lock);
- return;
+ if (!fsvq->connected ||
+ (fsvq->aborting &&
+ req->in.h.opcode != FUSE_DESTROY)) {
+ spin_unlock(&fsvq->lock);
+ ret = -ENOTCONN;
+ } else {
+ list_add_tail(&req->list,
+ &fsvq->queued_reqs);
+ spin_unlock(&fsvq->lock);
+ return;
+ }
}
req->out.h.error = ret;
spin_lock(&fsvq->lock);
@@ -603,7 +651,7 @@ static void virtio_fs_request_dispatch_work(struct work_struct *work)
spin_unlock(&fsvq->lock);
pr_err("virtio-fs: virtio_fs_enqueue_req() failed %d\n",
ret);
- fuse_request_end(req);
+ virtio_fs_end_req(req);
}
}
}
@@ -1226,6 +1274,28 @@ static void virtio_fs_stop_all_queues(struct virtio_fs *fs)
}
}
+/*
+ * Stop admitting new requests before draining the queues. Admission and
+ * virtqueue submission are serialized by fsvq->lock, so a request is
+ * either rejected before reaching the device or accounted in in_flight
+ * and covered by the drain.
+ *
+ * FUSE_DESTROY is exempt because fuse_conn_destroy() sends it after the
+ * first drain and waits for its completion before aborting the channel.
+ */
+static void virtio_fs_stop_new_requests(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->aborting = true;
+ spin_unlock(&fsvq->lock);
+ }
+}
+
static void virtio_fs_remove(struct virtio_device *vdev)
{
struct virtio_fs *fs = vdev->priv;
@@ -1479,7 +1549,8 @@ static int virtio_fs_enqueue_req(struct virtio_fs_vq *fsvq,
spin_lock(&fsvq->lock);
- if (!fsvq->connected) {
+ if (!fsvq->connected ||
+ (fsvq->aborting && req->in.h.opcode != FUSE_DESTROY)) {
spin_unlock(&fsvq->lock);
ret = -ENOTCONN;
goto out;
@@ -1551,12 +1622,22 @@ static void virtio_fs_send_req(struct fuse_iqueue *fiq, struct fuse_req *req)
/*
* Virtqueue full. Retry submission from worker
* context as we might be holding fc->chan->bg_lock.
+ * Re-check admission under the same lock that
+ * increments in_flight: teardown may have started
+ * between enqueue_req() dropping its lock and here.
*/
spin_lock(&fsvq->lock);
- list_add_tail(&req->list, &fsvq->queued_reqs);
- inc_in_flight_req(fsvq);
- spin_unlock(&fsvq->lock);
- return;
+ if (!fsvq->connected ||
+ (fsvq->aborting &&
+ req->in.h.opcode != FUSE_DESTROY)) {
+ spin_unlock(&fsvq->lock);
+ ret = -ENOTCONN;
+ } else {
+ list_add_tail(&req->list, &fsvq->queued_reqs);
+ inc_in_flight_req(fsvq);
+ spin_unlock(&fsvq->lock);
+ return;
+ }
}
req->out.h.error = ret;
pr_err("virtio-fs: virtio_fs_enqueue_req() failed %d\n", ret);
@@ -1669,6 +1750,9 @@ static void virtio_fs_conn_destroy(struct fuse_mount *fm)
spin_lock(&fsvq->lock);
fsvq->connected = false;
spin_unlock(&fsvq->lock);
+
+ /* Nothing may be queued after the drain below */
+ virtio_fs_stop_new_requests(vfs);
virtio_fs_drain_all_queues(vfs);
fuse_conn_destroy(fm);
--
2.54.0 (Apple Git-157)
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH v3] fuse: virtio-fs: stop new requests before draining queues
2026-11-09 2:39 [PATCH v3] fuse: virtio-fs: stop new requests before draining queues Qihang
@ 2026-11-09 3:16 ` sashiko-bot
2026-11-12 3:09 ` Li Wang
1 sibling, 0 replies; 3+ messages in thread
From: sashiko-bot @ 2026-11-09 3:16 UTC (permalink / raw)
To: Qihang; +Cc: virtualization, Michael S. Tsirkin, Eugenio Perez
Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] fuse: virtio-fs: hang in virtio_fs_drain_queue() on stranded
queued_reqs
--
commit 01cb846c58c4f5ed59cf9d9ee391ec0c2e114c82
Author: Qihang <q.h.hack.winter@gmail.com>
fuse: virtio-fs: stop new requests before draining queues
This commit adds a per-virtqueue aborting flag to reject new requests
(other than FUSE_DESTROY) before draining queues during teardown, and
offloads blocking request end callbacks from the dispatch worker.
> diff --git a/fs/fuse/virtio_fs.c b/fs/fuse/virtio_fs.c
> index ba8d79c4bcac6..a46536c9fdc8e 100644
> --- a/fs/fuse/virtio_fs.c
> +++ b/fs/fuse/virtio_fs.c
[ ... ]
> @@ -593,9 +633,17 @@ static void virtio_fs_request_dispatch_work(struct work_struct *work)
> if (ret < 0) {
> if (ret == -ENOSPC) {
> spin_lock(&fsvq->lock);
> - list_add_tail(&req->list, &fsvq->queued_reqs);
> - spin_unlock(&fsvq->lock);
> - return;
> + if (!fsvq->connected ||
> + (fsvq->aborting &&
> + req->in.h.opcode != FUSE_DESTROY)) {
> + spin_unlock(&fsvq->lock);
> + ret = -ENOTCONN;
> + } else {
> + list_add_tail(&req->list,
> + &fsvq->queued_reqs);
> + spin_unlock(&fsvq->lock);
> + return;
> + }
> }
[ ... ]
> @@ -1226,6 +1274,28 @@ static void virtio_fs_stop_all_queues(struct virtio_fs *fs)
> }
> }
>
> +/*
> + * Stop admitting new requests before draining the queues. Admission and
> + * virtqueue submission are serialized by fsvq->lock, so a request is
> + * either rejected before reaching the device or accounted in in_flight
> + * and covered by the drain.
> + *
> + * FUSE_DESTROY is exempt because fuse_conn_destroy() sends it after the
> + * first drain and waits for its completion before aborting the channel.
> + */
> +static void virtio_fs_stop_new_requests(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->aborting = true;
> + spin_unlock(&fsvq->lock);
> + }
> +}
[ ... ]
> @@ -1551,12 +1622,22 @@ static void virtio_fs_send_req(struct fuse_iqueue *fiq, struct fuse_req *req)
> /*
> * Virtqueue full. Retry submission from worker
> * context as we might be holding fc->chan->bg_lock.
> + * Re-check admission under the same lock that
> + * increments in_flight: teardown may have started
> + * between enqueue_req() dropping its lock and here.
> */
> spin_lock(&fsvq->lock);
> - list_add_tail(&req->list, &fsvq->queued_reqs);
> - inc_in_flight_req(fsvq);
> - spin_unlock(&fsvq->lock);
> - return;
> + if (!fsvq->connected ||
> + (fsvq->aborting &&
> + req->in.h.opcode != FUSE_DESTROY)) {
> + spin_unlock(&fsvq->lock);
> + ret = -ENOTCONN;
> + } else {
> + list_add_tail(&req->list, &fsvq->queued_reqs);
> + inc_in_flight_req(fsvq);
> + spin_unlock(&fsvq->lock);
> + return;
> + }
> }
[Severity: Medium]
Can a request added to fsvq->queued_reqs on -ENOSPC still be stranded with
fsvq->in_flight > 0 and hang virtio_fs_drain_queue()?
virtio_fs_enqueue_req() drops fsvq->lock before returning -ENOSPC, and
neither virtio_fs_send_req() nor virtio_fs_request_dispatch_work()
schedules fsvq->dispatch_work when placing req on fsvq->queued_reqs; they
rely on virtio_fs_requests_done_work() seeing a non-empty queued_reqs when
in-flight virtqueue requests complete.
If the virtqueue drains during the window where fsvq->lock is dropped,
and the caller then re-acquires fsvq->lock before teardown sets
fsvq->aborting, nothing kicks fsvq->dispatch_work:
CPU0 CPU1 (kworker)
virtio_fs_send_req()
virtio_fs_enqueue_req()
spin_lock(&fsvq->lock)
virtqueue_add_sgs() -> -ENOSPC
spin_unlock(&fsvq->lock)
virtio_fs_requests_done_work()
spin_lock(&fsvq->lock)
/* drains all vq buffers */
!list_empty(&fsvq->queued_reqs) -> false
spin_unlock(&fsvq->lock)
spin_lock(&fsvq->lock)
/* connected && !aborting */
list_add_tail(..., &fsvq->queued_reqs)
inc_in_flight_req(fsvq)
spin_unlock(&fsvq->lock)
When virtio_fs_conn_destroy() then runs virtio_fs_stop_new_requests() and
virtio_fs_drain_all_queues(), fsvq->vq is already empty so
virtio_fs_requests_done_work() never runs again, and virtio_fs_drain_queue()
waits forever in wait_for_completion(&fsvq->in_flight_zero) before
reaching flush_work(&fsvq->dispatch_work). (The same window exists in
virtio_fs_request_dispatch_work() after it removes req from queued_reqs,
and against virtio_fs_stop_all_queues() in virtio_fs_remove().)
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261109023957.34500-1-q.h.hack.winter@gmail.com?part=1
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH v3] fuse: virtio-fs: stop new requests before draining queues
2026-11-09 2:39 [PATCH v3] fuse: virtio-fs: stop new requests before draining queues Qihang
2026-11-09 3:16 ` sashiko-bot
@ 2026-11-12 3:09 ` Li Wang
1 sibling, 0 replies; 3+ messages in thread
From: Li Wang @ 2026-11-12 3:09 UTC (permalink / raw)
To: Qihang, German Maglione, Vivek Goyal, Stefan Hajnoczi,
Miklos Szeredi
Cc: Eugenio Perez, virtualization, linux-fsdevel, fuse-devel,
linux-kernel
Hi Hang,
On 09/11/2026 10:39, Qihang wrote:
> virtio_fs_conn_destroy() drains the virtqueues and then calls
> fuse_conn_destroy(), which aborts the connection. The abort ends every
> request the fuse device still owns, and each ended request is freed as
> soon as its waiter wakes up.
>
> The drain only covers the requests that were already counted in
> fsvq->in_flight when it started. A request submitted after it returns is
> not covered, but is still handed to the device before the abort runs:
>
> CPU 0 (umount) CPU 1
> --------------------------------- --------------------------------
> virtio_fs_drain_all_queues()
> /* in_flight drops to 0 and
> the drain returns */
> virtio_fs_enqueue_req()
> virtqueue_add_sgs()
> inc_in_flight_req() -> 1
> fuse_conn_destroy()
> fuse_send_destroy()
> fuse_chan_abort()
> /* ends and frees the
> request above */
> fuse_chan_wait_aborted()
> /* the device still owns the
> virtqueue descriptors and
> DMA-writes into the freed
> request and its argbuf */
>
> Stop admitting requests on each queue before draining begins. The flag
> is set per queue, but that is sufficient: request admission,
> virtqueue_add_sgs() and, on the submission path, the in_flight
> increment are serialized by the same fsvq->lock. A request is
> therefore either rejected before the device can see it, or already
> counted in fsvq->in_flight and covered by the drain. There is no
> window in between.
>
> Rejected requests use the existing -ENOTCONN error path and are ended by
> virtio_fs_request_dispatch_work() through end_reqs, so no new request
> completion or ownership state is introduced. Nothing has to be
> discarded in the completion path, and fsvq->in_flight remains the only
> per-queue accounting state.
>
> The -ENOSPC path needs the same admission check. virtio_fs_enqueue_req()
> drops fsvq->lock before returning -ENOSPC, and the caller queues the
> request in a second critical section; teardown can finish in between and
> leave the request stranded on queued_reqs, which nothing drains.
> Re-check connected/aborting before queueing, in both the submission path
> and the dispatch worker. After aborting is set, no request can newly
> enter queued_reqs or the virtqueue, except the explicitly permitted
> FUSE_DESTROY path.
>
> Rejected requests are ended by virtio_fs_request_dispatch_work(). If
> their end callback may block (a DAX release), ending them inline can
> deadlock: the callback may issue a synchronous request that needs the
> same worker. Offload those endings to a separate work item, like the
> device completion path already does.
>
> The new state is not folded into fsvq->connected because the request
> queues have to stay usable: fuse_conn_destroy() sends FUSE_DESTROY
> through them, with args.force set so that fuse_simple_request() waits
> for the reply. Clearing fsvq->connected would make that request fail
> with -ENOTCONN and the server would never see the destroy.
>
> FUSE_DESTROY is the sole exception while aborting is set. It is sent
> only after the first drain, and its completion is waited for before
> fuse_chan_abort() can end requests, so the destroy is the only request
> the device sees after the drain. virtio-fs exposes no userspace
> /dev/fuse fd (and no_control disables the fusectl interface), so
> userspace cannot inject another request.
>
> virtio_fs_start_all_queues() clears the flag, so a mount that follows an
> unmount starts with admission enabled again.
>
> Fixes: a62a8ef9d97d ("virtio-fs: add virtiofs filesystem")
> Cc: stable@vger.kernel.org
> Signed-off-by: Qihang <q.h.hack.winter@gmail.com>
> ---
> V2 -> V3:
> - re-check admission before queueing on -ENOSPC, in virtio_fs_send_req()
> and in the dispatch worker, so teardown cannot strand a request on
> queued_reqs
> - offload may_block request endings in the dispatch worker to avoid a
> self-deadlock through fuse_simple_request()
> - add Fixes: and Cc: stable
> V1 -> V2:
> - prevent new requests from reaching the virtqueue at teardown time
> instead of skipping completions of aborted requests
> - keep FUSE_DESTROY working: it is the teardown request and is waited
> for before fuse_chan_abort()
> - drop the per-vq aborted flag and the completion-skip branch
> fs/fuse/virtio_fs.c | 104 +++++++++++++++++++++++++++++++++++++++-----
> 1 file changed, 94 insertions(+), 10 deletions(-)
>
> diff --git a/fs/fuse/virtio_fs.c b/fs/fuse/virtio_fs.c
> index ba8d79c4bcac6..a46536c9fdc8e 100644
> --- a/fs/fuse/virtio_fs.c
> +++ b/fs/fuse/virtio_fs.c
> @@ -55,6 +55,7 @@ struct virtio_fs_vq {
> struct work_struct dispatch_work;
> struct fuse_dev *fud;
> bool connected;
> + bool aborting; /* reject new requests except FUSE_DESTROY */
> long in_flight;
> struct completion in_flight_zero; /* No inflight requests */
> struct kobject *kobj;
> @@ -97,6 +98,11 @@ struct virtio_fs_req_work {
> struct work_struct done_work;
> };
>
> +struct virtio_fs_end_work {
> + struct work_struct work;
> + struct fuse_req *req;
> +};
We can avoid introducing it and reuse virtio_fs_req_work instead.
Thanks,
Li Wang
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-11-12 3:09 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-09 2:39 [PATCH v3] fuse: virtio-fs: stop new requests before draining queues Qihang
2026-11-09 3:16 ` sashiko-bot
2026-11-12 3:09 ` Li Wang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox