[PATCH 0/6] provided buffer cleanups

[PATCH 0/6] provided buffer cleanups

[Pavel Begunkov]

Random small improvements for provided buffers [un]registration,
with Patch 6 deduplicating and shrinking legacy pbuf buffer
management opcodes.

Pavel Begunkov (6):
  io_uring/kbuf: account ring io_buffer_list memory
  io_uring/kbuf: use mem_is_zero()
  io_uring/kbuf: drop extra vars in io_register_pbuf_ring
  io_uring/kbuf: don't compute size twice on prep
  io_uring/kbuf: refactor __io_remove_buffers
  io_uring/kbuf: unify legacy buf provision and removal

 io_uring/kbuf.c  | 131 +++++++++++++++++------------------------------
 io_uring/kbuf.h  |   4 +-
 io_uring/opdef.c |   4 +-
 3 files changed, 50 insertions(+), 89 deletions(-)

-- 
2.49.0

[PATCH 1/6] io_uring/kbuf: account ring io_buffer_list memory

[Pavel Begunkov]

Follow the non-ringed pbuf struct io_buffer_list allocations and account
it against the memcg. There is low chance of that being an actual
problem as ring provided buffer should either ping user memory or
allocate it, which is already accounted.

Cc: stable@vger.kernel.org # 6.1
Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 io_uring/kbuf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/io_uring/kbuf.c b/io_uring/kbuf.c
index 1cf0d2c01287..446207db1edf 100644
--- a/io_uring/kbuf.c
+++ b/io_uring/kbuf.c
@@ -621,7 +621,7 @@ int io_register_pbuf_ring(struct io_ring_ctx *ctx, void __user *arg)
 		io_destroy_bl(ctx, bl);
 	}
 
-	free_bl = bl = kzalloc(sizeof(*bl), GFP_KERNEL);
+	free_bl = bl = kzalloc(sizeof(*bl), GFP_KERNEL_ACCOUNT);
 	if (!bl)
 		return -ENOMEM;
 
-- 
2.49.0

Re: [PATCH 1/6] io_uring/kbuf: account ring io_buffer_list memory

[Jens Axboe]

On 5/13/25 11:26 AM, Pavel Begunkov wrote:
> Follow the non-ringed pbuf struct io_buffer_list allocations and account
> it against the memcg. There is low chance of that being an actual
> problem as ring provided buffer should either ping user memory or

*pin

Outside of that, nothing to complain about in this series. I fixed
it up.

-- 
Jens Axboe

[PATCH 2/6] io_uring/kbuf: use mem_is_zero()

[Pavel Begunkov]

Make use of mem_is_zero() for reserved fields checking.

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 io_uring/kbuf.c | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/io_uring/kbuf.c b/io_uring/kbuf.c
index 446207db1edf..344517d1d921 100644
--- a/io_uring/kbuf.c
+++ b/io_uring/kbuf.c
@@ -602,8 +602,7 @@ int io_register_pbuf_ring(struct io_ring_ctx *ctx, void __user *arg)
 
 	if (copy_from_user(&reg, arg, sizeof(reg)))
 		return -EFAULT;
-
-	if (reg.resv[0] || reg.resv[1] || reg.resv[2])
+	if (!mem_is_zero(reg.resv, sizeof(reg.resv)))
 		return -EINVAL;
 	if (reg.flags & ~(IOU_PBUF_RING_MMAP | IOU_PBUF_RING_INC))
 		return -EINVAL;
@@ -679,9 +678,7 @@ int io_unregister_pbuf_ring(struct io_ring_ctx *ctx, void __user *arg)
 
 	if (copy_from_user(&reg, arg, sizeof(reg)))
 		return -EFAULT;
-	if (reg.resv[0] || reg.resv[1] || reg.resv[2])
-		return -EINVAL;
-	if (reg.flags)
+	if (!mem_is_zero(reg.resv, sizeof(reg.resv)) || reg.flags)
 		return -EINVAL;
 
 	bl = io_buffer_get_list(ctx, reg.bgid);
