Re: [PATCH v4 19/25] fuse: add io-uring kernel-managed buffer ring

[Joanne Koong <joannelkoong@gmail.com>]

On Wed, Feb 25, 2026 at 9:55 AM Bernd Schubert <bernd@bsbernd.com> wrote:
> On 1/28/26 22:44, Bernd Schubert wrote:
> > On 1/17/26 00:30, Joanne Koong wrote:
> >> diff --git a/fs/fuse/dev_uring.c b/fs/fuse/dev_uring.c
> >> @@ -940,6 +1188,7 @@ static int fuse_uring_commit_fetch(struct io_uring_cmd *cmd, int issue_flags,
> >>      unsigned int qid = READ_ONCE(cmd_req->qid);
> >>      struct fuse_pqueue *fpq;
> >>      struct fuse_req *req;
> >> +    bool send;
> >>
> >>      err = -ENOTCONN;
> >>      if (!ring)
> >> @@ -990,7 +1239,12 @@ static int fuse_uring_commit_fetch(struct io_uring_cmd *cmd, int issue_flags,
> >>
> >>      /* without the queue lock, as other locks are taken */
> >>      fuse_uring_prepare_cancel(cmd, issue_flags, ent);
> >> -    fuse_uring_commit(ent, req, issue_flags);
> >> +
> >> +    err = fuse_uring_headers_prep(ent, ITER_SOURCE, issue_flags);
> >> +    if (err)
> >> +            fuse_uring_req_end(ent, req, err);
> >> +    else
> >> +            fuse_uring_commit(ent, req, issue_flags);
> >>
> >>      /*
> >>       * Fetching the next request is absolutely required as queued
> >> @@ -998,7 +1252,9 @@ static int fuse_uring_commit_fetch(struct io_uring_cmd *cmd, int issue_flags,
> >>       * and fetching is done in one step vs legacy fuse, which has separated
> >>       * read (fetch request) and write (commit result).
> >>       */
> >> -    if (fuse_uring_get_next_fuse_req(ent, queue))
> >> +    send = fuse_uring_get_next_fuse_req(ent, queue, issue_flags);
> >> +    fuse_uring_headers_cleanup(ent, issue_flags);
> >> +    if (send)
> >>              fuse_uring_send(ent, cmd, 0, issue_flags);
> >>      return 0;
>
>
> Hello Joanne,
>
> couldn't it call fuse_uring_headers_cleanup() before the
> fuse_uring_get_next_fuse_req()? I find it a bit confusing that it firsts
> gets the next request and then cleans up the buffer from the previous
> request.

Hi Bernd,

Thanks for taking a look.

The fuse_uring_headers_cleanup() call has to happen after the
fuse_uring_get_next_fuse_req() call because
fuse_uring_get_next_fuse_req() copies payload to the header, so we
can't yet relinquish the refcount on the headers buffer / clean it up
yet. I can add a comment about this to make this more clear.

>
> As I understand it, the the patch basically adds the feature of 0-byte
> payloads. Maybe worth mentioning in the commit message?

Hmm I'm not really sure I am seeing where the 0-byte payload gets
added. On the server side, they don't receive payloads that are
0-bytes. If there is no next fuse request to send, then nothing gets
sent. But maybe I'm not interpreting your comment about 0-byte
payloads correctly?

> I also wonder if it would be worth to document as code comment that
> fuse_uring_ent_assign_req / fuse_uring_next_req_update_buffer are
> allowed to fail for a buffer upgrade (i.e. 0 to max-payload). At least

Good idea, I'll add a comment about this.

> the current comment of  "Fetching the next request is absolutely
> required" is actually not entirely true anymore.
>

I don't think this patch introduces new behavior on this front.
fuse_uring_get_next_fuse_req() is still called to fetch the next
request AFAICS.

Thanks,
Joanne
>
> Thanks,
> Bernd