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

On 2026/8/21 00:16, Joanne Koong wrote:
> On Thu, Aug 20, 2026 at 1:02 AM Baokun Li <[email protected]> wrote:
>> Hi all,
>>
>> On 2026/8/20 04:05, Bernd Schubert 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.
>> I share Bernd's concern here. Allowing io-uring without
>> FUSE_OVER_IO_URING means enabling a capability beyond what was
>> negotiated. We should honor the negotiated feature set, and print
>> the negotiated flags to dmesg at INIT time so issues like this are
>> easy to spot.
> Not sure if you missed this reply [1], but will copy and paste it here:
>
> This is pre-existing behavior that's been there since the beginning
> (kernel version 6.14). I don't think we can change this now, or
> it'll break backwards compatibility, like Miklos's loraw program.

Yes, I saw it, but pre-existing doesn't necessarily mean correct.
Enabling capabilities beyond what was negotiated easily introduces
inconsistencies.

In our recent hardening work we found that the fch->io_uring switch
causes anomalies in several mechanisms (mostly in our downstream tree),
which is why I replied in Bernd's thread — he raised the
enable_uring / fch->io_uring bypass there.

>>> <... 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;
>>>       }
>> BTW, the current code clears fch->io_uring on REGISTER failure:
>>
>>                 if (err) {
>>                         fch->io_uring = 0;
>>                         wake_up_all(&fch->blocked_waitq);
>>                         return err;
>>                 }
>>
>> But by then other entries may have already registered successfully,
>> fiq->ops is switched to fuse_io_uring_ops, and requests are flowing
>> through the uring path. Clearing fch->io_uring here may reject all
>> subsequent io_uring commands with -EOPNOTSUPP while the data path
>> is still running on uring ops.
>>
> This is also pre-existing behavior. The check in fuse_uring_cmd() is a
> && not an ||. It'll only return -EOPNOTSUPP if admin disables
> enable_uring.

Yes, that's exactly the problem. Right now we have several flags
with unclear boundaries:

  enable_uring: user-controlled, can be toggled at any time
  fch->io_uring: set to 1 at INIT negotiation, but cleared to 0
      on any entry registration failure — even if other entries
      have already registered successfully and fiq->ops has been
      switched to uring ops. The two states are inconsistent.
  ring->ready: set when at least one entry has registered

I think the clean design would be: at INIT time, decide based on
enable_uring and the negotiated flags whether to initialize uring
and set fch->io_uring. After that, enable_uring is no longer
consulted — fch->io_uring is write-once. When ring->ready (at
least one entry registered), take the io_uring path; otherwise
fall back to the classic /dev/fuse path. This way each
variable has a single, clear semantic.


Thanks,
Baokun