@@ -701,14 +698,11 @@ int io_register_pbuf_status(struct io_ring_ctx *ctx, void __user *arg)
 {
 	struct io_uring_buf_status buf_status;
 	struct io_buffer_list *bl;
-	int i;
 
 	if (copy_from_user(&buf_status, arg, sizeof(buf_status)))
 		return -EFAULT;
-
-	for (i = 0; i < ARRAY_SIZE(buf_status.resv); i++)
-		if (buf_status.resv[i])
-			return -EINVAL;
+	if (!mem_is_zero(buf_status.resv, sizeof(buf_status.resv)))
+		return -EINVAL;
 
 	bl = io_buffer_get_list(ctx, buf_status.buf_group);
 	if (!bl)
-- 
2.49.0

[PATCH 3/6] io_uring/kbuf: drop extra vars in io_register_pbuf_ring

[Pavel Begunkov]

bl and free_bl variables in io_register_pbuf_ring() always point to the
same list since we started to reallocate the pre-existent list. Drop
free_bl.

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 io_uring/kbuf.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/io_uring/kbuf.c b/io_uring/kbuf.c
index 344517d1d921..406e8a9b42c3 100644
--- a/io_uring/kbuf.c
+++ b/io_uring/kbuf.c
@@ -591,7 +591,7 @@ int io_provide_buffers(struct io_kiocb *req, unsigned int issue_flags)
 int io_register_pbuf_ring(struct io_ring_ctx *ctx, void __user *arg)
 {
 	struct io_uring_buf_reg reg;
-	struct io_buffer_list *bl, *free_bl = NULL;
+	struct io_buffer_list *bl;
 	struct io_uring_region_desc rd;
 	struct io_uring_buf_ring *br;
 	unsigned long mmap_offset;
@@ -620,7 +620,7 @@ int io_register_pbuf_ring(struct io_ring_ctx *ctx, void __user *arg)
 		io_destroy_bl(ctx, bl);
 	}
 
-	free_bl = bl = kzalloc(sizeof(*bl), GFP_KERNEL_ACCOUNT);
+	bl = kzalloc(sizeof(*bl), GFP_KERNEL_ACCOUNT);
 	if (!bl)
 		return -ENOMEM;
 
@@ -665,7 +665,7 @@ int io_register_pbuf_ring(struct io_ring_ctx *ctx, void __user *arg)
 	return 0;
 fail:
 	io_free_region(ctx, &bl->region);
-	kfree(free_bl);
+	kfree(bl);
 	return ret;
 }
 
-- 
2.49.0

[PATCH 4/6] io_uring/kbuf: don't compute size twice on prep

[Pavel Begunkov]

The size in prep is calculated by io_provide_buffers_prep(), so remove
the recomputation a few lines after.

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 io_uring/kbuf.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/io_uring/kbuf.c b/io_uring/kbuf.c
index 406e8a9b42c3..eb666c02f488 100644
--- a/io_uring/kbuf.c
+++ b/io_uring/kbuf.c
@@ -509,8 +509,6 @@ int io_provide_buffers_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe
 		return -EOVERFLOW;
 	if (check_add_overflow((unsigned long)p->addr, size, &tmp_check))
 		return -EOVERFLOW;
-
-	size = (unsigned long)p->len * p->nbufs;
 	if (!access_ok(u64_to_user_ptr(p->addr), size))
 		return -EFAULT;
 
-- 
2.49.0

[PATCH 5/6] io_uring/kbuf: refactor __io_remove_buffers

[Pavel Begunkov]

__io_remove_buffers used for two purposes, the first is removing
buffers for non ring based lists, which implies that it can be called
multiple times for the same list. And the second is for destroying
lists, which is not perfectly reentrable for ring based lists.

It's confusing, so just have a helper for the legacy pbuf buffer
removal, make sure it's not called for ring pbuf, and open code all ring
pbuf destruction into io_put_bl().

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 io_uring/kbuf.c | 38 +++++++++++++-------------------------
 1 file changed, 13 insertions(+), 25 deletions(-)

diff --git a/io_uring/kbuf.c b/io_uring/kbuf.c
index eb666c02f488..df8aeb42e910 100644
--- a/io_uring/kbuf.c
+++ b/io_uring/kbuf.c
@@ -376,45 +376,33 @@ unsigned int __io_put_kbufs(struct io_kiocb *req, int len, int nbufs)
 	return ret;
 }
 
