Re: [PATCH v2] fuse: virtio-fs: stop new requests before draining queues

[Qihang <q.h.hack.winter@gmail.com>]

Hi Sashiko,

Thanks for the review.  Both Medium findings are valid; v3 fixes them
and adds the missing Fixes tag.  The v3 patch is a separate thread.

#1 (-ENOSPC can bypass aborting)

Agreed.  virtio_fs_enqueue_req() drops fsvq->lock before returning
-ENOSPC, and virtio_fs_send_req() queues the request in a second
critical section.  Teardown can complete in between and leave the
request stranded on queued_reqs, which fuse_chan_abort() does not walk.

v3 re-checks connected/aborting in that second critical section, and in
the dispatch worker's -ENOSPC requeue path.  A request now either joins
queued_reqs before aborting is set (and is counted in in_flight), or is
rejected with -ENOTCONN and ended through end_reqs.  After aborting is
set, no request can newly enter queued_reqs or the virtqueue, except
FUSE_DESTROY.

I instrumented the window: enqueue returns -ENOSPC for a FUSE_STATFS,
send_req() waits before re-checking, and conn_destroy() sets aborting
and drains in that window.  Three runs:

  VF_INV: q=requests.0 in_flight=0 queued=0 connected=1 aborting=1
  VF_ENOSPC: rejected
  VF_ENOSPC: STATFS returned -107
  VF_INV: q=requests.0 in_flight=0 queued=0 connected=0 aborting=1
  umount rc=0

No stranded request, no warning.

#2 (dispatch worker may_block self-deadlock)

Agreed as well.  Rejecting requests earlier means a DAX release can be
ended by virtio_fs_request_dispatch_work(); its end callback issues a
synchronous removemapping request, which is rejected and needs the same
worker.

v3 offloads endings with req->args->may_block to a separate work item
that only calls fuse_request_end().  virtio_fs_request_complete()
cannot be reused: it also consumes argbuf and drops in_flight, while a
rejected request has neither.

I verified the path with a synthetic callback: a background STATFS with
may_block=true and an end callback that issues a synchronous STATFS.

  VF_DEADLOCK: queue outer
  VF_DEADLOCK: offload may_block end
  VF_DEADLOCK: outer end error=-107
  VF_DEADLOCK: inner returned -107
  VF_DEADLOCK: outer end done
  umount rc=0

The old inline fuse_request_end() would stop at "outer end error" and
wait for itself.

#3 (missing Fixes)

Added:

  Fixes: a62a8ef9d97d ("virtio-fs: add virtiofs filesystem")
  Cc: stable@vger.kernel.org

Thanks,
Qihang