Re: [PATCH v2 1/8] io_uring/uring_cmd: add io_uring_cmd_import_fixed_full()

[Joanne Koong <joannelkoong@gmail.com>]

On Fri, Oct 31, 2025 at 3:27 AM Bernd Schubert <bschubert@ddn.com> wrote:
>
> On 10/31/25 00:50, Joanne Koong wrote:
> > On Thu, Oct 30, 2025 at 3:24 PM Bernd Schubert <bschubert@ddn.com> wrote:
> >>
> >> On 10/30/25 19:06, Pavel Begunkov wrote:
> >>> On 10/29/25 18:37, Joanne Koong wrote:
> >>>> On Wed, Oct 29, 2025 at 7:01 AM Pavel Begunkov <asml.silence@gmail.com> wrote:
> >>>>>
> >>>>> On 10/27/25 22:28, Joanne Koong wrote:
> >>>>>> Add an API for fetching the registered buffer associated with a
> >>>>>> io_uring cmd. This is useful for callers who need access to the buffer
> >>>>>> but do not have prior knowledge of the buffer's user address or length.
> >>>>>
> >>>>> Joanne, is it needed because you don't want to pass {offset,size}
> >>>>> via fuse uapi? It's often more convenient to allocate and register
> >>>>> one large buffer and let requests to use subchunks. Shouldn't be
> >>>>> different for performance, but e.g. if you try to overlay it onto
> >>>>> huge pages it'll be severely overaccounted.
> >>>>>
> >>>>
> >>>> Hi Pavel,
> >>>>
> >>>> Yes, I was thinking this would be a simpler interface than the
> >>>> userspace caller having to pass in the uaddr and size on every
> >>>> request. Right now the way it is structured is that userspace
> >>>> allocates a buffer per request, then registers all those buffers. On
> >>>> the kernel side when it fetches the buffer, it'll always fetch the
> >>>> whole buffer (eg offset is 0 and size is the full size).
> >>>>
> >>>> Do you think it is better to allocate one large buffer and have the
> >>>> requests use subchunks?
> >>>
> >>> I think so, but that's general advice, I don't know the fuse
> >>> implementation details, and it's not a strong opinion. It'll be great
> >>> if you take a look at what other server implementations might want and
> >>> do, and if whether this approach is flexible enough, and how amendable
> >>> it is if you change it later on. E.g. how many registered buffers it
> >>> might need? io_uring caps it at some 1000s. How large buffers are?
> >>> Each separate buffer has memory footprint. And because of the same
> >>> footprint there might be cache misses as well if there are too many.
> >>> Can you always predict the max number of buffers to avoid resizing
> >>> the table? Do you ever want to use huge pages while being
> >>> restricted by mlock limits? And so on.
> >>>
> >>> In either case, I don't have a problem with this patch, just
> >>> found it a bit off.
> >>
> >> Maybe we could address that later on, so far I don't like the idea
> >> of a single buffer size for all ring entries. Maybe it would make
> >> sense to introduce buffer pools of different sizes and let ring
> >> entries use a needed buffer size dynamically.
> >>
> >> The part I'm still not too happy about is the need for fuse server
> >> changes - my alternative patch didn't need that at all.
> >>
> >
> > With pinning through io-uring registered buffers, this lets us also
> > automatically use pinned pages for writing it out (eg if we're writing
> > it out to local disk, we can pass that sqe directly to
> > io_uring_prep_rw() and since it's marked as a registered buffer in io
> > uring, it'll skip that pinning/translation overhead).
>
> Ah that is good to know, maybe worth to be mentioned to the commit message.

Will do. I will add this to the commit message.

>
> Btw, I will start to work on libfuse around next week to add another
> io-uring interface, so that the application can own the ring and
> let libfuse submit and fetch from it. I.e. that way the same ring can be
> used for libfuse and application IO.

Sounds great! Looking forward to the changes.

Thanks,
Joanne
>
> Thanks,
> Bernd