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

On 8/19/26 22:29, Joanne Koong wrote:
> On Wed, Aug 19, 2026 at 1:05 PM Bernd Schubert <[email protected]> wrote:
>>
>>
>>
>> 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.
> 
> This is pre-existing behavior that's been there since the beginning
> (kernel version 6.14) [1]. I don't think we can change this now, or
> it'll break backwards compatibility, like Miklos's loraw program.
> 
>>
>> <... 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);
>>
> 
> The condition check in fuse_uring_cmd() is an && and not an ||. It

Aaaarg. That makes the entire condition useless :(

> doesn't enforce that the server must have sent over FUSE_OVER_IO_URING
> in the init reply to use fuse io-uring. With enable_uring=1 (which is
> needed for the kernel to advertise FUSE_OVER_IO_URING in the init
> request in the first place), the fuse io-uring logic proceeds
> regardless of whether the server replied with the FUSE_OVER_IO_URING
> flag or not.
> 
> Thanks,
> Joanne
> 
> [1] https://elixir.bootlin.com/linux/v6.14-rc1/source/fs/fuse/inode.c#L1455

The comment there is definitely outdated, at least it was *supposed* to.

Well, then we have a problem because we have a lock order inversion
issue. After adding in the reduce-queue series I had planned to work on
distributing fch->num_background among queues, so that each queue gets
its own num_background, so that holding two locks like in
fuse_uring_queue_bq_req() wouldn't be needed anymore.

Also again, without FUSE_OVER_IO_URING reply the condition in
fuse_block_alloc() doesn't make any sense.


Thanks,
Bernd