Re: [PATCH v1 07/30] io_uring/rsrc: add fixed buffer table pinning/unpinning

[Caleb Sander Mateos <csander@purestorage.com>]

On Wed, Dec 3, 2025 at 2:52 PM Joanne Koong <joannelkoong@gmail.com> wrote:
>
> On Tue, Dec 2, 2025 at 8:49 PM Caleb Sander Mateos
> <csander@purestorage.com> wrote:
> >
> > On Tue, Dec 2, 2025 at 4:36 PM Joanne Koong <joannelkoong@gmail.com> wrote:
> > >
> > > Add kernel APIs to pin and unpin the buffer table for fixed buffers,
> > > preventing userspace from unregistering or updating the fixed buffers
> > > table while it is pinned by the kernel.
> > >
> > > This has two advantages:
> > > a) Eliminating the overhead of having to fetch and construct an iter for
> > > a fixed buffer per every cmd. Instead, the caller can pin the buffer
> > > table, fetch/construct the iter once, and use that across cmds for
> > > however long it needs to until it is ready to unpin the buffer table.
> > >
> > > b) Allowing a fixed buffer lookup at any index. The buffer table must be
> > > pinned in order to allow this, otherwise we would have to keep track of
> > > all the nodes that have been looked up by the io_kiocb so that we can
> > > properly adjust the refcounts for those nodes. Ensuring that the buffer
> > > table must first be pinned before being able to fetch a buffer at any
> > > index makes things logistically a lot neater.
> >
> > Why is it necessary to pin the entire buffer table rather than
> > specific entries? That's the purpose of the existing io_rsrc_node refs
> > field.
>
> How would this work with userspace buffer unregistration (which works
> at the table level)? If buffer unregistration should still succeed
> then fuse would need a way to be notified that the buffer has been
> unregistered since the buffer belongs to userspace (eg it would be
> wrong if fuse continues using it even though fuse retains a refcount
> on it). If buffer unregistration should fail, then we would need to
> track this pinned state inside the node instead of relying just on the
> refs field, as buffers can be unregistered even if there are in-flight
> refs (eg we would need to differentiate the ref being from a pin vs
> from not a pin), and I think this would make unregistration more
> cumbersome as well (eg we would have to iterate through all the
> entries looking to see if any are pinned before iterating through them
> again to do the actual unregistration).

Not sure I would say buffer unregistration operates on the table as a
whole. Each registered buffer node is unregistered individually and
stores its own reference count. io_put_rsrc_node() will be called on
each buffer node in the table. However, io_put_rsrc_node() just
removes the one reference from the buffer node. If there are other
references on the buffer node (such as an inflight io_uring request
using it), io_free_rsrc_node() won't be called to free the buffer node
until all those references are dropped too. So fuse holding a
reference on the buffer node would allow it to be unregistered, but
prevent it from being freed until fuse dropped its reference.
I'm not sure I understand the problem with fuse continuing to hold
onto a registered buffer node after userspace has unregistered it from
the buffer table. (It looks like the buffer node in question is the
one at FUSE_URING_FIXED_HEADERS_INDEX?) Wouldn't pinning the buffer
table present similar issues? How would userspace get fuse to drop its
pin if it wants to modify the buffer registrations? I would imagine
the code path that calls io_uring_buf_table_unpin() currently could
instead call into io_put_rsrc_node() (maybe by completing an io_uring
request that has imported the registered buffer) to release its
reference on the buffer node. For ublk, userspace can request to stop
a ublk device or the kernel will do so automatically if userspace
drops its file handle (e.g. if the process exits), which will release
any io_uring resources the ublk device is using.

