Re: [PATCH 2/4] export/fuse: process FUSE-over-io_uring requests

[Stefan Hajnoczi <stefanha@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 5131 bytes --]

On Mon, Sep 08, 2025 at 03:09:57PM -0400, Brian Song wrote:
> 
> 
> On 9/3/25 7:51 AM, Stefan Hajnoczi wrote:
> > On Fri, Aug 29, 2025 at 10:50:23PM -0400, Brian Song wrote:
> > > https://docs.kernel.org/filesystems/fuse-io-uring.html
> > > 
> > > As described in the kernel documentation, after FUSE-over-io_uring
> > > initialization and handshake, FUSE interacts with the kernel using
> > > SQE/CQE to send requests and receive responses. This corresponds to
> > > the "Sending requests with CQEs" section in the docs.
> > > 
> > > This patch implements three key parts: registering the CQE handler
> > > (fuse_uring_cqe_handler), processing FUSE requests (fuse_uring_co_
> > > process_request), and sending response results (fuse_uring_send_
> > > response). It also merges the traditional /dev/fuse request handling
> > > with the FUSE-over-io_uring handling functions.
> > > 
> > > Suggested-by: Kevin Wolf <kwolf@redhat.com>
> > > Suggested-by: Stefan Hajnoczi <stefanha@redhat.com>
> > > Signed-off-by: Brian Song <hibriansong@gmail.com>
> > > ---
> > >   block/export/fuse.c | 457 ++++++++++++++++++++++++++++++--------------
> > >   1 file changed, 309 insertions(+), 148 deletions(-)
> > > 
> > > diff --git a/block/export/fuse.c b/block/export/fuse.c
> > > index 19bf9e5f74..07f74fc8ec 100644
> > > --- a/block/export/fuse.c
> > > +++ b/block/export/fuse.c
> > > @@ -310,6 +310,47 @@ static const BlockDevOps fuse_export_blk_dev_ops = {
> > >   };
> > >   #ifdef CONFIG_LINUX_IO_URING
> > > +static void coroutine_fn fuse_uring_co_process_request(FuseRingEnt *ent);
> > > +
> > > +static void coroutine_fn co_fuse_uring_queue_handle_cqes(void *opaque)
> > 
> > This function appears to handle exactly one cqe. A singular function
> > name would be clearer than a plural: co_fuse_uring_queue_handle_cqe().
> > 
> > > +{
> > > +    FuseRingEnt *ent = opaque;
> > > +    FuseExport *exp = ent->rq->q->exp;
> > > +
> > > +    /* Going to process requests */
> > > +    fuse_inc_in_flight(exp);
> > 
> > What is the rationale for taking a reference here? Normally something
> > already holds a reference (e.g. the request itself) and it will be
> > dropped somewhere inside a function we're about to call, but we still
> > need to access exp afterwards, so we temporarily take a reference.
> > Please document the specifics in a comment.
> > 
> > I think blk_exp_ref()/blk_exp_unref() are appropriate instead of
> > fuse_inc_in_flight()/fuse_dec_in_flight() since we only need to hold
> > onto the export and don't care about drain behavior.
> > 
> 
> Stefan:
> 
> When handling FUSE requests, we don’t want the FuseExport to be accidentally
> deleted. Therefore, we use fuse_inc_in_flight in the CQE handler to
> increment the in_flight counter, and when a request is completed, we call
> fuse_dec_in_flight to decrement it. Once the last request has been
> processed, fuse_dec_in_flight brings the in_flight counter down to 0,
> indicating that the export can safely be deleted. The usage of in_flight
> follows the same logic as in traditional FUSE request handling.
> 
> Since submitted SQEs for FUSE cannot be canceled, once we register or commit
> them we must wait for the kernel to return a CQE. Otherwise, the kernel may
> deliver a CQE and invoke its handler after the export has already been
> deleted. For this reason, we directly call blk_exp_ref and blk_exp_unref
> when submitting an SQE and when receiving its CQE, to explicitly control the
> export reference and prevent accidental deletion.
> 
> The doc/comment for co_fuse_uring_queue_handle_cqe:
> 
> Protect FuseExport from premature deletion while handling FUSE requests. CQE
> handlers inc/dec the in_flight counter; when it reaches 0, the export can be
> freed. This follows the same logic as traditional FUSE.
> 
> Since FUSE SQEs cannot be canceled, a CQE may arrive after commit even if
> the export is deleted. To prevent this, we ref/unref the export explicitly
> at SQE submission and CQE completion.

I looked at your "final" branch on GitHub and the refcount changes there
match what I was thinking of.

In case it helps for writing comments, I'll try to describe my mental
model of the refcounts:

- fuse_inc_in_flight()/fuse_dec_in_flight() must wrap the lifecycle of
  FUSE requests that the server is processing. This ensures that the
  block layer's drain operation waits for requests to complete and that
  the export cannot be deleted while the requests are still in progress.

- blk_exp_ref()/blk_exp_unref() prevents the export from being deleted
  while something that still depends on it remains outstanding.

How this maps to FUSE-over-io_uring:

- When an SQE is submitted blk_exp_ref() must be called. After the CQE
  has been processed, blk_exp_unref() must be called. This way the
  export cannot be deleted before all CQEs have been handled.

- The coroutine that processes a FUSE request must call
  fuse_inc_in_flight() before processing begins and fuse_dec_in_flight()
  after processing ends.

Thanks,
Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]