[PATCH 0/2] fixes for removing provided buffers

[PATCH 0/2] fixes for removing provided buffers

[Wojciech Lukowicz]

Hi,

Two fixes for removing provided buffers. They are in the same area, but
otherwise unrelated. I'll send a liburing test for the first one
shortly.

Thanks,
Wojciech

Wojciech Lukowicz (2):
  io_uring: fix return value when removing provided buffers
  io_uring: fix memory leak when removing provided buffers

 io_uring/io_uring.c | 2 +-
 io_uring/kbuf.c     | 7 ++++---
 2 files changed, 5 insertions(+), 4 deletions(-)

-- 
2.30.2

[PATCH 1/2] io_uring: fix return value when removing provided buffers

[Wojciech Lukowicz]

When a request to remove buffers is submitted, and the given number to be
removed is larger than available in the specified buffer group, the
resulting CQE result will be the number of removed buffers + 1, which is
1 more than it should be.

Previously, the head was part of the list and it got removed after the
loop, so the increment was needed. Now, the head is not an element of
the list, so the increment shouldn't be there anymore.

Fixes: dbc7d452e7cf ("io_uring: manage provided buffers strictly ordered")
Signed-off-by: Wojciech Lukowicz <wlukowicz01@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 3002dc827195..0fdcc0adbdbc 100644
--- a/io_uring/kbuf.c
+++ b/io_uring/kbuf.c
@@ -228,7 +228,6 @@ static int __io_remove_buffers(struct io_ring_ctx *ctx,
 		return i;
 	}
 
-	/* the head kbuf is the list itself */
 	while (!list_empty(&bl->buf_list)) {
 		struct io_buffer *nxt;
 
@@ -238,7 +237,6 @@ static int __io_remove_buffers(struct io_ring_ctx *ctx,
 			return i;
 		cond_resched();
 	}
-	i++;
 
 	return i;
 }
-- 
2.30.2

[PATCH 2/2] io_uring: fix memory leak when removing provided buffers

[Wojciech Lukowicz]

When removing provided buffers, io_buffer structs are not being disposed
of, leading to a memory leak. They can't be freed individually, because
they are allocated in page-sized groups. They need to be added to some
free list instead, such as io_buffers_cache. All callers already hold
the lock protecting it, apart from when destroying buffers, so had to
extend the lock there.

Fixes: cc3cec8367cb ("io_uring: speedup provided buffer handling")
Signed-off-by: Wojciech Lukowicz <wlukowicz01@gmail.com>
---
 io_uring/io_uring.c | 2 +-
 io_uring/kbuf.c     | 5 ++++-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
index 722624b6d0dc..2a8b8c304d2a 100644
--- a/io_uring/io_uring.c
+++ b/io_uring/io_uring.c
@@ -2789,8 +2789,8 @@ static __cold void io_ring_ctx_free(struct io_ring_ctx *ctx)
 	io_eventfd_unregister(ctx);
 	io_alloc_cache_free(&ctx->apoll_cache, io_apoll_cache_free);
 	io_alloc_cache_free(&ctx->netmsg_cache, io_netmsg_cache_free);
-	mutex_unlock(&ctx->uring_lock);
 	io_destroy_buffers(ctx);
+	mutex_unlock(&ctx->uring_lock);
 	if (ctx->sq_creds)
 		put_cred(ctx->sq_creds);
 	if (ctx->submitter_task)
diff --git a/io_uring/kbuf.c b/io_uring/kbuf.c
index 0fdcc0adbdbc..a90c820ce99e 100644
--- a/io_uring/kbuf.c
+++ b/io_uring/kbuf.c
@@ -228,11 +228,14 @@ static int __io_remove_buffers(struct io_ring_ctx *ctx,
 		return i;
 	}
 
+	/* protects io_buffers_cache */
+	lockdep_assert_held(&ctx->uring_lock);
+
 	while (!list_empty(&bl->buf_list)) {
 		struct io_buffer *nxt;
 
 		nxt = list_first_entry(&bl->buf_list, struct io_buffer, list);
-		list_del(&nxt->list);
+		list_move(&nxt->list, &ctx->io_buffers_cache);
 		if (++i == nbufs)
 			return i;
 		cond_resched();
-- 
2.30.2

Re: [PATCH 0/2] fixes for removing provided buffers

[Jens Axboe]

On 4/1/23 1:50 PM, Wojciech Lukowicz wrote:
> Hi,
> 
> Two fixes for removing provided buffers. They are in the same area, but
> otherwise unrelated. I'll send a liburing test for the first one
> shortly.

Looks good, thanks for sending these in!

-- 
Jens Axboe

Re: [PATCH 0/2] fixes for removing provided buffers

[Jens Axboe]

On Sat, 01 Apr 2023 20:50:37 +0100, Wojciech Lukowicz wrote:
> Two fixes for removing provided buffers. They are in the same area, but
> otherwise unrelated. I'll send a liburing test for the first one
> shortly.
> 
> Thanks,
> Wojciech
> 
> [...]

Applied, thanks!

[1/2] io_uring: fix return value when removing provided buffers
      commit: b4a72c0589fdea6259720375426179888969d6a2
[2/2] io_uring: fix memory leak when removing provided buffers
      commit: b4a72c0589fdea6259720375426179888969d6a2

Best regards,
-- 
Jens Axboe