Re: [PATCH 3/4] io_uring/rw: extract helper for iovec import

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

On Mon, Feb 24, 2025 at 8:07 AM Pavel Begunkov <asml.silence@gmail.com> wrote:
>
> Split out a helper out of __io_import_rw_buffer() that handles vectored
> buffers. I'll need it for registered vectored buffers, but it also looks
> cleaner, especially with parameters being properly named.
>
> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
> ---
>  io_uring/rw.c | 57 ++++++++++++++++++++++++++++-----------------------
>  1 file changed, 31 insertions(+), 26 deletions(-)
>
> diff --git a/io_uring/rw.c b/io_uring/rw.c
> index e636be4850a7..0e0d2a19f21d 100644
> --- a/io_uring/rw.c
> +++ b/io_uring/rw.c
> @@ -76,41 +76,23 @@ static int io_iov_buffer_select_prep(struct io_kiocb *req)
>         return 0;
>  }
>
> -static int __io_import_rw_buffer(int ddir, struct io_kiocb *req,
> -                            struct io_async_rw *io,
> -                            unsigned int issue_flags)
> +static int io_import_vec(int ddir, struct io_kiocb *req,
> +                        struct io_async_rw *io, void __user *uvec,
> +                        size_t uvec_segs)

Could use a more specific type for uvec:  const struct iovec __user *uvec?

>  {
> -       const struct io_issue_def *def = &io_issue_defs[req->opcode];
> -       struct io_rw *rw = io_kiocb_to_cmd(req, struct io_rw);
> +       int ret, nr_segs;
>         struct iovec *iov;
> -       void __user *buf;
> -       int nr_segs, ret;
> -       size_t sqe_len;
> -
> -       buf = u64_to_user_ptr(rw->addr);
> -       sqe_len = rw->len;
> -
> -       if (!def->vectored || req->flags & REQ_F_BUFFER_SELECT) {
> -               if (io_do_buffer_select(req)) {
> -                       buf = io_buffer_select(req, &sqe_len, issue_flags);
> -                       if (!buf)
> -                               return -ENOBUFS;
> -                       rw->addr = (unsigned long) buf;
> -                       rw->len = sqe_len;
> -               }
> -
> -               return import_ubuf(ddir, buf, sqe_len, &io->iter);
> -       }
>
>         if (io->free_iovec) {
>                 nr_segs = io->free_iov_nr;
>                 iov = io->free_iovec;
>         } else {
> -               iov = &io->fast_iov;
>                 nr_segs = 1;
> +               iov = &io->fast_iov;
>         }
> -       ret = __import_iovec(ddir, buf, sqe_len, nr_segs, &iov, &io->iter,
> -                               io_is_compat(req->ctx));
> +
> +       ret = __import_iovec(ddir, uvec, uvec_segs, nr_segs, &iov, &io->iter,
> +                            io_is_compat(req->ctx));
>         if (unlikely(ret < 0))
>                 return ret;
>         if (iov) {
> @@ -122,6 +104,29 @@ static int __io_import_rw_buffer(int ddir, struct io_kiocb *req,
>         return 0;
>  }
>
> +static int __io_import_rw_buffer(int ddir, struct io_kiocb *req,
> +                            struct io_async_rw *io,
> +                            unsigned int issue_flags)
> +{
> +       const struct io_issue_def *def = &io_issue_defs[req->opcode];
> +       struct io_rw *rw = io_kiocb_to_cmd(req, struct io_rw);
> +       void __user *buf = u64_to_user_ptr(rw->addr);
> +       size_t sqe_len = rw->len;
> +
> +       if (!def->vectored || req->flags & REQ_F_BUFFER_SELECT) {
> +               if (io_do_buffer_select(req)) {
> +                       buf = io_buffer_select(req, &sqe_len, issue_flags);
> +                       if (!buf)
> +                               return -ENOBUFS;
> +                       rw->addr = (unsigned long) buf;
> +                       rw->len = sqe_len;
> +               }
> +               return import_ubuf(ddir, buf, sqe_len, &io->iter);
> +       }
> +
> +       return io_import_vec(ddir, req, io, buf, sqe_len);

nit: an early return for the vectored case could reduce indentation here.

Best,
Caleb

> +}
> +
>  static inline int io_import_rw_buffer(int rw, struct io_kiocb *req,
>                                       struct io_async_rw *io,
>                                       unsigned int issue_flags)
> --
> 2.48.1
>
>