Re: [PATCH 1/8] io_uring: introduce struct iou_vec

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

On Mon, Mar 3, 2025 at 7:50 AM Pavel Begunkov <asml.silence@gmail.com> wrote:
>
> I need a convenient way to pass around and work with iovec+size pair,
> put them into a structure and makes use of it in rw.c
>
> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
> ---
>  include/linux/io_uring_types.h |  5 +++++
>  io_uring/rsrc.c                |  9 +++++++++
>  io_uring/rsrc.h                | 17 +++++++++++++++++
>  io_uring/rw.c                  | 17 +++++++----------
>  io_uring/rw.h                  |  6 ++++--
>  5 files changed, 42 insertions(+), 12 deletions(-)
>
> diff --git a/include/linux/io_uring_types.h b/include/linux/io_uring_types.h
> index 35fc241c4672..9101f12d21ef 100644
> --- a/include/linux/io_uring_types.h
> +++ b/include/linux/io_uring_types.h
> @@ -110,6 +110,11 @@ struct io_uring_task {
>         } ____cacheline_aligned_in_smp;
>  };
>
> +struct iou_vec {
> +       struct iovec            *iovec;
> +       unsigned                nr;
> +};
> +
>  struct io_uring {
>         u32 head;
>         u32 tail;
> diff --git a/io_uring/rsrc.c b/io_uring/rsrc.c
> index d6ac41840900..9b05e614819e 100644
> --- a/io_uring/rsrc.c
> +++ b/io_uring/rsrc.c
> @@ -1264,3 +1264,12 @@ int io_register_clone_buffers(struct io_ring_ctx *ctx, void __user *arg)
>         fput(file);
>         return ret;
>  }
> +
> +void io_vec_free(struct iou_vec *iv)
> +{
> +       if (!iv->iovec)
> +               return;
> +       kfree(iv->iovec);
> +       iv->iovec = NULL;
> +       iv->nr = 0;
> +}
> diff --git a/io_uring/rsrc.h b/io_uring/rsrc.h
> index 662244282b2c..e3f1cfb2ff7b 100644
> --- a/io_uring/rsrc.h
> +++ b/io_uring/rsrc.h
> @@ -3,6 +3,7 @@
>  #define IOU_RSRC_H
>
>  #include <linux/lockdep.h>
> +#include <linux/io_uring_types.h>

I sent out a separate patch for this a couple days ago:
https://lore.kernel.org/io-uring/20250301183612.937529-1-csander@purestorage.com/T/#u

>
>  enum {
>         IORING_RSRC_FILE                = 0,
> @@ -144,4 +145,20 @@ static inline void __io_unaccount_mem(struct user_struct *user,
>         atomic_long_sub(nr_pages, &user->locked_vm);
>  }
>
> +void io_vec_free(struct iou_vec *iv);
> +
> +static inline void io_vec_reset_iovec(struct iou_vec *iv,
> +                                     struct iovec *iovec, unsigned nr)
> +{
> +       io_vec_free(iv);
> +       iv->iovec = iovec;
> +       iv->nr = nr;
> +}
> +
> +static inline void io_alloc_cache_vec_kasan(struct iou_vec *iv)
> +{
> +       if (IS_ENABLED(CONFIG_KASAN))
> +               io_vec_free(iv);
> +}
> +
>  #endif
> diff --git a/io_uring/rw.c b/io_uring/rw.c
> index 5ee9f8949e8b..ad7f647d48e9 100644
> --- a/io_uring/rw.c
> +++ b/io_uring/rw.c
> @@ -87,9 +87,9 @@ static int io_import_vec(int ddir, struct io_kiocb *req,
>         int ret, nr_segs;
>         struct iovec *iov;
>
> -       if (io->free_iovec) {
> -               nr_segs = io->free_iov_nr;
> -               iov = io->free_iovec;
> +       if (io->vec.iovec) {
> +               nr_segs = io->vec.nr;
> +               iov = io->vec.iovec;
>         } else {
>                 nr_segs = 1;
>                 iov = &io->fast_iov;
> @@ -101,9 +101,7 @@ static int io_import_vec(int ddir, struct io_kiocb *req,
>                 return ret;
>         if (iov) {
>                 req->flags |= REQ_F_NEED_CLEANUP;
> -               io->free_iov_nr = io->iter.nr_segs;
> -               kfree(io->free_iovec);
> -               io->free_iovec = iov;
> +               io_vec_reset_iovec(&io->vec, iov, io->iter.nr_segs);
>         }
>         return 0;
>  }
> @@ -151,7 +149,7 @@ static void io_rw_recycle(struct io_kiocb *req, unsigned int issue_flags)
>         if (unlikely(issue_flags & IO_URING_F_UNLOCKED))
>                 return;
>
> -       io_alloc_cache_kasan(&rw->free_iovec, &rw->free_iov_nr);
> +       io_alloc_cache_vec_kasan(&rw->vec);
>         if (io_alloc_cache_put(&req->ctx->rw_cache, rw)) {
>                 req->async_data = NULL;
>                 req->flags &= ~REQ_F_ASYNC_DATA;
> @@ -201,7 +199,7 @@ static int io_rw_alloc_async(struct io_kiocb *req)
>         rw = io_uring_alloc_async_data(&ctx->rw_cache, req);
>         if (!rw)
>                 return -ENOMEM;
> -       if (rw->free_iovec)
> +       if (rw->vec.iovec)
>                 req->flags |= REQ_F_NEED_CLEANUP;
>         rw->bytes_done = 0;
>         return 0;
> @@ -1327,7 +1325,6 @@ void io_rw_cache_free(const void *entry)
>  {
>         struct io_async_rw *rw = (struct io_async_rw *) entry;
>
> -       if (rw->free_iovec)
> -               kfree(rw->free_iovec);
> +       io_vec_free(&rw->vec);
>         kfree(rw);
>  }
> diff --git a/io_uring/rw.h b/io_uring/rw.h
> index bf121b81ebe8..e86a3858f48b 100644
> --- a/io_uring/rw.h
> +++ b/io_uring/rw.h
> @@ -3,19 +3,21 @@
>  #include <linux/io_uring_types.h>
>  #include <linux/pagemap.h>
>
> +#include "rsrc.h"

Why is this include necessary? struct iou_vec is defined in
io_uring_types.h. Seems like the include would make more sense in
rw.c.

Best,
Caleb

> +
>  struct io_meta_state {
>         u32                     seed;
>         struct iov_iter_state   iter_meta;
>  };
>
>  struct io_async_rw {
> +       struct iou_vec                  vec;
>         size_t                          bytes_done;
> -       struct iovec                    *free_iovec;
> +
>         struct_group(clear,
>                 struct iov_iter                 iter;
>                 struct iov_iter_state           iter_state;
>                 struct iovec                    fast_iov;
> -               int                             free_iov_nr;
>                 /*
>                  * wpq is for buffered io, while meta fields are used with
>                  * direct io
> --
> 2.48.1
>
>