[PATCH 0/8] alloc cache and iovec assorted cleanups

[PATCH 0/8] alloc cache and iovec assorted cleanups

[Pavel Begunkov]

A bunch of patches cleaning allocation caches and various bits
related to io vectors.

Pavel Begunkov (8):
  io_uring: include all deps for alloc_cache.h
  io_uring: dont ifdef io_alloc_cache_kasan()
  io_uring: add alloc_cache.c
  io_uring/net: make io_net_vec_assign() return void
  io_uring/net: clean io_msg_copy_hdr()
  io_uring/net: extract io_send_select_buffer()
  io_uring: remove !KASAN guards from cache free
  io_uring/rw: simplify io_rw_recycle()

 io_uring/Makefile      |   2 +-
 io_uring/alloc_cache.c |  44 +++++++++++++++++
 io_uring/alloc_cache.h |  60 +++++++----------------
 io_uring/net.c         | 105 +++++++++++++++++++++++------------------
 io_uring/rw.c          |  18 ++-----
 5 files changed, 123 insertions(+), 106 deletions(-)
 create mode 100644 io_uring/alloc_cache.c

-- 
2.47.1

[PATCH 1/8] io_uring: include all deps for alloc_cache.h

[Pavel Begunkov]

alloc_cache.h uses types it doesn't declare and thus depends on the
order in which it's included. Make it self contained and pull all needed
definitions.

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 io_uring/alloc_cache.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/io_uring/alloc_cache.h b/io_uring/alloc_cache.h
index cca96aff3277e..28436f413bd2c 100644
--- a/io_uring/alloc_cache.h
+++ b/io_uring/alloc_cache.h
@@ -1,6 +1,8 @@
 #ifndef IOU_ALLOC_CACHE_H
 #define IOU_ALLOC_CACHE_H
 
+#include <linux/io_uring_types.h>
+
 /*
  * Don't allow the cache to grow beyond this size.
  */
-- 
2.47.1

[PATCH 2/8] io_uring: dont ifdef io_alloc_cache_kasan()

[Pavel Begunkov]

Use IS_ENABLED in io_alloc_cache_kasan() so at least it gets compile
tested without KASAN.

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 io_uring/alloc_cache.h | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/io_uring/alloc_cache.h b/io_uring/alloc_cache.h
index 28436f413bd2c..9eb374ad7490c 100644
--- a/io_uring/alloc_cache.h
+++ b/io_uring/alloc_cache.h
@@ -8,18 +8,14 @@
  */
 #define IO_ALLOC_CACHE_MAX	128
 
-#if defined(CONFIG_KASAN)
-static inline void io_alloc_cache_kasan(struct iovec **iov, int *nr)
-{
-	kfree(*iov);
-	*iov = NULL;
-	*nr = 0;
-}
-#else
 static inline void io_alloc_cache_kasan(struct iovec **iov, int *nr)
 {
+	if (IS_ENABLED(CONFIG_KASAN)) {
+		kfree(*iov);
+		*iov = NULL;
+		*nr = 0;
+	}
 }
-#endif
 
 static inline bool io_alloc_cache_put(struct io_alloc_cache *cache,
 				      void *entry)
-- 
2.47.1

[PATCH 3/8] io_uring: add alloc_cache.c

[Pavel Begunkov]

Avoid inlining all and everything from alloc_cache.h and move cold bits
into a new file.

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 io_uring/Makefile      |  2 +-
 io_uring/alloc_cache.c | 44 ++++++++++++++++++++++++++++++++++++++++++
 io_uring/alloc_cache.h | 44 +++++++++---------------------------------
 3 files changed, 54 insertions(+), 36 deletions(-)
 create mode 100644 io_uring/alloc_cache.c

diff --git a/io_uring/Makefile b/io_uring/Makefile
index 53167bef37d77..d695b60dba4f0 100644
--- a/io_uring/Makefile
+++ b/io_uring/Makefile
@@ -13,7 +13,7 @@ obj-$(CONFIG_IO_URING)		+= io_uring.o opdef.o kbuf.o rsrc.o notif.o \
 					sync.o msg_ring.o advise.o openclose.o \
 					epoll.o statx.o timeout.o fdinfo.o \
 					cancel.o waitid.o register.o \
-					truncate.o memmap.o
+					truncate.o memmap.o alloc_cache.o
 obj-$(CONFIG_IO_WQ)		+= io-wq.o
 obj-$(CONFIG_FUTEX)		+= futex.o
 obj-$(CONFIG_NET_RX_BUSY_POLL) += napi.o
