Re: [PATCH v6 4/6] fuse: support registered buffer pools in io-uring

On Tue, Aug 11, 2026 at 3:45 PM Bernd Schubert <[email protected]> wrote:
>
>
>
> On 7/16/26 19:59, Joanne Koong wrote:
> > Allow servers to use a buffer pool that is also registered through
> > io-uring. When the server registers a buffer pool with io-uring, the
> > pages backing the pool are pinned upfront. This eliminates the overhead
> > of pinning/unpinning user pages and translating virtual addresses per
> > i/o request. This also allows servers to use the same registered memory
> > for subsequent backing store I/O (eg read_fixed/write_fixed), keeping
> > data in the same pinned pages without additional pinning or mapping
> > overhead required.
> >
> > To use this, the server needs to set the FUSE_URING_REGISTERED_BUFPOOL
> > flag when adding a bufpool through the FUSE_IO_URING_CMD_ADD_BUFPOOL
> > cmd. For every sqe submitted (including the one for adding the bufpool),
> > it should set sqe->uring_cmd_flags to include IORING_URING_CMD_FIXED,
> > and pass in the index where the registered bufpool resides to
> > sqe->buf_index.
> >
> > Benchmarked with passthrough_hp (--nopassthrough, q_depth=8) on a
> > 2-socket Intel Xeon Gold 6138 (40 cores / 80 threads), using fio (sync
> > engine, bs=1M, O_DIRECT, numjobs=2, 30s run + 10s ramp, 3 runs) where
> > direct-I/O throughput is against a RAM-backed (tmpfs) source (backing
> > I/O is not the bottleneck):
> >
> >                   baseline      registered buffers
> >   direct read       ~5.1 GB/s     ~5.4 GB/s   (+~5%)
> >   direct write      ~3.4 GB/s     ~4.8 GB/s   (+~45%)
> >
> > Registered buffers bring up the write path speed up closer to speed of
> > reads. There isn't much improvement for reads because it is already fast
> > enough where it's at the copy-bound ceiling (surpassing that requires
> > doing zero-copy). On a device-bound NVMe though, the differences are
> > within noise, as backing I/O dominates per-request latency.
> >
> > Signed-off-by: Joanne Koong <[email protected]>
> > ---
> >  fs/fuse/dev_uring.c       | 122 +++++++++++++++++++++++++++++++-------
> >  fs/fuse/dev_uring_i.h     |   8 +++
> >  include/uapi/linux/fuse.h |   4 ++
> >  3 files changed, 112 insertions(+), 22 deletions(-)
> >
> > diff --git a/fs/fuse/dev_uring.c b/fs/fuse/dev_uring.c
> > index e9da92b46090..ed7e783bd68d 100644
> > --- a/fs/fuse/dev_uring.c
> > +++ b/fs/fuse/dev_uring.c
> > @@ -22,6 +22,8 @@ MODULE_PARM_DESC(enable_uring,
> >  #define FUSE_URING_IOV_HEADERS 0
> >  #define FUSE_URING_IOV_PAYLOAD 1
> >
> > +#define FUSE_URING_ADD_BUFPOOL_FLAGS (FUSE_URING_REGISTERED_BUFPOOL)
> > +
> >  bool fuse_uring_enabled(void)
> >  {
> >       return enable_uring;
> > @@ -47,6 +49,27 @@ static inline bool bufpool_enabled(struct fuse_ring_queue *queue)
> >       return queue->payload_mode == FUSE_PAYLOAD_BUFPOOL;
> >  }
> >
> > +static inline bool bufpool_registered(struct fuse_ring_queue *queue)
> > +{
> > +     return queue->bufpool && queue->bufpool->registered;
> > +}
> > +
> > +/*
> > + * For a registered bufpool, every sqe that drives a payload import (REGISTER,
> > + * COMMIT_AND_FETCH) must carry the registered buffer index of the pool.
> > + * This also must be called from the command's issue handler, where cmd->sqe is
> > + * still valid
> > + */
> > +static inline bool fuse_uring_cmd_index_ok(struct io_uring_cmd *cmd,
> > +                                        struct fuse_ring_queue *queue)
> > +{
> > +     if (!bufpool_registered(queue))
> > +             return true;
> > +
> > +     return (cmd->flags & IORING_URING_CMD_FIXED) &&
> > +            READ_ONCE(cmd->sqe->buf_index) == queue->bufpool->registered_index;
> > +}
> > +
> >  static void uring_cmd_set_ring_ent(struct io_uring_cmd *cmd,
> >                                  struct fuse_ring_ent *ring_ent)
> >  {
> > @@ -653,19 +676,42 @@ static int copy_header_from_ring(struct fuse_ring_ent *ent,
> >       return 0;
> >  }
> >
> > +static int fuse_uring_import_payload(struct fuse_ring_ent *ent, int dir,
> > +                                  struct iov_iter *iter,
> > +                                  unsigned int issue_flags)
> > +{
> > +     void __user *base = ent->payload.iov_base;
> > +     size_t len = ent->payload.iov_len;
> > +     int err = 0;
> > +
> > +     if (!base) {
> > +             memset(iter, 0, sizeof(*iter));
> > +             return 0;
> > +     }
> > +
> > +     if (bufpool_registered(ent->queue))
> > +             err = io_uring_cmd_import_fixed((u64)(uintptr_t)base, len, dir,
> > +                                             iter, ent->cmd, issue_flags);
> > +     else
> > +             err = import_ubuf(dir, base, len, iter);
>
> Hmm ok, just an annotation, I have to admit that I don't like
> issue_flags too much. Especially for the optimization that DDN has by
> avoiding io_uring_cmd_complete_in_task(), IO_URING_F_UNLOCKED is rather
> painful. Additionaly with that optimization the lock might get
> contended. The new addition here doesn't introduce it, but also doesn't
> make it any better.

For what it's worth, the locks aren't taken on these paths (eg sends
go through fuse_uring_dispatch_ent() ->
io_uring_cmd_complete_in_task() and task work runs with
ctx->uring_lock held, and commits run in the command's issue handler,
where the lock is likewise held).

Threading the issue_flags through gets a bit annoying, but I
personally don't see a better way for core io-uring infrastructure to
determine whether a given call path already holds the lock or not.

>
> > +
> > +     if (err)
> > +             pr_info_ratelimited("fuse: Import of user buffer failed\n");
> > +
> > +     return err;
> > +}
> > +
> > @@ -1362,8 +1421,21 @@ static int fuse_uring_add_bufpool(struct io_uring_cmd *cmd,
> >       uintptr_t pool_uaddr;
> >       unsigned int pool_len, nr_bufs;
> >       size_t pool_size, buf_size;
> > +     bool registered = flags & FUSE_URING_REGISTERED_BUFPOOL;
> > +
> > +     if (!ring || qid >= ring->nr_queues)
> > +             return -EINVAL;
> >
> > -     if (!ring || qid >= ring->nr_queues || flags)
> > +     if (flags & ~FUSE_URING_ADD_BUFPOOL_FLAGS)
> > +             return -EINVAL;
> > +
> > +     /*
> > +      * A registered bufpool is reached through an io_uring fixed buffer, so
> > +      * this command must be submitted with IORING_URING_CMD_FIXED and the
> > +      * registered buffer index in sqe->buf_index. That index is recorded
> > +      * below and enforced on every REGISTER / COMMIT_AND_FETCH sqe.
> > +      */
> > +     if (registered && !(cmd->flags & IORING_URING_CMD_FIXED))
> >               return -EINVAL;
>
> Hmm, why do we actually need FUSE_URING_REGISTERED_BUFPOOL if the
> decision can be made by IORING_URING_CMD_FIXED?

It's not strictly needed, but imo it makes uapi more explicit and the
contract more clear. I don't feel strongly about this though, I'm fine
with either approach.

>
> >
> > diff --git a/fs/fuse/dev_uring_i.h b/fs/fuse/dev_uring_i.h
> > index cdf56f8b38b5..e142cae43022 100644
> > --- a/fs/fuse/dev_uring_i.h
> > +++ b/fs/fuse/dev_uring_i.h
> > @@ -49,6 +49,14 @@ enum fuse_queue_payload_mode {
> >  };
> >
> >  struct fuse_bufpool {
> > +     bool registered;
> > +
> > +     /*
> > +      * io_uring registered buffer table index for this pool, bound at
> > +      * ADD_BUFPOOL time. Only valid if the bufpool is registered
> > +      */
> > +     u16 registered_index;
>
> Dunno, could this go into the previous commit or be reserved in the
> previous commit? This addition make bisect hard due to ABI change?

This is an addition to a kernel-internal struct, not an uapi-defined
one, so I don't think this affects ABI. The previous commit doesn't
have any notion of registered buffers, so I think moving these
definitions to the previous commit would be confusing.

Thanks,
Joanne