Re: [PATCH v7 1/6] fuse: decouple fuse_ring creation from ent registration

On 8/19/26 19:56, Joanne Koong wrote:
> On Wed, Aug 19, 2026 at 4:35 AM Miklos Szeredi <[email protected]> wrote:
>>
>> On Fri, 14 Aug 2026 at 21:00, Joanne Koong <[email protected]> wrote:
>>>
>>> Currently, the connection's fuse_ring is created lazily on the first
>>> FUSE_IO_URING_CMD_REGISTER command. A server registers entries from one
>>> thread per queue (one per CPU) and those threads issue their first
>>> REGISTER command concurrently. They then race to create the single
>>> per-connection fuse_ring, which required open-coded handling in
>>> fuse_uring_create() to detect and protect against concurrent creations.
>>>
>>> Decouple fuse_ring creation from ent registration and move it to
>>> FUSE_INIT reply processing after a server has negotiated and set
>>> FUSE_OVER_IO_URING. The ring is published before the connection is
>>> marked initialized. fuse_uring_register() no longer creates the ring and
>>> it instead uses the ring set up at init time.
>>
>> I tested this with loraw (a "raw" loopback tester that doesn't use
>> libfuse) and it fails with
>>
>> root@kvm:~# ./loraw -u  /mnt/fuse
>> loraw: loraw.c:1010: lo_start_uring: Assertion `!cqe->res' failed.
>>
>> cqe->res is -22 (EINVAL).
>>
>> Attaching the reproducer.  To compile:
>>
>> cp $(KERNEL_TREE)/include/uapi/linux/fuse.h fuse_kernel.h
>> gcc loraw.c -oloraw -luring
>>
> 
> Thanks for attaching the repro.
> 
> This is happening because this patch uses the FUSE_OVER_IO_URING init
> reply as a signal that the ring should be created, but I missed that
> the FUSE_OVER_IO_URING reply is *optional*.
> 
> Prior to this patch, there's two scenarios:
> a) server sets FUSE_OVER_IO_URING reply at init time - requests will
> automatically block until fuse-io-uring is completely set up
> b) server does not set FUSE_OVER_IO_URING but later sends uring
> register request - requests will continue along /dev/fuse path until
> fuse-io-uring is completely set up
> 
> Libfuse sets FUSE_OVER_IO_URING in the reply, but the loraw.c server does not.
> 
> I think the best way to fix this is to have the ring creation happen
> when the kernel receives the first io-uring command instead of at
> FUSE_INIT or at FUSE_IO_URING_CMD_REGISTER ent creation time, given
> that FUSE_IO_URING_ADD_QUEUE needs the ring to exist:

I don't think we should allow io-uring without FUSE_OVER_IO_URING and 
I really thought that was disabled. 

<... checking the code ...>

I'm on a ublk branch without your commits a applied, i.e. plain upstream 7.2 fuse

fuse_uring_cmd()
	/* Once a connection has io-uring enabled on it, it can't be disabled */
	if (!enable_uring && !fch->io_uring) {
		pr_info_ratelimited("fuse-io-uring is disabled\n");
		return -EOPNOTSUPP;
	}



In process_init_reply()

if (flags & FUSE_OVER_IO_URING && fuse_uring_enabled())
    fuse_chan_io_uring_enable(fc->chan);


And this is also absolutely needed to block requests in fuse_block_alloc(),
which is a requirement to avoid lock order issues between queue->lock and
fch->bg_lock (at least I believe that has not been solved yet).


I'm going to try Miklos' reproducer in a bit, but one way or the other
let's please not allow io-uring without FUSE_OVER_IO_URING reply.


Thanks,
Bernd


> 
> Subject: [PATCH] fuse: create fuse_ring on the first io-uring command
> 
> Commit 6330b1f61ed1 ("fuse: decouple fuse_ring creation from ent
> registration") moved fuse_ring creation to FUSE_INIT reply processing,
> gated on the server setting FUSE_OVER_IO_URING in its reply flags.
> 
> However, that flag is optional. Libfuse sets it but servers not going
> through libfuse may not.
> 
> Create the ring on the first io-uring command instead, independent of
> what the server negotiated. Ring creation stays decoupled from ent
> registration, which FUSE_IO_URING_CMD_ADD_QUEUE depends on since it
> needs the ring to exist before any entry is registered.
> 
> Fixes: 6330b1f61ed1 ("fuse: decouple fuse_ring creation from ent registration")
> Reported-by: Miklos Szeredi <[email protected]>
> Signed-off-by: Joanne Koong <[email protected]>
> ---
>  fs/fuse/dev.c         |  2 +-
>  fs/fuse/dev_uring.c   | 18 +++++++++++-------
>  fs/fuse/dev_uring_i.h |  5 -----
>  3 files changed, 12 insertions(+), 13 deletions(-)
> 
> diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
> index 4fec31fc0b84..a665d76292c9 100644
> --- a/fs/fuse/dev.c
> +++ b/fs/fuse/dev.c
> @@ -77,7 +77,7 @@ void fuse_chan_set_initialized(struct fuse_chan
> *fch, struct fuse_chan_param *pa
>                 fch->max_pages = param->max_pages;
> 
>                 if (param->io_uring_enabled)
> -                       fuse_uring_conn_init(fch);
> +                       fch->io_uring = 1;
>         }
> 
>         /* Pairs with smp_load_acquire() readers of fch->initialized */
> diff --git a/fs/fuse/dev_uring.c b/fs/fuse/dev_uring.c
> index e22a48c9a678..23c26099d159 100644
> --- a/fs/fuse/dev_uring.c
> +++ b/fs/fuse/dev_uring.c
> @@ -303,6 +303,7 @@ static struct fuse_ring *fuse_uring_create(struct
> fuse_chan *fch)
>  {
>         struct fuse_ring *ring;
>         size_t nr_queues = num_possible_cpus();
> +       struct fuse_ring *res = NULL;
>         size_t max_payload_size;
> 
>         ring = kzalloc_obj(*ring, GFP_KERNEL_ACCOUNT);
> @@ -322,6 +323,12 @@ static struct fuse_ring *fuse_uring_create(struct
> fuse_chan *fch)
>                 spin_unlock(&fch->lock);
>                 goto out_err;
>         }
> +       if (fch->ring) {
> +               /* race, another thread created the ring in the meantime */
> +               spin_unlock(&fch->lock);
> +               res = fch->ring;
> +               goto out_err;
> +       }
> 
>         init_waitqueue_head(&ring->stop_waitq);
> 
> @@ -336,13 +343,7 @@ static struct fuse_ring *fuse_uring_create(struct
> fuse_chan *fch)
>  out_err:
>         kfree(ring->queues);
>         kfree(ring);
> -       return NULL;
> -}
> -
> -void fuse_uring_conn_init(struct fuse_chan *fch)
> -{
> -       if (fuse_uring_create(fch))
> -               fch->io_uring = 1;
> +       return res;
>  }
> 
>  static struct fuse_ring_queue *fuse_uring_create_queue(struct fuse_ring *ring,
> @@ -1685,6 +1686,9 @@ int fuse_uring_cmd(struct io_uring_cmd *cmd,
> unsigned int issue_flags)
>         if (!smp_load_acquire(&fch->initialized))
>                 return -EAGAIN;
> 
> +       if (!smp_load_acquire(&fch->ring) && !fuse_uring_create(fch))
> +               return -ENOMEM;
> +
>         switch (cmd_op) {
>         case FUSE_IO_URING_CMD_REGISTER:
>                 err = fuse_uring_register(cmd, issue_flags, fch);
> diff --git a/fs/fuse/dev_uring_i.h b/fs/fuse/dev_uring_i.h
> index 263d0f8b9714..3233b07430d2 100644
> --- a/fs/fuse/dev_uring_i.h
> +++ b/fs/fuse/dev_uring_i.h
> @@ -184,7 +184,6 @@ struct fuse_ring {
>         bool ready;
>  };
> 
> -void fuse_uring_conn_init(struct fuse_chan *fch);
>  void fuse_uring_stop_queues(struct fuse_ring *ring);
>  void fuse_uring_abort_end_requests(struct fuse_ring *ring);
>  int fuse_uring_cmd(struct io_uring_cmd *cmd, unsigned int issue_flags);
> @@ -224,10 +223,6 @@ static inline bool fuse_uring_ready(struct fuse_chan *fch)
> 
>  #else /* CONFIG_FUSE_IO_URING */
> 
> -static inline void fuse_uring_conn_init(struct fuse_chan *fch)
> -{
> -}
> -
>  static inline void fuse_uring_abort(struct fuse_chan *fch)
>  {
>  }
> --
> 2.52.0
> 
> 
> If you'd prefer an inline replacement for the original commit instead
> of a fixup patch on top of the tree, please let me know and I'd be
> happy to send that over. Whatever would be easiest for you.
> 
> Thanks,
> Joanne