Re: [RFC PATCH 1/2] io_uring: cmd: introduce io_uring_cmd_import_fixed_vec

[Pavel Begunkov <asml.silence@gmail.com>]

On 3/11/25 11:40, Sidong Yang wrote:
> io_uring_cmd_import_fixed_vec() could be used for using multiple
> fixed buffer in uring_cmd callback.
> 
> Signed-off-by: Sidong Yang <sidong.yang@furiosa.ai>
> ---
>   include/linux/io_uring/cmd.h | 14 ++++++++++++++
>   io_uring/uring_cmd.c         | 29 +++++++++++++++++++++++++++++
>   2 files changed, 43 insertions(+)
> 
> diff --git a/include/linux/io_uring/cmd.h b/include/linux/io_uring/cmd.h
> index 598cacda4aa3..75cf25c1e730 100644
> --- a/include/linux/io_uring/cmd.h
> +++ b/include/linux/io_uring/cmd.h
> @@ -44,6 +44,13 @@ int io_uring_cmd_import_fixed(u64 ubuf, unsigned long len, int rw,
>   			      struct io_uring_cmd *ioucmd,
>   			      unsigned int issue_flags);
>   
> +int io_uring_cmd_import_fixed_vec(const struct iovec __user *uiovec,
> +				  unsigned long nr_segs, int rw,
> +				  struct iov_iter *iter,
> +				  struct io_uring_cmd *ioucmd,

nit: it's better to be the first arg

> +				  struct iou_vec *iou_vec, bool compat,

Same comment, iou_vec should not be exposed. And why do we
need to pass compat here? Instead of io_is_compat() inside
the helper.

> +				  unsigned int issue_flags);
> +
>   /*
>    * Completes the request, i.e. posts an io_uring CQE and deallocates @ioucmd
>    * and the corresponding io_uring request.
> @@ -76,6 +83,13 @@ io_uring_cmd_import_fixed(u64 ubuf, unsigned long len, int rw,
>   {
>   	return -EOPNOTSUPP;
>   }
> +int io_uring_cmd_import_fixed_vec(int rw, struct iov_iter *iter,
> +				  struct io_uring_cmd *ioucmd,
> +				  struct iou_vec *vec, unsigned nr_iovs,
> +				  unsigned iovec_off, unsigned int issue_flags)
> +{
> +	return -EOPNOTSUPP;
> +}
>   static inline void io_uring_cmd_done(struct io_uring_cmd *cmd, ssize_t ret,
>   		u64 ret2, unsigned issue_flags)
>   {
> diff --git a/io_uring/uring_cmd.c b/io_uring/uring_cmd.c
> index de39b602aa82..58e2932f29e7 100644
> --- a/io_uring/uring_cmd.c
> +++ b/io_uring/uring_cmd.c
> @@ -255,6 +255,35 @@ int io_uring_cmd_import_fixed(u64 ubuf, unsigned long len, int rw,
>   }
>   EXPORT_SYMBOL_GPL(io_uring_cmd_import_fixed);
>   
> +int io_uring_cmd_import_fixed_vec(const struct iovec __user *uiovec,
> +				  unsigned long nr_segs, int rw,
> +				  struct iov_iter *iter,
> +				  struct io_uring_cmd *ioucmd,
> +				  struct iou_vec *iou_vec, bool compat,
> +				  unsigned int issue_flags)
> +{
> +	struct io_kiocb *req = cmd_to_io_kiocb(ioucmd);
> +	struct iovec *iov;
> +	int ret;
> +
> +	iov = iovec_from_user(uiovec, nr_segs, 0, NULL, compat);
> +	if (IS_ERR(iov))
> +		return PTR_ERR(iov);

That's one allocation

> +
> +	ret = io_vec_realloc(iou_vec, nr_segs);

That's a second one

> +	if (ret) {
> +		kfree(iov);
> +		return ret;
> +	}
> +	memcpy(iou_vec->iovec, iov, sizeof(*iov) * nr_segs);
> +	kfree(iov);
> +
> +	ret = io_import_reg_vec(rw, iter, req, iou_vec, iou_vec->nr, 0,

It's slightly out of date, the import side should use io_prep_reg_iovec(),
it abstracts from iovec placement questions.

> +				issue_flags);

And there will likely be a 3rd one. That's pretty likely why
performance is not up to expectations, unlike the rw/net
side which cache it to eventually 0 realloctions.

The first one can be easily removed, but it'll need better
abstractions for cmds not to expose iou_vec. Let me think
what would be a good approach here.

-- 
Pavel Begunkov