[RFC PATCH 0/2] introduce io_uring_cmd_import_fixed_vec

[RFC PATCH 0/2] introduce io_uring_cmd_import_fixed_vec

[Sidong Yang]

This patche series introduce io_uring_cmd_import_vec. With this function,
Multiple fixed buffer could be used in uring cmd. It's vectored version
for io_uring_cmd_import_fixed(). Also this patch series includes a usage
for new api for encoded read in btrfs by using uring cmd.

Sidong Yang (2):
  io_uring: cmd: introduce io_uring_cmd_import_fixed_vec
  btrfs: ioctl: use registered buffer for IORING_URING_CMD_FIXED

 fs/btrfs/ioctl.c             | 26 +++++++++++++++++++++-----
 include/linux/io_uring/cmd.h | 14 ++++++++++++++
 io_uring/uring_cmd.c         | 29 +++++++++++++++++++++++++++++
 3 files changed, 64 insertions(+), 5 deletions(-)

---

Recently, I've found that io_import_reg_vec() was added for io-uring. I think
it could be used for io-uring cmd. I've tested for btrfs encoded read and it
works. But it seems that there is no performance improvements and I'll keep
find why.

If there is no need to use fixed buffer for btrfs, I think it's good to use
for nvme.

2.43.0

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

[Sidong Yang]

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,
+				  struct iou_vec *iou_vec, bool compat,
+				  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);
+
+	ret = io_vec_realloc(iou_vec, nr_segs);
+	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,
+				issue_flags);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(io_uring_cmd_import_fixed_vec);
+
 void io_uring_cmd_issue_blocking(struct io_uring_cmd *ioucmd)
 {
 	struct io_kiocb *req = cmd_to_io_kiocb(ioucmd);
-- 
2.43.0

[RFC PATCH 2/2] btrfs: ioctl: use registered buffer for IORING_URING_CMD_FIXED

[Sidong Yang]

This patch supports IORING_URING_CMD_FIXED flags in io-uring cmd. It
means that user provided buf_index in sqe that is registered before
submitting requests. In this patch, btrfs_uring_encoded_read() makes
iov_iter bvec type by checking the io-uring cmd flag. And there is
additional iou_vec field in btrfs_uring_priv for remaining bvecs
lifecycle.

Signed-off-by: Sidong Yang <sidong.yang@furiosa.ai>
---
 fs/btrfs/ioctl.c | 26 +++++++++++++++++++++-----
 1 file changed, 21 insertions(+), 5 deletions(-)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 6c18bad53cd3..586671eea622 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -4643,6 +4643,7 @@ struct btrfs_uring_priv {
 	struct page **pages;
 	unsigned long nr_pages;
 	struct kiocb iocb;
+	struct iou_vec iou_vec;
 	struct iovec *iov;
 	struct iov_iter iter;
 	struct extent_state *cached_state;
@@ -4711,6 +4712,8 @@ static void btrfs_uring_read_finished(struct io_uring_cmd *cmd, unsigned int iss
 
 	kfree(priv->pages);
 	kfree(priv->iov);
+	if (priv->iou_vec.iovec)
+		kfree(priv->iou_vec.iovec);
 	kfree(priv);
 }
 
@@ -4730,7 +4733,8 @@ static int btrfs_uring_read_extent(struct kiocb *iocb, struct iov_iter *iter,
 				   struct extent_state *cached_state,
 				   u64 disk_bytenr, u64 disk_io_size,
 				   size_t count, bool compressed,
-				   struct iovec *iov, struct io_uring_cmd *cmd)
+				   struct iovec *iov, struct io_uring_cmd *cmd,
+				   struct iou_vec *iou_vec)
 {
 	struct btrfs_inode *inode = BTRFS_I(file_inode(iocb->ki_filp));
 	struct extent_io_tree *io_tree = &inode->io_tree;
@@ -4767,6 +4771,7 @@ static int btrfs_uring_read_extent(struct kiocb *iocb, struct iov_iter *iter,
 	priv->start = start;
 	priv->lockend = lockend;
 	priv->err = 0;
+	priv->iou_vec = *iou_vec;
 
 	ret = btrfs_encoded_read_regular_fill_pages(inode, disk_bytenr,
 						    disk_io_size, pages, priv);
@@ -4818,6 +4823,7 @@ static int btrfs_uring_encoded_read(struct io_uring_cmd *cmd, unsigned int issue
 	u64 start, lockend;
 	void __user *sqe_addr;
 	struct btrfs_uring_encoded_data *data = io_uring_cmd_get_async_data(cmd)->op_data;
+	struct iou_vec iou_vec = {};
 
 	if (!capable(CAP_SYS_ADMIN)) {
 		ret = -EPERM;
@@ -4875,9 +4881,19 @@ static int btrfs_uring_encoded_read(struct io_uring_cmd *cmd, unsigned int issue
 		}
 
 		data->iov = data->iovstack;
-		ret = import_iovec(ITER_DEST, data->args.iov, data->args.iovcnt,
-				   ARRAY_SIZE(data->iovstack), &data->iov,
-				   &data->iter);
+
+		if (cmd && (cmd->flags & IORING_URING_CMD_FIXED)) {
+			ret = io_uring_cmd_import_fixed_vec(
+				data->args.iov, data->args.iovcnt, ITER_DEST,
+				&data->iter, cmd, &iou_vec, false, issue_flags);
+			data->iov = NULL;
+		} else {
+			ret = import_iovec(ITER_DEST, data->args.iov,
+					   data->args.iovcnt,
+					   ARRAY_SIZE(data->iovstack),
+					   &data->iov, &data->iter);
+		}
+
 		if (ret < 0)
 			goto out_acct;
 
@@ -4929,7 +4945,7 @@ static int btrfs_uring_encoded_read(struct io_uring_cmd *cmd, unsigned int issue
 		ret = btrfs_uring_read_extent(&kiocb, &data->iter, start, lockend,
 					      cached_state, disk_bytenr, disk_io_size,
 					      count, data->args.compression,
-					      data->iov, cmd);
+					      data->iov, cmd, &iou_vec);
 
 		goto out_acct;
 	}
-- 
2.43.0

Re: [RFC PATCH 2/2] btrfs: ioctl: use registered buffer for IORING_URING_CMD_FIXED

[Pavel Begunkov]

On 3/11/25 11:40, Sidong Yang wrote:
> This patch supports IORING_URING_CMD_FIXED flags in io-uring cmd. It
> means that user provided buf_index in sqe that is registered before
> submitting requests. In this patch, btrfs_uring_encoded_read() makes
> iov_iter bvec type by checking the io-uring cmd flag. And there is
> additional iou_vec field in btrfs_uring_priv for remaining bvecs
> lifecycle.
> 
> Signed-off-by: Sidong Yang <sidong.yang@furiosa.ai>
> ---
>   fs/btrfs/ioctl.c | 26 +++++++++++++++++++++-----
>   1 file changed, 21 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> index 6c18bad53cd3..586671eea622 100644
> --- a/fs/btrfs/ioctl.c
> +++ b/fs/btrfs/ioctl.c
> @@ -4643,6 +4643,7 @@ struct btrfs_uring_priv {
>   	struct page **pages;
>   	unsigned long nr_pages;
>   	struct kiocb iocb;
> +	struct iou_vec iou_vec;

This structure should not be leaked out of core io_uring ...

>   	struct iovec *iov;
>   	struct iov_iter iter;
>   	struct extent_state *cached_state;
> @@ -4711,6 +4712,8 @@ static void btrfs_uring_read_finished(struct io_uring_cmd *cmd, unsigned int iss
>   
>   	kfree(priv->pages);
>   	kfree(priv->iov);
> +	if (priv->iou_vec.iovec)
> +		kfree(priv->iou_vec.iovec);

... exactly because if this. This line relies on details it
shouldn't.

-- 
Pavel Begunkov

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

[Pavel Begunkov]

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

Re: [RFC PATCH 2/2] btrfs: ioctl: use registered buffer for IORING_URING_CMD_FIXED

[Sidong Yang]

On Tue, Mar 11, 2025 at 12:55:48PM +0000, Pavel Begunkov wrote:

Hi Pavel,

> On 3/11/25 11:40, Sidong Yang wrote:
> > This patch supports IORING_URING_CMD_FIXED flags in io-uring cmd. It
> > means that user provided buf_index in sqe that is registered before
> > submitting requests. In this patch, btrfs_uring_encoded_read() makes
> > iov_iter bvec type by checking the io-uring cmd flag. And there is
> > additional iou_vec field in btrfs_uring_priv for remaining bvecs
> > lifecycle.
> > 
> > Signed-off-by: Sidong Yang <sidong.yang@furiosa.ai>
> > ---
> >   fs/btrfs/ioctl.c | 26 +++++++++++++++++++++-----
> >   1 file changed, 21 insertions(+), 5 deletions(-)
> > 
> > diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> > index 6c18bad53cd3..586671eea622 100644
> > --- a/fs/btrfs/ioctl.c
> > +++ b/fs/btrfs/ioctl.c
> > @@ -4643,6 +4643,7 @@ struct btrfs_uring_priv {
> >   	struct page **pages;
> >   	unsigned long nr_pages;
> >   	struct kiocb iocb;
> > +	struct iou_vec iou_vec;
> 
> This structure should not be leaked out of core io_uring ...

Agreed, but this was needed that priv needs to have bvec than iovec.
Thinking about this, just adding bvec or make union with iov is
simple way to do this.

> 
> >   	struct iovec *iov;
> >   	struct iov_iter iter;
> >   	struct extent_state *cached_state;
> > @@ -4711,6 +4712,8 @@ static void btrfs_uring_read_finished(struct io_uring_cmd *cmd, unsigned int iss
> >   	kfree(priv->pages);
> >   	kfree(priv->iov);
> > +	if (priv->iou_vec.iovec)
> > +		kfree(priv->iou_vec.iovec);
> 
> ... exactly because if this. This line relies on details it
> shouldn't.

Yes, we don't need this.

Thanks,
Sidong

> 
> -- 
> Pavel Begunkov
> 

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

[Sidong Yang]

On Tue, Mar 11, 2025 at 01:08:14PM +0000, Pavel Begunkov wrote:
> 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

Thanks for tip.
> 
> > +				  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.

Actually I don't know about io_is_compat(). Thanks.

> 
> > +				  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.

Totally agreed, There is too many allocation for this. It should be done
allocation.

Thanks,
Sidong
> 
> -- 
> Pavel Begunkov
>