diff --git a/io_uring/alloc_cache.c b/io_uring/alloc_cache.c
new file mode 100644
index 0000000000000..58423888b736e
--- /dev/null
+++ b/io_uring/alloc_cache.c
@@ -0,0 +1,44 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include "alloc_cache.h"
+
+void io_alloc_cache_free(struct io_alloc_cache *cache,
+			 void (*free)(const void *))
+{
+	void *entry;
+
+	if (!cache->entries)
+		return;
+
+	while ((entry = io_alloc_cache_get(cache)) != NULL)
+		free(entry);
+
+	kvfree(cache->entries);
+	cache->entries = NULL;
+}
+
+/* returns false if the cache was initialized properly */
+bool io_alloc_cache_init(struct io_alloc_cache *cache,
+			 unsigned max_nr, unsigned int size,
+			 unsigned int init_bytes)
+{
+	cache->entries = kvmalloc_array(max_nr, sizeof(void *), GFP_KERNEL);
+	if (!cache->entries)
+		return true;
+
+	cache->nr_cached = 0;
+	cache->max_cached = max_nr;
+	cache->elem_size = size;
+	cache->init_clear = init_bytes;
+	return false;
+}
+
+void *io_cache_alloc_new(struct io_alloc_cache *cache, gfp_t gfp)
+{
+	void *obj;
+
+	obj = kmalloc(cache->elem_size, gfp);
+	if (obj && cache->init_clear)
+		memset(obj, 0, cache->init_clear);
+	return obj;
+}
diff --git a/io_uring/alloc_cache.h b/io_uring/alloc_cache.h
index 9eb374ad7490c..0dd17d8ba93a8 100644
--- a/io_uring/alloc_cache.h
+++ b/io_uring/alloc_cache.h
@@ -8,6 +8,14 @@
  */
 #define IO_ALLOC_CACHE_MAX	128
 
+void io_alloc_cache_free(struct io_alloc_cache *cache,
+			 void (*free)(const void *));
+bool io_alloc_cache_init(struct io_alloc_cache *cache,
+			 unsigned max_nr, unsigned int size,
+			 unsigned int init_bytes);
+
+void *io_cache_alloc_new(struct io_alloc_cache *cache, gfp_t gfp);
+
 static inline void io_alloc_cache_kasan(struct iovec **iov, int *nr)
 {
 	if (IS_ENABLED(CONFIG_KASAN)) {
@@ -57,41 +65,7 @@ static inline void *io_cache_alloc(struct io_alloc_cache *cache, gfp_t gfp)
 	obj = io_alloc_cache_get(cache);
 	if (obj)
 		return obj;
-
-	obj = kmalloc(cache->elem_size, gfp);
-	if (obj && cache->init_clear)
-		memset(obj, 0, cache->init_clear);
-	return obj;
-}
-
-/* returns false if the cache was initialized properly */
-static inline bool io_alloc_cache_init(struct io_alloc_cache *cache,
-				       unsigned max_nr, unsigned int size,
-				       unsigned int init_bytes)
-{
-	cache->entries = kvmalloc_array(max_nr, sizeof(void *), GFP_KERNEL);
-	if (cache->entries) {
-		cache->nr_cached = 0;
-		cache->max_cached = max_nr;
-		cache->elem_size = size;
-		cache->init_clear = init_bytes;
-		return false;
-	}
-	return true;
+	return io_cache_alloc_new(cache, gfp);
 }
 
-static inline void io_alloc_cache_free(struct io_alloc_cache *cache,
-				       void (*free)(const void *))
-{
-	void *entry;
-
-	if (!cache->entries)
-		return;
-
-	while ((entry = io_alloc_cache_get(cache)) != NULL)
-		free(entry);
-
-	kvfree(cache->entries);
-	cache->entries = NULL;
-}
 #endif
-- 
2.47.1

[PATCH 4/8] io_uring/net: make io_net_vec_assign() return void

[Pavel Begunkov]

io_net_vec_assign() can only return 0 and it doesn't make sense for it
to fail, so make it return void.

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

diff --git a/io_uring/net.c b/io_uring/net.c
index 41eef286f8b9a..e72205802055f 100644
--- a/io_uring/net.c
+++ b/io_uring/net.c
@@ -168,7 +168,7 @@ static struct io_async_msghdr *io_msg_alloc_async(struct io_kiocb *req)
 }
 
 /* assign new iovec to kmsg, if we need to */