-static int __io_remove_buffers(struct io_ring_ctx *ctx,
-			       struct io_buffer_list *bl, unsigned nbufs)
+static int io_remove_buffers_legacy(struct io_ring_ctx *ctx,
+				    struct io_buffer_list *bl,
+				    unsigned long nbufs)
 {
-	unsigned i = 0;
-
-	/* shouldn't happen */
-	if (!nbufs)
-		return 0;
-
-	if (bl->flags & IOBL_BUF_RING) {
-		i = bl->buf_ring->tail - bl->head;
-		io_free_region(ctx, &bl->region);
-		/* make sure it's seen as empty */
-		INIT_LIST_HEAD(&bl->buf_list);
-		bl->flags &= ~IOBL_BUF_RING;
-		return i;
-	}
+	unsigned long i = 0;
+	struct io_buffer *nxt;
 
 	/* protects io_buffers_cache */
 	lockdep_assert_held(&ctx->uring_lock);
+	WARN_ON_ONCE(bl->flags & IOBL_BUF_RING);
 
-	while (!list_empty(&bl->buf_list)) {
-		struct io_buffer *nxt;
-
+	for (i = 0; i < nbufs && !list_empty(&bl->buf_list); i++) {
 		nxt = list_first_entry(&bl->buf_list, struct io_buffer, list);
 		list_del(&nxt->list);
 		kfree(nxt);
-
-		if (++i == nbufs)
-			return i;
 		cond_resched();
 	}
-
 	return i;
 }
 
 static void io_put_bl(struct io_ring_ctx *ctx, struct io_buffer_list *bl)
 {
-	__io_remove_buffers(ctx, bl, -1U);
+	if (bl->flags & IOBL_BUF_RING)
+		io_free_region(ctx, &bl->region);
+	else
+		io_remove_buffers_legacy(ctx, bl, -1U);
+
 	kfree(bl);
 }
 
@@ -477,7 +465,7 @@ int io_remove_buffers(struct io_kiocb *req, unsigned int issue_flags)
 		ret = -EINVAL;
 		/* can't use provide/remove buffers command on mapped buffers */
 		if (!(bl->flags & IOBL_BUF_RING))
-			ret = __io_remove_buffers(ctx, bl, p->nbufs);
+			ret = io_remove_buffers_legacy(ctx, bl, p->nbufs);
 	}
 	io_ring_submit_unlock(ctx, issue_flags);
 	if (ret < 0)
-- 
2.49.0

[PATCH 6/6] io_uring/kbuf: unify legacy buf provision and removal

[Pavel Begunkov]

Combine IORING_OP_PROVIDE_BUFFERS and IORING_OP_REMOVE_BUFFERS
->issue(), so that we can deduplicate ring locking and list lookups.
This way we further reduce code for legacy provided buffers. Locking is
also separated from buffer related handling, which makes it a bit
simpler with label jumps.

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 io_uring/kbuf.c  | 73 +++++++++++++++++++-----------------------------
 io_uring/kbuf.h  |  4 +--
 io_uring/opdef.c |  4 +--
 3 files changed, 31 insertions(+), 50 deletions(-)

diff --git a/io_uring/kbuf.c b/io_uring/kbuf.c
index df8aeb42e910..823e7eb15fb2 100644
--- a/io_uring/kbuf.c
+++ b/io_uring/kbuf.c
@@ -450,30 +450,6 @@ int io_remove_buffers_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 	return 0;
 }
 