>
> >
> > >
> > > This is a preparatory patch for fuse io-uring's usage of fixed buffers.
> > >
> > > Signed-off-by: Joanne Koong <joannelkoong@gmail.com>
> > > ---
> > >  include/linux/io_uring/buf.h   | 13 +++++++++++
> > >  include/linux/io_uring_types.h |  9 ++++++++
> > >  io_uring/rsrc.c                | 42 ++++++++++++++++++++++++++++++++++
> > >  3 files changed, 64 insertions(+)
> > >
> > > diff --git a/include/linux/io_uring/buf.h b/include/linux/io_uring/buf.h
> > > index 7a1cf197434d..c997c01c24c4 100644
> > > --- a/include/linux/io_uring/buf.h
> > > +++ b/include/linux/io_uring/buf.h
> > > @@ -9,6 +9,9 @@ int io_uring_buf_ring_pin(struct io_ring_ctx *ctx, unsigned buf_group,
> > >                           unsigned issue_flags, struct io_buffer_list **bl);
> > >  int io_uring_buf_ring_unpin(struct io_ring_ctx *ctx, unsigned buf_group,
> > >                             unsigned issue_flags);
> > > +
> > > +int io_uring_buf_table_pin(struct io_ring_ctx *ctx, unsigned issue_flags);
> > > +int io_uring_buf_table_unpin(struct io_ring_ctx *ctx, unsigned issue_flags);
> > >  #else
> > >  static inline int io_uring_buf_ring_pin(struct io_ring_ctx *ctx,
> > >                                         unsigned buf_group,
> > > @@ -23,6 +26,16 @@ static inline int io_uring_buf_ring_unpin(struct io_ring_ctx *ctx,
> > >  {
> > >         return -EOPNOTSUPP;
> > >  }
> > > +static inline int io_uring_buf_table_pin(struct io_ring_ctx *ctx,
> > > +                                        unsigned issue_flags)
> > > +{
> > > +       return -EOPNOTSUPP;
> > > +}
> > > +static inline int io_uring_buf_table_unpin(struct io_ring_ctx *ctx,
> > > +                                          unsigned issue_flags)
> > > +{
> > > +       return -EOPNOTSUPP;
> > > +}
> > >  #endif /* CONFIG_IO_URING */
> > >
> > >  #endif /* _LINUX_IO_URING_BUF_H */
> > > diff --git a/include/linux/io_uring_types.h b/include/linux/io_uring_types.h
> > > index 36fac08db636..e1a75cfe57d9 100644
> > > --- a/include/linux/io_uring_types.h
> > > +++ b/include/linux/io_uring_types.h
> > > @@ -57,8 +57,17 @@ struct io_wq_work {
> > >         int cancel_seq;
> > >  };
> > >
> > > +/*
> > > + * struct io_rsrc_data flag values:
> > > + *
> > > + * IO_RSRC_DATA_PINNED: data is pinned and cannot be unregistered by userspace
> > > + * until it has been unpinned. Currently this is only possible on buffer tables.
> > > + */
> > > +#define IO_RSRC_DATA_PINNED            BIT(0)
> > > +
> > >  struct io_rsrc_data {
> > >         unsigned int                    nr;
> > > +       u8                              flags;
> > >         struct io_rsrc_node             **nodes;
> > >  };
> > >
> > > diff --git a/io_uring/rsrc.c b/io_uring/rsrc.c
> > > index 3765a50329a8..67331cae0a5a 100644
> > > --- a/io_uring/rsrc.c
> > > +++ b/io_uring/rsrc.c
> > > @@ -9,6 +9,7 @@
> > >  #include <linux/hugetlb.h>
> > >  #include <linux/compat.h>
> > >  #include <linux/io_uring.h>
> > > +#include <linux/io_uring/buf.h>
> > >  #include <linux/io_uring/cmd.h>
> > >
> > >  #include <uapi/linux/io_uring.h>
> > > @@ -304,6 +305,8 @@ static int __io_sqe_buffers_update(struct io_ring_ctx *ctx,
> > >                 return -ENXIO;
> > >         if (up->offset + nr_args > ctx->buf_table.nr)
> > >                 return -EINVAL;
> > > +       if (ctx->buf_table.flags & IO_RSRC_DATA_PINNED)
> > > +               return -EBUSY;
> >
> > IORING_REGISTER_CLONE_BUFFERS can also be used to unregister existing
> > buffers, so it may need the check too?
>
> Ah I didn't realize this existed, thanks. imo I think it's okay to
> clone the buffers in a source ring's pinned buffer table to the
> destination ring (where the destination ring's buffer table is
> unpinned) since the clone acquires its own refcounts on the underlying
> nodes and the clone is its own entity. Do you think this makes sense
> or do you think it's better to just not allow this?

I think cloning buffers to unused buffer table slots on another ring
is fine (analogous to registering new buffers in unused slots). But
with IORING_REGISTER_DST_REPLACE set, it can also be used to
unregister whatever existing buffers happen to be registered in those
slots.

Best,
Caleb


>
> >
> > >
> > >         for (done = 0; done < nr_args; done++) {
> > >                 struct io_rsrc_node *node;
> > > @@ -615,6 +618,8 @@ int io_sqe_buffers_unregister(struct io_ring_ctx *ctx)
> > >  {
> > >         if (!ctx->buf_table.nr)
> > >                 return -ENXIO;
> > > +       if (ctx->buf_table.flags & IO_RSRC_DATA_PINNED)
> > > +               return -EBUSY;
> >
> > io_buffer_unregister_bvec() can also be used to unregister ublk
> > zero-copy buffers (also under control of userspace), so it may need
> > the check too? But maybe fuse ensures that it never uses a ublk
> > zero-copy buffer?
>
> fuse doesn't expose a way for userspace to unregister a zero-copy
> buffer, but thanks for considering this possibility.
>
> Thanks,
> Joanne
> >
> > Best,
> > Caleb