-static int io_net_vec_assign(struct io_kiocb *req, struct io_async_msghdr *kmsg,
+static void io_net_vec_assign(struct io_kiocb *req, struct io_async_msghdr *kmsg,
 			     struct iovec *iov)
 {
 	if (iov) {
@@ -178,7 +178,6 @@ static int io_net_vec_assign(struct io_kiocb *req, struct io_async_msghdr *kmsg,
 			kfree(kmsg->free_iov);
 		kmsg->free_iov = iov;
 	}
-	return 0;
 }
 
 static inline void io_mshot_prep_retry(struct io_kiocb *req,
@@ -240,7 +239,8 @@ static int io_compat_msg_copy_hdr(struct io_kiocb *req,
 	if (unlikely(ret < 0))
 		return ret;
 
-	return io_net_vec_assign(req, iomsg, iov);
+	io_net_vec_assign(req, iomsg, iov);
+	return 0;
 }
 #endif
 
@@ -299,7 +299,8 @@ static int io_msg_copy_hdr(struct io_kiocb *req, struct io_async_msghdr *iomsg,
 	if (unlikely(ret < 0))
 		return ret;
 
-	return io_net_vec_assign(req, iomsg, iov);
+	io_net_vec_assign(req, iomsg, iov);
+	return 0;
 }
 
 static int io_sendmsg_copy_hdr(struct io_kiocb *req,
-- 
2.47.1

[PATCH 5/8] io_uring/net: clean io_msg_copy_hdr()

[Pavel Begunkov]

Put msg->msg_iov into a local variable in io_msg_copy_hdr(), it reads
better and clearly shows the used types.

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

diff --git a/io_uring/net.c b/io_uring/net.c
index e72205802055f..dedf274fc049a 100644
--- a/io_uring/net.c
+++ b/io_uring/net.c
@@ -280,11 +280,12 @@ static int io_msg_copy_hdr(struct io_kiocb *req, struct io_async_msghdr *iomsg,
 			ret = -EINVAL;
 			goto ua_end;
 		} else {
+			struct iovec __user *uiov = msg->msg_iov;
+
 			/* we only need the length for provided buffers */
-			if (!access_ok(&msg->msg_iov[0].iov_len, sizeof(__kernel_size_t)))
+			if (!access_ok(&uiov->iov_len, sizeof(uiov->iov_len)))
 				goto ua_end;
-			unsafe_get_user(iov->iov_len, &msg->msg_iov[0].iov_len,
-					ua_end);
+			unsafe_get_user(iov->iov_len, &uiov->iov_len, ua_end);
 			sr->len = iov->iov_len;
 		}
 		ret = 0;
-- 
2.47.1

[PATCH 6/8] io_uring/net: extract io_send_select_buffer()

[Pavel Begunkov]

Extract a helper out of io_send() for provided buffer selection to
improve readability as it has grown to take too many lines.

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

diff --git a/io_uring/net.c b/io_uring/net.c
index dedf274fc049a..4d21f7bd2149e 100644
--- a/io_uring/net.c
+++ b/io_uring/net.c
@@ -566,6 +566,54 @@ int io_sendmsg(struct io_kiocb *req, unsigned int issue_flags)
 	return IOU_OK;
 }
 
+static int io_send_select_buffer(struct io_kiocb *req, unsigned int issue_flags,
+				 struct io_async_msghdr *kmsg)
+{
+	struct io_sr_msg *sr = io_kiocb_to_cmd(req, struct io_sr_msg);
+
+	int ret;
+	struct buf_sel_arg arg = {
+		.iovs = &kmsg->fast_iov,
+		.max_len = min_not_zero(sr->len, INT_MAX),
+		.nr_iovs = 1,
+	};
+
+	if (kmsg->free_iov) {
+		arg.nr_iovs = kmsg->free_iov_nr;
+		arg.iovs = kmsg->free_iov;
+		arg.mode = KBUF_MODE_FREE;
+	}
+
+	if (!(sr->flags & IORING_RECVSEND_BUNDLE))
+		arg.nr_iovs = 1;
+	else
+		arg.mode |= KBUF_MODE_EXPAND;
+
+	ret = io_buffers_select(req, &arg, issue_flags);
+	if (unlikely(ret < 0))
+		return ret;
+
+	if (arg.iovs != &kmsg->fast_iov && arg.iovs != kmsg->free_iov) {
+		kmsg->free_iov_nr = ret;
+		kmsg->free_iov = arg.iovs;
+		req->flags |= REQ_F_NEED_CLEANUP;
+	}
+	sr->len = arg.out_len;
+
+	if (ret == 1) {
+		sr->buf = arg.iovs[0].iov_base;
+		ret = import_ubuf(ITER_SOURCE, sr->buf, sr->len,
+					&kmsg->msg.msg_iter);
+		if (unlikely(ret))
+			return ret;
+	} else {
+		iov_iter_init(&kmsg->msg.msg_iter, ITER_SOURCE,
+				arg.iovs, ret, arg.out_len);
+	}
+
+	return 0;
+}
+
 int io_send(struct io_kiocb *req, unsigned int issue_flags)
 {
 	struct io_sr_msg *sr = io_kiocb_to_cmd(req, struct io_sr_msg);
@@ -589,44 +637,9 @@ int io_send(struct io_kiocb *req, unsigned int issue_flags)
 
 retry_bundle:
 	if (io_do_buffer_select(req)) {
-		struct buf_sel_arg arg = {
-			.iovs = &kmsg->fast_iov,
-			.max_len = min_not_zero(sr->len, INT_MAX),
-			.nr_iovs = 1,
-		};
-
-		if (kmsg->free_iov) {
-			arg.nr_iovs = kmsg->free_iov_nr;
-			arg.iovs = kmsg->free_iov;
-			arg.mode = KBUF_MODE_FREE;
-		}
-
-		if (!(sr->flags & IORING_RECVSEND_BUNDLE))
-			arg.nr_iovs = 1;
-		else
-			arg.mode |= KBUF_MODE_EXPAND;
-
-		ret = io_buffers_select(req, &arg, issue_flags);
-		if (unlikely(ret < 0))
+		ret = io_send_select_buffer(req, issue_flags, kmsg);
+		if (ret)
 			return ret;
-
-		if (arg.iovs != &kmsg->fast_iov && arg.iovs != kmsg->free_iov) {
-			kmsg->free_iov_nr = ret;
-			kmsg->free_iov = arg.iovs;
-			req->flags |= REQ_F_NEED_CLEANUP;
-		}
-		sr->len = arg.out_len;
-
-		if (ret == 1) {
-			sr->buf = arg.iovs[0].iov_base;
-			ret = import_ubuf(ITER_SOURCE, sr->buf, sr->len,
-						&kmsg->msg.msg_iter);
-			if (unlikely(ret))
-				return ret;
-		} else {
-			iov_iter_init(&kmsg->msg.msg_iter, ITER_SOURCE,
-					arg.iovs, ret, arg.out_len);
-		}
 	}
 
 	/*
-- 
2.47.1

[PATCH 7/8] io_uring: remove !KASAN guards from cache free

[Pavel Begunkov]

Test setups (with KASAN) will avoid !KASAN sections, and so it's not
testing paths that would be exercised otherwise. That's bad as to be
sure that your code works you now have to specifically test both KASAN
and !KASAN configs.

Remove !CONFIG_KASAN guards from io_netmsg_cache_free() and
io_rw_cache_free(). The free functions should always be getting valid
entries, and even though for KASAN iovecs should already be cleared,
that's better than skipping the chunks completely.

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 io_uring/net.c | 2 --
 io_uring/rw.c  | 2 --
 2 files changed, 4 deletions(-)

diff --git a/io_uring/net.c b/io_uring/net.c
index 4d21f7bd2149e..d89c39f853e39 100644
--- a/io_uring/net.c
+++ b/io_uring/net.c
@@ -1813,10 +1813,8 @@ void io_netmsg_cache_free(const void *entry)
 {
 	struct io_async_msghdr *kmsg = (struct io_async_msghdr *) entry;
 
-#if !defined(CONFIG_KASAN)
 	if (kmsg->free_iov)
 		io_netmsg_iovec_free(kmsg);
-#endif
 	kfree(kmsg);
 }
 #endif
diff --git a/io_uring/rw.c b/io_uring/rw.c
index 991ecfbea88e3..c496f195aae2b 100644
--- a/io_uring/rw.c
+++ b/io_uring/rw.c
@@ -1309,9 +1309,7 @@ void io_rw_cache_free(const void *entry)
 {
 	struct io_async_rw *rw = (struct io_async_rw *) entry;
 
-#if !defined(CONFIG_KASAN)
 	if (rw->free_iovec)
 		io_rw_iovec_free(rw);
-#endif
 	kfree(rw);
 }
-- 
2.47.1

[PATCH 8/8] io_uring/rw: simplify io_rw_recycle()

[Pavel Begunkov]

Instead of freeing iovecs in case of IO_URING_F_UNLOCKED in
io_rw_recycle(), leave it be and rely on the core io_uring code to
call io_readv_writev_cleanup() later. This way the iovec will get
recycled and we can clean up io_rw_recycle() and kill
io_rw_iovec_free().

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

diff --git a/io_uring/rw.c b/io_uring/rw.c
index c496f195aae2b..7aa1e4c9f64a3 100644
--- a/io_uring/rw.c
+++ b/io_uring/rw.c
@@ -146,23 +146,13 @@ static inline int io_import_iovec(int rw, struct io_kiocb *req,
 	return 0;
 }
 
-static void io_rw_iovec_free(struct io_async_rw *rw)
-{
-	if (rw->free_iovec) {
-		kfree(rw->free_iovec);
-		rw->free_iov_nr = 0;
-		rw->free_iovec = NULL;
-	}
-}
-
 static void io_rw_recycle(struct io_kiocb *req, unsigned int issue_flags)
 {
 	struct io_async_rw *rw = req->async_data;
 
-	if (unlikely(issue_flags & IO_URING_F_UNLOCKED)) {
-		io_rw_iovec_free(rw);
+	if (unlikely(issue_flags & IO_URING_F_UNLOCKED))
 		return;
-	}
+
 	io_alloc_cache_kasan(&rw->free_iovec, &rw->free_iov_nr);
 	if (io_alloc_cache_put(&req->ctx->rw_cache, rw)) {
 		req->async_data = NULL;
@@ -1310,6 +1300,6 @@ void io_rw_cache_free(const void *entry)
 	struct io_async_rw *rw = (struct io_async_rw *) entry;
 
 	if (rw->free_iovec)
-		io_rw_iovec_free(rw);
+		kfree(rw->free_iovec);
 	kfree(rw);
 }
-- 
2.47.1

Re: [PATCH 0/8] alloc cache and iovec assorted cleanups

[Gabriel Krisman Bertazi]

Pavel Begunkov <asml.silence@gmail.com> writes:

> A bunch of patches cleaning allocation caches and various bits
> related to io vectors.

Reviewed-by: Gabriel Krisman Bertazi <krisman@suse.de>

>
> Pavel Begunkov (8):
>   io_uring: include all deps for alloc_cache.h
>   io_uring: dont ifdef io_alloc_cache_kasan()
>   io_uring: add alloc_cache.c
>   io_uring/net: make io_net_vec_assign() return void
>   io_uring/net: clean io_msg_copy_hdr()
>   io_uring/net: extract io_send_select_buffer()
>   io_uring: remove !KASAN guards from cache free
>   io_uring/rw: simplify io_rw_recycle()
>
>  io_uring/Makefile      |   2 +-
>  io_uring/alloc_cache.c |  44 +++++++++++++++++
>  io_uring/alloc_cache.h |  60 +++++++----------------
>  io_uring/net.c         | 105 +++++++++++++++++++++++------------------
>  io_uring/rw.c          |  18 ++-----
>  5 files changed, 123 insertions(+), 106 deletions(-)
>  create mode 100644 io_uring/alloc_cache.c

-- 
Gabriel Krisman Bertazi

Re: [PATCH 0/8] alloc cache and iovec assorted cleanups

[Jens Axboe]

On Tue, 28 Jan 2025 20:56:08 +0000, Pavel Begunkov wrote:
> A bunch of patches cleaning allocation caches and various bits
> related to io vectors.
> 
> Pavel Begunkov (8):
>   io_uring: include all deps for alloc_cache.h
>   io_uring: dont ifdef io_alloc_cache_kasan()
>   io_uring: add alloc_cache.c
>   io_uring/net: make io_net_vec_assign() return void
>   io_uring/net: clean io_msg_copy_hdr()
>   io_uring/net: extract io_send_select_buffer()
>   io_uring: remove !KASAN guards from cache free
>   io_uring/rw: simplify io_rw_recycle()
> 
> [...]

Applied, thanks!

[1/8] io_uring: include all deps for alloc_cache.h
      commit: 299276502d41cd86376f47b7e087d017eaa0f914
[2/8] io_uring: dont ifdef io_alloc_cache_kasan()
      commit: 16ac51a0a7aa051fd3b82fa077597488b5572d41
[3/8] io_uring: add alloc_cache.c
      commit: d19af0e9366298aa60afc0fb51ffcbd6205edcee
[4/8] io_uring/net: make io_net_vec_assign() return void
      commit: fefcb0dcd02fd34f808e91b13ce25f9847e52eb9
[5/8] io_uring/net: clean io_msg_copy_hdr()
      commit: 2b350f756b7acf84afab31d65ce6e3d496213ae5
[6/8] io_uring/net: extract io_send_select_buffer()
      commit: 86e62354eef16993834be5bd218d38ec96c47f16
[7/8] io_uring: remove !KASAN guards from cache free
      commit: 0d124578fed92cadeaca47d734da782beacdc1a7
[8/8] io_uring/rw: simplify io_rw_recycle()
      commit: d1fdab8c06791945d9454fb430951533eba9e175

Best regards,
-- 
Jens Axboe