-int io_remove_buffers(struct io_kiocb *req, unsigned int issue_flags)
-{
-	struct io_provide_buf *p = io_kiocb_to_cmd(req, struct io_provide_buf);
-	struct io_ring_ctx *ctx = req->ctx;
-	struct io_buffer_list *bl;
-	int ret = 0;
-
-	io_ring_submit_lock(ctx, issue_flags);
-
-	ret = -ENOENT;
-	bl = io_buffer_get_list(ctx, p->bgid);
-	if (bl) {
-		ret = -EINVAL;
-		/* can't use provide/remove buffers command on mapped buffers */
-		if (!(bl->flags & IOBL_BUF_RING))
-			ret = io_remove_buffers_legacy(ctx, bl, p->nbufs);
-	}
-	io_ring_submit_unlock(ctx, issue_flags);
-	if (ret < 0)
-		req_set_fail(req);
-	io_req_set_res(req, ret, 0);
-	return IOU_OK;
-}
-
 int io_provide_buffers_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 {
 	unsigned long size, tmp_check;
@@ -535,37 +511,44 @@ static int io_add_buffers(struct io_ring_ctx *ctx, struct io_provide_buf *pbuf,
 	return i ? 0 : -ENOMEM;
 }
 
-int io_provide_buffers(struct io_kiocb *req, unsigned int issue_flags)
+static int __io_manage_buffers_legacy(struct io_kiocb *req,
+					struct io_buffer_list *bl)
 {
 	struct io_provide_buf *p = io_kiocb_to_cmd(req, struct io_provide_buf);
-	struct io_ring_ctx *ctx = req->ctx;
-	struct io_buffer_list *bl;
-	int ret = 0;
-
-	io_ring_submit_lock(ctx, issue_flags);
+	int ret;
 
-	bl = io_buffer_get_list(ctx, p->bgid);
-	if (unlikely(!bl)) {
+	if (!bl) {
+		if (req->opcode != IORING_OP_PROVIDE_BUFFERS)
+			return -ENOENT;
 		bl = kzalloc(sizeof(*bl), GFP_KERNEL_ACCOUNT);
-		if (!bl) {
-			ret = -ENOMEM;
-			goto err;
-		}
+		if (!bl)
+			return -ENOMEM;
+
 		INIT_LIST_HEAD(&bl->buf_list);
-		ret = io_buffer_add_list(ctx, bl, p->bgid);
+		ret = io_buffer_add_list(req->ctx, bl, p->bgid);
 		if (ret) {
 			kfree(bl);
-			goto err;
+			return ret;
 		}
 	}
-	/* can't add buffers via this command for a mapped buffer ring */
-	if (bl->flags & IOBL_BUF_RING) {
-		ret = -EINVAL;
-		goto err;
-	}
+	/* can't use provide/remove buffers command on mapped buffers */
+	if (bl->flags & IOBL_BUF_RING)
+		return -EINVAL;
+	if (req->opcode == IORING_OP_PROVIDE_BUFFERS)
+		return io_add_buffers(req->ctx, p, bl);
+	return io_remove_buffers_legacy(req->ctx, bl, p->nbufs);
+}
+
+int io_manage_buffers_legacy(struct io_kiocb *req, unsigned int issue_flags)
+{
+	struct io_provide_buf *p = io_kiocb_to_cmd(req, struct io_provide_buf);
+	struct io_ring_ctx *ctx = req->ctx;
+	struct io_buffer_list *bl;
+	int ret;
 
-	ret = io_add_buffers(ctx, p, bl);
-err:
+	io_ring_submit_lock(ctx, issue_flags);
+	bl = io_buffer_get_list(ctx, p->bgid);
+	ret = __io_manage_buffers_legacy(req, bl);
 	io_ring_submit_unlock(ctx, issue_flags);
 
 	if (ret < 0)
diff --git a/io_uring/kbuf.h b/io_uring/kbuf.h
index 0798a732e6cb..4d2c209d1a41 100644
--- a/io_uring/kbuf.h
+++ b/io_uring/kbuf.h
@@ -66,10 +66,8 @@ int io_buffers_peek(struct io_kiocb *req, struct buf_sel_arg *arg);
 void io_destroy_buffers(struct io_ring_ctx *ctx);
 
 int io_remove_buffers_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe);
-int io_remove_buffers(struct io_kiocb *req, unsigned int issue_flags);
-
 int io_provide_buffers_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe);
-int io_provide_buffers(struct io_kiocb *req, unsigned int issue_flags);
+int io_manage_buffers_legacy(struct io_kiocb *req, unsigned int issue_flags);
 
 int io_register_pbuf_ring(struct io_ring_ctx *ctx, void __user *arg);
 int io_unregister_pbuf_ring(struct io_ring_ctx *ctx, void __user *arg);
diff --git a/io_uring/opdef.c b/io_uring/opdef.c
index db36433c2294..6e0882b051f9 100644
--- a/io_uring/opdef.c
+++ b/io_uring/opdef.c
@@ -333,13 +333,13 @@ const struct io_issue_def io_issue_defs[] = {
 		.audit_skip		= 1,
 		.iopoll			= 1,
 		.prep			= io_provide_buffers_prep,
-		.issue			= io_provide_buffers,
+		.issue			= io_manage_buffers_legacy,
 	},
 	[IORING_OP_REMOVE_BUFFERS] = {
 		.audit_skip		= 1,
 		.iopoll			= 1,
 		.prep			= io_remove_buffers_prep,
-		.issue			= io_remove_buffers,
+		.issue			= io_manage_buffers_legacy,
 	},
 	[IORING_OP_TEE] = {
 		.needs_file		= 1,
-- 
2.49.0

Re: [PATCH 0/6] provided buffer cleanups

[Jens Axboe]

On Tue, 13 May 2025 18:26:45 +0100, Pavel Begunkov wrote:
> Random small improvements for provided buffers [un]registration,
> with Patch 6 deduplicating and shrinking legacy pbuf buffer
> management opcodes.
> 
> Pavel Begunkov (6):
>   io_uring/kbuf: account ring io_buffer_list memory
>   io_uring/kbuf: use mem_is_zero()
>   io_uring/kbuf: drop extra vars in io_register_pbuf_ring
>   io_uring/kbuf: don't compute size twice on prep
>   io_uring/kbuf: refactor __io_remove_buffers
>   io_uring/kbuf: unify legacy buf provision and removal
> 
> [...]

Applied, thanks!

[1/6] io_uring/kbuf: account ring io_buffer_list memory
      commit: 475a8d30371604a6363da8e304a608a5959afc40
[2/6] io_uring/kbuf: use mem_is_zero()
      commit: 1724849072854a66861d461b298b04612702d685
[3/6] io_uring/kbuf: drop extra vars in io_register_pbuf_ring
      commit: 4e9fda29d66b06caf5c81b8acbe0a504effc73fb
[4/6] io_uring/kbuf: don't compute size twice on prep
      commit: 52a05d0cf8f3b4569c525153132a90661c32fe11
[5/6] io_uring/kbuf: refactor __io_remove_buffers
      commit: c724e801239ffc3714afe65cf6e721ddd04199d0
[6/6] io_uring/kbuf: unify legacy buf provision and removal
      commit: 2b61bb1d9aa601ec393054a61be0a707a5bea928

Best regards,
-- 
Jens Axboe