Re: [PATCH 2/2] io_uring/nop: use io_find_buf_node()

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

On Fri, Feb 28, 2025 at 5:40 PM Pavel Begunkov <asml.silence@gmail.com> wrote:
>
> On 3/1/25 00:16, Caleb Sander Mateos wrote:
> > Call io_find_buf_node() to avoid duplicating it in io_nop().
>
> IORING_NOP_FIXED_BUFFER interface looks odd, instead of pretending
> to use a buffer, it basically pokes directly into internal infra,
> it's not something userspace should be able to do.

I assumed it was just for benchmarking the overhead of fixed buffer
lookup. Since a normal IORING_OP_NOP doesn't use any buffer, it makes
sense for IORING_NOP_FIXED_BUFFER not to do anything with the fixed
buffer either.

Added in this commit:
commit a85f31052bce52111b4e9d5a536003481d0421d0
Author: Jens Axboe <axboe@kernel.dk>
Date:   Sun Oct 27 08:59:10 2024

    io_uring/nop: add support for testing registered files and buffers

    Useful for testing performance/efficiency impact of registered files
    and buffers, vs (particularly) non-registered files.

    Signed-off-by: Jens Axboe <axboe@kernel.dk>

Best,
Caleb

>
> Jens, did use it anywhere? It's new, I'd rather kill it or align with
> how requests consume buffers, i.e. addr+len, and then do
> io_import_reg_buf() instead. That'd break the api though, but would
> anyone care?
>
>
> > Signed-off-by: Caleb Sander Mateos <csander@purestorage.com>
> > ---
> >   io_uring/nop.c | 13 ++-----------
> >   1 file changed, 2 insertions(+), 11 deletions(-)
> >
> > diff --git a/io_uring/nop.c b/io_uring/nop.c
> > index ea539531cb5f..28f06285fdc2 100644
> > --- a/io_uring/nop.c
> > +++ b/io_uring/nop.c
> > @@ -59,21 +59,12 @@ int io_nop(struct io_kiocb *req, unsigned int issue_flags)
> >                       ret = -EBADF;
> >                       goto done;
> >               }
> >       }
> >       if (nop->flags & IORING_NOP_FIXED_BUFFER) {
> > -             struct io_ring_ctx *ctx = req->ctx;
> > -             struct io_rsrc_node *node;
> > -
> > -             ret = -EFAULT;
> > -             io_ring_submit_lock(ctx, issue_flags);
> > -             node = io_rsrc_node_lookup(&ctx->buf_table, req->buf_index);
> > -             if (node) {
> > -                     io_req_assign_buf_node(req, node);
> > -                     ret = 0;
> > -             }
> > -             io_ring_submit_unlock(ctx, issue_flags);
> > +             if (!io_find_buf_node(req, issue_flags))
> > +                     ret = -EFAULT;
> >       }
> >   done:
> >       if (ret < 0)
> >               req_set_fail(req);
> >       io_req_set_res(req, nop->result, 0);
>
> --
> Pavel Begunkov
>