Re: [PATCH v1 11/30] io_uring/kbuf: return buffer id in buffer selection

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

On Tue, Dec 2, 2025 at 4:36 PM Joanne Koong <joannelkoong@gmail.com> wrote:
>
> Return the id of the selected buffer in io_buffer_select(). This is
> needed for kernel-managed buffer rings to later recycle the selected
> buffer.
>
> Signed-off-by: Joanne Koong <joannelkoong@gmail.com>
> ---
>  include/linux/io_uring/cmd.h   | 2 +-
>  include/linux/io_uring_types.h | 2 ++
>  io_uring/kbuf.c                | 7 +++++--
>  3 files changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/io_uring/cmd.h b/include/linux/io_uring/cmd.h
> index a4b5eae2e5d1..795b846d1e11 100644
> --- a/include/linux/io_uring/cmd.h
> +++ b/include/linux/io_uring/cmd.h
> @@ -74,7 +74,7 @@ void io_uring_cmd_issue_blocking(struct io_uring_cmd *ioucmd);
>
>  /*
>   * Select a buffer from the provided buffer group for multishot uring_cmd.
> - * Returns the selected buffer address and size.
> + * Returns the selected buffer address, size, and id.
>   */
>  struct io_br_sel io_uring_cmd_buffer_select(struct io_uring_cmd *ioucmd,
>                                             unsigned buf_group, size_t *len,
> diff --git a/include/linux/io_uring_types.h b/include/linux/io_uring_types.h
> index e1a75cfe57d9..dcc95e73f12f 100644
> --- a/include/linux/io_uring_types.h
> +++ b/include/linux/io_uring_types.h
> @@ -109,6 +109,8 @@ struct io_br_sel {
>                 void *kaddr;
>         };
>         ssize_t val;
> +       /* id of the selected buffer */
> +       unsigned buf_id;

Looks like this could be unioned with val? I think val's size can be
reduced to an int since only int values are assigned to it.

>  };
>
>
> diff --git a/io_uring/kbuf.c b/io_uring/kbuf.c
> index 8a94de6e530f..3ecb6494adea 100644
> --- a/io_uring/kbuf.c
> +++ b/io_uring/kbuf.c
> @@ -239,6 +239,7 @@ static struct io_br_sel io_ring_buffer_select(struct io_kiocb *req, size_t *len,
>         req->flags |= REQ_F_BUFFER_RING | REQ_F_BUFFERS_COMMIT;
>         req->buf_index = buf->bid;
>         sel.buf_list = bl;
> +       sel.buf_id = buf->bid;

This is userspace mapped, so probably should be using READ_ONCE() and
reusing the value between req->buf_index and buf->bid? Looks like an
existing bug that the reads of buf->bid and buf->addr aren't using
READ_ONCE().

>         if (bl->flags & IOBL_KERNEL_MANAGED)
>                 sel.kaddr = (void *)buf->addr;
>         else
> @@ -262,10 +263,12 @@ struct io_br_sel io_buffer_select(struct io_kiocb *req, size_t *len,
>
>         bl = io_buffer_get_list(ctx, buf_group);
>         if (likely(bl)) {
> -               if (bl->flags & IOBL_BUF_RING)
> +               if (bl->flags & IOBL_BUF_RING) {
>                         sel = io_ring_buffer_select(req, len, bl, issue_flags);
> -               else
> +               } else {
>                         sel.addr = io_provided_buffer_select(req, len, bl);
> +                       sel.buf_id = req->buf_index;

Could this cover both IOBL_BUF_RING and !IOBL_BUF_RING cases to avoid
the additional logic in io_ring_buffer_select()?

Best,
Caleb

> +               }
>         }
>         io_ring_submit_unlock(req->ctx, issue_flags);
>         return sel;
> --
> 2.47.3
>