[RFC PATCH 0/3] io_uring/rsrc: reduce node allocation cost on sparse file table installs

[RFC PATCH 0/3] io_uring/rsrc: reduce node allocation cost on sparse file table installs

[Uzair Beg]

Chengfeng Lin reported an 11.6% per-install slowdown in MSG_RING SEND_FD
fixed-file installation, bisected to 7029acd8a950 ("io_uring/rsrc: get rid
of per-ring io_rsrc_node list"):

  https://lore.kernel.org/io-uring/CANGjgdmt0FQ=offsdfn+wEaDxbOFoAa6bi92X_vEo4S6aCZ56A@mail.gmail.com/

That commit is not being questioned here. It removed a real serialisation
cost. The side effect is that each fixed-file install now allocates its own
io_rsrc_node, and on a first fill of a sparse table every one of those is
an allocator miss: io_alloc_cache_init() only allocates the pointer array,
io_alloc_cache_put() on the free path is the only thing that populates it,
and io_reset_rsrc_node() returns early on a NULL slot so nothing is freed
during a first fill. With IO_ALLOC_CACHE_MAX at 128, warming the existing
cache cannot cover a 4,096-slot fill.

We spent some time isolating where the per-install cost actually lives.
All timing below is Chengfeng's, on a bare-metal i7-12700KF with a pinned
core, performance governor, turbo off and fresh boots per point; the
evidence tree with reproducers and full logs is linked at the end.

  hypothesis                          result
  ----------------------------------  ---------------------------------
  slab merging defeats locality       slab_nomerge: no change
  SLAB_ACCOUNT on the new cache       +8.5% cost; removed (memcg
                                      accounting the old path never did)
  allocator call overhead             bulk refill, 32x fewer calls:
                                      +0.007%, path verified by probe
  fresh slab page creation            slab primed, 0 new pages: +0.9%
  per-object SLUB allocation path     prefill removes it: -9.5%

So the cost is the per-object allocation itself, not calls or pages, and
the only way to take it off the install path is to not allocate there.

This series does that in three steps:

  0001  dedicated kmem_cache for io_rsrc_node. Neutral on its own; exists
        so 0002/0003 can use kmem_cache_alloc_bulk().
  0002  bulk refill of the per-ring cache on a miss. Also neutral on the
        reported workload, for the reason above; kept for the machinery.
  0003  when a sparse table of N slots is registered, grow the per-ring
        cache to min(N, 4096) and bulk-fill it.

Measured on the actual series (v6.18-rc4, 6146a0f1dfae):

  workload                            unpatched   0001+2    +0003
  ----------------------------------  ---------  --------  --------
  reported 4,096-slot first fill,     119.423    118.151   107.731   -9.8%
    ns/install
  same-ring remove+refill, 4,096      712.451    710.384   578.365   -18.8%
    files, us
  one-shot register through fill,     518.387    520.663   526.873   +1.6%
    4,096 slots, us
  register 4096 / install 64, us       20.361     21.193    84.407   worse

The second row was not predicted: the enlarged cache retains nodes released
by FILES_UPDATE, so steady-state churn stops allocating entirely.

The last two rows are the cost, stated plainly. Prefill moves the
allocation work to registration rather than removing it, so a program
that registers once and fills once sees no total saving, and a program
that registers many slots and installs few pays for nodes it never uses
(up to 4096 x 32 bytes plus a 32 KiB pointer array, freed at ring
teardown).

Whether that trade is acceptable as default behaviour is the question this
RFC raises. An alternative would be an opt-in registration flag, so a
program that intends to fill the table can say so and nobody else pays. I
am happy to rework 0003 into that shape if it is the preferred one.

Testing: builds and boots on io_uring-6.18; the file table, rsrc and
msg_ring liburing tests pass on the patched kernel. The full runtests
suite was not run to completion on my build VM (the networking tests take
its interface down); the targeted set was. Chengfeng ran the alloc_cache
code through allocation-failure, limit and cleanup cases under
ASan/UBSan/LeakSanitizer, and verified the 8,192-slot case prefills 4,096
and falls through to bulk for the remainder.

Evidence, reproducers and diagnostic patches:
  https://github.com/lcf0399/linux-regression-evidence/tree/af2bf8eaae547d940c64ae8dcd1803baf31f2139/io-uring-msg-ring-send-fd-install

Uzair Beg (3):
  io_uring/rsrc: allocate io_rsrc_node from a dedicated kmem_cache
  io_uring/rsrc: bulk refill the node cache on allocation miss
  io_uring/rsrc: prefill the node cache when a file table is registered
    empty

 include/linux/io_uring_types.h |  1 +
 io_uring/alloc_cache.c         | 75 ++++++++++++++++++++++++++++++++--
 io_uring/alloc_cache.h         | 11 ++++-
 io_uring/io_uring.c            |  5 +++
 io_uring/io_uring.h            |  1 +
 io_uring/rsrc.c                |  6 +++
 6 files changed, 94 insertions(+), 5 deletions(-)

-- 
2.43.0

[RFC PATCH 1/3] io_uring/rsrc: allocate io_rsrc_node from a dedicated kmem_cache

[Uzair Beg]

io_rsrc_node allocations come from the generic kmalloc-32 bucket via
io_cache_alloc_new(). On a first fill of a sparse fixed file table the
per-ring node cache is empty by construction, since io_reset_rsrc_node()
returns early on a NULL slot and nothing is freed back, so every install
takes an allocator round trip.

Add an optional kmem_cache to io_alloc_cache and use it for the node
cache. The slab pointer defaults to NULL, so other io_alloc_cache users
are unchanged and imu_cache keeps using kmalloc, its element size being
variable. All three free paths honour the slab.

On its own this is neutral on the reported workload. It exists so that
the following patches can use kmem_cache_alloc_bulk(), which has no
equivalent for plain kmalloc.

Reported-by: Chengfeng Lin <lin2530632123@gmail.com>
Closes: https://lore.kernel.org/io-uring/CANGjgdmt0FQ=offsdfn+wEaDxbOFoAa6bi92X_vEo4S6aCZ56A@mail.gmail.com/
Tested-by: Chengfeng Lin <lin2530632123@gmail.com>
Signed-off-by: Uzair Beg <uzairbeg11@gmail.com>
---
 include/linux/io_uring_types.h |  1 +
 io_uring/alloc_cache.c         | 14 +++++++++++---
 io_uring/alloc_cache.h         |  8 ++++++--
 io_uring/io_uring.c            |  5 +++++
 io_uring/io_uring.h            |  1 +
 io_uring/rsrc.c                |  2 ++
 6 files changed, 26 insertions(+), 5 deletions(-)

diff --git a/include/linux/io_uring_types.h b/include/linux/io_uring_types.h
index c2ea6280901..e8d5a585a60 100644
--- a/include/linux/io_uring_types.h
+++ b/include/linux/io_uring_types.h
@@ -253,6 +253,7 @@ struct io_alloc_cache {
 	unsigned int		max_cached;
 	unsigned int		elem_size;
 	unsigned int		init_clear;
+	struct kmem_cache	*slab;
 };
 
 struct io_ring_ctx {
diff --git a/io_uring/alloc_cache.c b/io_uring/alloc_cache.c
index 58423888b73..a44b82a80f1 100644
--- a/io_uring/alloc_cache.c
+++ b/io_uring/alloc_cache.c
@@ -10,8 +10,12 @@ void io_alloc_cache_free(struct io_alloc_cache *cache,
 	if (!cache->entries)
 		return;
 
-	while ((entry = io_alloc_cache_get(cache)) != NULL)
-		free(entry);
+	while ((entry = io_alloc_cache_get(cache)) != NULL) {
+		if (cache->slab)
+			kmem_cache_free(cache->slab, entry);
+		else
+			free(entry);
+	}
 
 	kvfree(cache->entries);
 	cache->entries = NULL;
@@ -30,6 +34,7 @@ bool io_alloc_cache_init(struct io_alloc_cache *cache,
 	cache->max_cached = max_nr;
 	cache->elem_size = size;
 	cache->init_clear = init_bytes;
+	cache->slab = NULL;
 	return false;
 }
 
@@ -37,7 +42,10 @@ void *io_cache_alloc_new(struct io_alloc_cache *cache, gfp_t gfp)
 {
 	void *obj;
 
-	obj = kmalloc(cache->elem_size, gfp);
+	if (cache->slab)
+		obj = kmem_cache_alloc(cache->slab, gfp);
+	else
+		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 d33ce159ef3..b288bfccc91 100644
--- a/io_uring/alloc_cache.h
+++ b/io_uring/alloc_cache.h
@@ -61,8 +61,12 @@ static inline void *io_cache_alloc(struct io_alloc_cache *cache, gfp_t gfp)
 
 static inline void io_cache_free(struct io_alloc_cache *cache, void *obj)
 {
-	if (!io_alloc_cache_put(cache, obj))
-		kfree(obj);
+	if (!io_alloc_cache_put(cache, obj)) {
+		if (cache->slab)
+			kmem_cache_free(cache->slab, obj);
+		else
+			kfree(obj);
+	}
 }
 
 #endif
diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
index 296667ba712..f375f0ccc0e 100644
--- a/io_uring/io_uring.c
+++ b/io_uring/io_uring.c
@@ -151,6 +151,7 @@ static void __io_req_caches_free(struct io_ring_ctx *ctx);
 static __read_mostly DEFINE_STATIC_KEY_FALSE(io_key_has_sqarray);
 
 struct kmem_cache *req_cachep;
+struct kmem_cache *io_rsrc_node_cachep;
 static struct workqueue_struct *iou_wq __ro_after_init;
 
 static int __read_mostly sysctl_io_uring_disabled;
@@ -4074,6 +4075,10 @@ static int __init io_uring_init(void)
 				SLAB_HWCACHE_ALIGN | SLAB_PANIC | SLAB_ACCOUNT |
 				SLAB_TYPESAFE_BY_RCU);
 
+	io_rsrc_node_cachep = kmem_cache_create("io_rsrc_node",
+						sizeof(struct io_rsrc_node), NULL,
+						SLAB_HWCACHE_ALIGN | SLAB_PANIC);
+
 	iou_wq = alloc_workqueue("iou_exit", WQ_UNBOUND, 64);
 	BUG_ON(!iou_wq);
 
diff --git a/io_uring/io_uring.h b/io_uring/io_uring.h
index 46d9141d772..6243dadd507 100644
--- a/io_uring/io_uring.h
+++ b/io_uring/io_uring.h
@@ -527,6 +527,7 @@ static inline bool io_req_cache_empty(struct io_ring_ctx *ctx)
 }
 
 extern struct kmem_cache *req_cachep;
+extern struct kmem_cache *io_rsrc_node_cachep;
 
 static inline struct io_kiocb *io_extract_req(struct io_ring_ctx *ctx)
 {
diff --git a/io_uring/rsrc.c b/io_uring/rsrc.c
index d787c16dc1c..6413682ebe4 100644
--- a/io_uring/rsrc.c
+++ b/io_uring/rsrc.c
@@ -175,6 +175,8 @@ bool io_rsrc_cache_init(struct io_ring_ctx *ctx)
 				  node_size, 0);
 	ret |= io_alloc_cache_init(&ctx->imu_cache, IO_ALLOC_CACHE_MAX,
 				   imu_cache_size, 0);
+	if (!ret)
+		ctx->node_cache.slab = io_rsrc_node_cachep;
 	return ret;
 }
 
-- 
2.43.0

[RFC PATCH 2/3] io_uring/rsrc: bulk refill the node cache on allocation miss

[Uzair Beg]

A first fill of a sparse fixed file table takes one allocator round
trip per install, since the per-ring node cache starts empty and
nothing is freed back during the fill. At 4,096 slots that is 4,096
calls into the slab allocator.

When the cache has a dedicated kmem_cache, refill it in batches on a
miss: allocate up to IO_ALLOC_CACHE_REFILL objects with
kmem_cache_alloc_bulk(), return one and stash the remainder in the
cache. A 4,096-slot first fill then enters the allocator roughly once
per batch instead of once per object. This mirrors the existing bulk
allocation of requests from req_cachep.

kmem_cache_alloc_bulk() may return fewer objects than requested,
including zero; both cases are handled. Stashed objects have their
init_clear region zeroed and are poisoned like any other cached
entry, and the cache never grows past max_cached. Callers without a
dedicated slab are unchanged.

Bare-metal measurement shows this is neutral on the reported workload:
the per-object cost is in the SLUB allocation path itself, not in the
number of allocator entries. It is kept because the following patch
relies on the same bulk machinery.

Reported-by: Chengfeng Lin <lin2530632123@gmail.com>
Closes: https://lore.kernel.org/io-uring/CANGjgdmt0FQ=offsdfn+wEaDxbOFoAa6bi92X_vEo4S6aCZ56A@mail.gmail.com/
Tested-by: Chengfeng Lin <lin2530632123@gmail.com>
Signed-off-by: Uzair Beg <uzairbeg11@gmail.com>
---
 io_uring/alloc_cache.c | 29 ++++++++++++++++++++++++++---
 io_uring/alloc_cache.h |  1 +
 2 files changed, 27 insertions(+), 3 deletions(-)

diff --git a/io_uring/alloc_cache.c b/io_uring/alloc_cache.c
index a44b82a80f1..cba0e6c5d66 100644
--- a/io_uring/alloc_cache.c
+++ b/io_uring/alloc_cache.c
@@ -42,10 +42,33 @@ void *io_cache_alloc_new(struct io_alloc_cache *cache, gfp_t gfp)
 {
 	void *obj;
 
-	if (cache->slab)
-		obj = kmem_cache_alloc(cache->slab, gfp);
-	else
+	if (cache->slab) {
+		unsigned int room = cache->max_cached - cache->nr_cached;
+		void **slot = &cache->entries[cache->nr_cached];
+		unsigned int batch, got, i;
+
+		if (unlikely(!room))
+			return kmem_cache_alloc(cache->slab, gfp);
+
+		batch = min_t(unsigned int, IO_ALLOC_CACHE_REFILL, room);
+		got = kmem_cache_alloc_bulk(cache->slab, gfp, batch, slot);
+		if (unlikely(!got))
+			return NULL;
+
+		/* return one object, stash the rest in the cache */
+		obj = slot[got - 1];
+		for (i = 0; i < got - 1; i++) {
+			if (cache->init_clear)
+				memset(slot[i], 0, cache->init_clear);
+			if (unlikely(!kasan_mempool_poison_object(slot[i])))
+				break;
+			cache->nr_cached++;
+		}
+		for (; i < got - 1; i++)
+			kmem_cache_free(cache->slab, slot[i]);
+	} else {
 		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 b288bfccc91..82d552c7517 100644
--- a/io_uring/alloc_cache.h
+++ b/io_uring/alloc_cache.h
@@ -7,6 +7,7 @@
  * Don't allow the cache to grow beyond this size.
  */
 #define IO_ALLOC_CACHE_MAX	128
+#define IO_ALLOC_CACHE_REFILL	32
 
 void io_alloc_cache_free(struct io_alloc_cache *cache,
 			 void (*free)(const void *));
-- 
2.43.0

[RFC PATCH 3/3] io_uring/rsrc: prefill the node cache when a file table is registered empty

[Uzair Beg]

Registering a sparse fixed file table allocates no nodes at
registration time; each node is allocated later, on the install path,
where MSG_RING SEND_FD pays for it. Bare-metal measurement of the
4,096-slot first fill shows the cost is not the allocator call
(bulk refill was neutral) nor fresh slab pages (priming the slab was
neutral), but the per-object SLUB allocation path itself. The only way
to take it off the install path is to not allocate there.

When a sparse table of N slots is registered, grow the per-ring node
cache to min(N, IO_ALLOC_CACHE_PREFILL_MAX) and bulk-fill it, so the
subsequent installs hit the cache. Prefill is best-effort: on any
failure the cache is left in a valid state (a successfully grown
pointer array is retained) and registration proceeds unchanged.
Non-sparse registrations are untouched, since they allocate every node
inline anyway.

On the reported 4,096-slot first fill this is 9.8% faster than
unpatched. Because the enlarged cache also retains nodes released by
FILES_UPDATE, a same-ring remove-and-refill of 4,096 files is 18.8%
faster. The cost is moved to registration rather than removed: a
one-shot register-then-fill is unchanged overall, and a program that
registers many slots and installs few pays for nodes it never uses.
Whether that trade is acceptable, or should be behind a registration
flag, is the question this patch is intended to raise.

The stash loop from the bulk refill path is factored into a helper so
both callers share it.

Reported-by: Chengfeng Lin <lin2530632123@gmail.com>
Closes: https://lore.kernel.org/io-uring/CANGjgdmt0FQ=offsdfn+wEaDxbOFoAa6bi92X_vEo4S6aCZ56A@mail.gmail.com/
Tested-by: Chengfeng Lin <lin2530632123@gmail.com>
Co-developed-by: Chengfeng Lin <lin2530632123@gmail.com>
Signed-off-by: Chengfeng Lin <lin2530632123@gmail.com>
Signed-off-by: Uzair Beg <uzairbeg11@gmail.com>
---
 io_uring/alloc_cache.c | 58 ++++++++++++++++++++++++++++++++++--------
 io_uring/alloc_cache.h |  2 ++
 io_uring/rsrc.c        |  4 +++
 3 files changed, 54 insertions(+), 10 deletions(-)

diff --git a/io_uring/alloc_cache.c b/io_uring/alloc_cache.c
index cba0e6c5d66..2c6e09313d2 100644
--- a/io_uring/alloc_cache.c
+++ b/io_uring/alloc_cache.c
@@ -38,6 +38,22 @@ bool io_alloc_cache_init(struct io_alloc_cache *cache,
 	return false;
 }
 
+static void io_cache_stash(struct io_alloc_cache *cache, void **slot,
+			   unsigned int nr)
+{
+	unsigned int i;
+
+	for (i = 0; i < nr; i++) {
+		if (cache->init_clear)
+			memset(slot[i], 0, cache->init_clear);
+		if (unlikely(!kasan_mempool_poison_object(slot[i])))
+			break;
+		cache->nr_cached++;
+	}
+	for (; i < nr; i++)
+		kmem_cache_free(cache->slab, slot[i]);
+}
+
 void *io_cache_alloc_new(struct io_alloc_cache *cache, gfp_t gfp)
 {
 	void *obj;
@@ -45,7 +61,7 @@ void *io_cache_alloc_new(struct io_alloc_cache *cache, gfp_t gfp)
 	if (cache->slab) {
 		unsigned int room = cache->max_cached - cache->nr_cached;
 		void **slot = &cache->entries[cache->nr_cached];
-		unsigned int batch, got, i;
+		unsigned int batch, got;
 
 		if (unlikely(!room))
 			return kmem_cache_alloc(cache->slab, gfp);
@@ -57,15 +73,7 @@ void *io_cache_alloc_new(struct io_alloc_cache *cache, gfp_t gfp)
 
 		/* return one object, stash the rest in the cache */
 		obj = slot[got - 1];
-		for (i = 0; i < got - 1; i++) {
-			if (cache->init_clear)
-				memset(slot[i], 0, cache->init_clear);
-			if (unlikely(!kasan_mempool_poison_object(slot[i])))
-				break;
-			cache->nr_cached++;
-		}
-		for (; i < got - 1; i++)
-			kmem_cache_free(cache->slab, slot[i]);
+		io_cache_stash(cache, slot, got - 1);
 	} else {
 		obj = kmalloc(cache->elem_size, gfp);
 	}
@@ -73,3 +81,33 @@ void *io_cache_alloc_new(struct io_alloc_cache *cache, gfp_t gfp)
 		memset(obj, 0, cache->init_clear);
 	return obj;
 }
+
+void io_alloc_cache_prefill(struct io_alloc_cache *cache, unsigned int nr)
+{
+	gfp_t gfp = GFP_KERNEL | __GFP_NOWARN;
+	unsigned int got;
+	void **entries;
+
+	if (!cache->slab || !cache->entries)
+		return;
+
+	nr = min_t(unsigned int, nr, IO_ALLOC_CACHE_PREFILL_MAX);
+	if (nr <= cache->nr_cached)
+		return;
+
+	if (nr > cache->max_cached) {
+		entries = kvmalloc_array(nr, sizeof(void *), gfp);
+		if (!entries)
+			return;
+		memcpy(entries, cache->entries,
+		       cache->nr_cached * sizeof(void *));
+		kvfree(cache->entries);
+		cache->entries = entries;
+		cache->max_cached = nr;
+	}
+
+	got = kmem_cache_alloc_bulk(cache->slab, gfp, nr - cache->nr_cached,
+				    &cache->entries[cache->nr_cached]);
+	if (got)
+		io_cache_stash(cache, &cache->entries[cache->nr_cached], got);
+}
diff --git a/io_uring/alloc_cache.h b/io_uring/alloc_cache.h
index 82d552c7517..ca6af52dd7d 100644
--- a/io_uring/alloc_cache.h
+++ b/io_uring/alloc_cache.h
@@ -8,6 +8,7 @@
  */
 #define IO_ALLOC_CACHE_MAX	128
 #define IO_ALLOC_CACHE_REFILL	32
+#define IO_ALLOC_CACHE_PREFILL_MAX	4096
 
 void io_alloc_cache_free(struct io_alloc_cache *cache,
 			 void (*free)(const void *));
@@ -16,6 +17,7 @@ bool io_alloc_cache_init(struct io_alloc_cache *cache,
 			 unsigned int init_bytes);
 
 void *io_cache_alloc_new(struct io_alloc_cache *cache, gfp_t gfp);
+void io_alloc_cache_prefill(struct io_alloc_cache *cache, unsigned int nr);
 
 static inline bool io_alloc_cache_put(struct io_alloc_cache *cache,
 				      void *entry)
diff --git a/io_uring/rsrc.c b/io_uring/rsrc.c
index 6413682ebe4..b7f78b6b514 100644
--- a/io_uring/rsrc.c
+++ b/io_uring/rsrc.c
@@ -560,6 +560,10 @@ int io_sqe_files_register(struct io_ring_ctx *ctx, void __user *arg,
 	if (!io_alloc_file_tables(ctx, &ctx->file_table, nr_args))
 		return -ENOMEM;
 
+	/* sparse table: nodes are installed later, so cache them now */
+	if (!fds)
+		io_alloc_cache_prefill(&ctx->node_cache, nr_args);
+
 	for (i = 0; i < nr_args; i++) {
 		struct io_rsrc_node *node;
 		u64 tag = 0;
-- 
2.43.0

Re: [RFC PATCH 1/3] io_uring/rsrc: allocate io_rsrc_node from a dedicated kmem_cache

[Gabriel Krisman Bertazi]

Uzair Beg <uzairbeg11@gmail.com> writes:

> io_rsrc_node allocations come from the generic kmalloc-32 bucket via
> io_cache_alloc_new(). On a first fill of a sparse fixed file table the
> per-ring node cache is empty by construction, since io_reset_rsrc_node()
> returns early on a NULL slot and nothing is freed back, so every install
> takes an allocator round trip.
>
> Add an optional kmem_cache to io_alloc_cache and use it for the node
> cache. The slab pointer defaults to NULL, so other io_alloc_cache users
> are unchanged and imu_cache keeps using kmalloc, its element size being
> variable. All three free paths honour the slab.
>
> On its own this is neutral on the reported workload. It exists so that
> the following patches can use kmem_cache_alloc_bulk(), which has no
> equivalent for plain kmalloc.
>
> Reported-by: Chengfeng Lin <lin2530632123@gmail.com>
> Closes: https://lore.kernel.org/io-uring/CANGjgdmt0FQ=offsdfn+wEaDxbOFoAa6bi92X_vEo4S6aCZ56A@mail.gmail.com/
> Tested-by: Chengfeng Lin <lin2530632123@gmail.com>
> Signed-off-by: Uzair Beg <uzairbeg11@gmail.com>
> ---
>  include/linux/io_uring_types.h |  1 +
>  io_uring/alloc_cache.c         | 14 +++++++++++---
>  io_uring/alloc_cache.h         |  8 ++++++--
>  io_uring/io_uring.c            |  5 +++++
>  io_uring/io_uring.h            |  1 +
>  io_uring/rsrc.c                |  2 ++
>  6 files changed, 26 insertions(+), 5 deletions(-)
>
> diff --git a/include/linux/io_uring_types.h b/include/linux/io_uring_types.h
> index c2ea6280901..e8d5a585a60 100644
> --- a/include/linux/io_uring_types.h
> +++ b/include/linux/io_uring_types.h
> @@ -253,6 +253,7 @@ struct io_alloc_cache {
>  	unsigned int		max_cached;
>  	unsigned int		elem_size;
>  	unsigned int		init_clear;
> +	struct kmem_cache	*slab;
>  };

Not opposed to having a slab, but I actually think it is
unnecessary.  see patch 2.  If we do that, we actually should delete the
async_size parameter in the opdef, since it is now redundant.  But
see comments in patch 2.

>  
>  struct io_ring_ctx {
> diff --git a/io_uring/alloc_cache.c b/io_uring/alloc_cache.c
> index 58423888b73..a44b82a80f1 100644
> --- a/io_uring/alloc_cache.c
> +++ b/io_uring/alloc_cache.c
> @@ -10,8 +10,12 @@ void io_alloc_cache_free(struct io_alloc_cache *cache,
>  	if (!cache->entries)
>  		return;
>  
> -	while ((entry = io_alloc_cache_get(cache)) != NULL)
> -		free(entry);
> +	while ((entry = io_alloc_cache_get(cache)) != NULL) {
> +		if (cache->slab)
> +			kmem_cache_free(cache->slab, entry);
> +		else
> +			free(entry);
> +	}

FWIW, this is exactly why we have the (*free) callback, you should have a function
for your specific type used as a callback that does the kmem_cache_free
so you don't need the if/else here.

But for a simple kmem_cache_free, kfree works just fine.

-- 
Gabriel Krisman Bertazi

Re: [RFC PATCH 3/3] io_uring/rsrc: prefill the node cache when a file table is registered empty

[Gabriel Krisman Bertazi]

Uzair Beg <uzairbeg11@gmail.com> writes:

> Registering a sparse fixed file table allocates no nodes at
> registration time; each node is allocated later, on the install path,
> where MSG_RING SEND_FD pays for it. Bare-metal measurement of the
> 4,096-slot first fill shows the cost is not the allocator call
> (bulk refill was neutral) nor fresh slab pages (priming the slab was
> neutral), but the per-object SLUB allocation path itself. The only way
> to take it off the install path is to not allocate there.
>
> When a sparse table of N slots is registered, grow the per-ring node
> cache to min(N, IO_ALLOC_CACHE_PREFILL_MAX) and bulk-fill it, so the
> subsequent installs hit the cache. Prefill is best-effort: on any
> failure the cache is left in a valid state (a successfully grown
> pointer array is retained) and registration proceeds unchanged.
> Non-sparse registrations are untouched, since they allocate every node
> inline anyway.

I'm unconvinced this is the right approach.  This is only relevant for
initialization overhead: once the ring is in operation, the overhead is
gone because nodes are recycled.  So this will really benefit
short-lived applications that create large tables, something that I
suspect is rare outside of artificial benchmarks.  On the other hand,
people are creating sparse but arbitrarily large tables.  Does it make
sense to pre-allocated up to 192KB in memory for short-lived
applications that might use only a couple of those nodes?

> On the reported 4,096-slot first fill this is 9.8% faster than
> unpatched. Because the enlarged cache also retains nodes released by
> FILES_UPDATE, a same-ring remove-and-refill of 4,096 files is 18.8%
> faster. The cost is moved to registration rather than removed: a
> one-shot register-then-fill is unchanged overall, and a program that
> registers many slots and installs few pays for nodes it never uses.
> Whether that trade is acceptable, or should be behind a registration
> flag, is the question this patch is intended to raise.
>
> The stash loop from the bulk refill path is factored into a helper so
> both callers share it.
>
> Reported-by: Chengfeng Lin <lin2530632123@gmail.com>
> Closes: https://lore.kernel.org/io-uring/CANGjgdmt0FQ=offsdfn+wEaDxbOFoAa6bi92X_vEo4S6aCZ56A@mail.gmail.com/
> Tested-by: Chengfeng Lin <lin2530632123@gmail.com>
> Co-developed-by: Chengfeng Lin <lin2530632123@gmail.com>
> Signed-off-by: Chengfeng Lin <lin2530632123@gmail.com>
> Signed-off-by: Uzair Beg <uzairbeg11@gmail.com>
> ---
>  io_uring/alloc_cache.c | 58 ++++++++++++++++++++++++++++++++++--------
>  io_uring/alloc_cache.h |  2 ++
>  io_uring/rsrc.c        |  4 +++
>  3 files changed, 54 insertions(+), 10 deletions(-)
>
> diff --git a/io_uring/alloc_cache.c b/io_uring/alloc_cache.c
> index cba0e6c5d66..2c6e09313d2 100644
> --- a/io_uring/alloc_cache.c
> +++ b/io_uring/alloc_cache.c
> @@ -38,6 +38,22 @@ bool io_alloc_cache_init(struct io_alloc_cache *cache,
>  	return false;
>  }
>  
> +static void io_cache_stash(struct io_alloc_cache *cache, void **slot,
> +			   unsigned int nr)
> +{
> +	unsigned int i;
> +
> +	for (i = 0; i < nr; i++) {
> +		if (cache->init_clear)
> +			memset(slot[i], 0, cache->init_clear);
> +		if (unlikely(!kasan_mempool_poison_object(slot[i])))
> +			break;
> +		cache->nr_cached++;
> +	}
> +	for (; i < nr; i++)
> +		kmem_cache_free(cache->slab, slot[i]);
> +}
> +
>  void *io_cache_alloc_new(struct io_alloc_cache *cache, gfp_t gfp)
>  {
>  	void *obj;
> @@ -45,7 +61,7 @@ void *io_cache_alloc_new(struct io_alloc_cache *cache, gfp_t gfp)
>  	if (cache->slab) {
>  		unsigned int room = cache->max_cached - cache->nr_cached;
>  		void **slot = &cache->entries[cache->nr_cached];
> -		unsigned int batch, got, i;
> +		unsigned int batch, got;
>  
>  		if (unlikely(!room))
>  			return kmem_cache_alloc(cache->slab, gfp);
> @@ -57,15 +73,7 @@ void *io_cache_alloc_new(struct io_alloc_cache *cache, gfp_t gfp)
>  
>  		/* return one object, stash the rest in the cache */
>  		obj = slot[got - 1];
> -		for (i = 0; i < got - 1; i++) {
> -			if (cache->init_clear)
> -				memset(slot[i], 0, cache->init_clear);
> -			if (unlikely(!kasan_mempool_poison_object(slot[i])))
> -				break;
> -			cache->nr_cached++;
> -		}
> -		for (; i < got - 1; i++)
> -			kmem_cache_free(cache->slab, slot[i]);
> +		io_cache_stash(cache, slot, got - 1);
>  	} else {
>  		obj = kmalloc(cache->elem_size, gfp);
>  	}
> @@ -73,3 +81,33 @@ void *io_cache_alloc_new(struct io_alloc_cache *cache, gfp_t gfp)
>  		memset(obj, 0, cache->init_clear);
>  	return obj;
>  }
> +
> +void io_alloc_cache_prefill(struct io_alloc_cache *cache, unsigned int nr)
> +{
> +	gfp_t gfp = GFP_KERNEL | __GFP_NOWARN;
> +	unsigned int got;
> +	void **entries;
> +
> +	if (!cache->slab || !cache->entries)
> +		return;
> +
> +	nr = min_t(unsigned int, nr, IO_ALLOC_CACHE_PREFILL_MAX);
> +	if (nr <= cache->nr_cached)
> +		return;
> +
> +	if (nr > cache->max_cached) {
> +		entries = kvmalloc_array(nr, sizeof(void *), gfp);
> +		if (!entries)
> +			return;
> +		memcpy(entries, cache->entries,
> +		       cache->nr_cached * sizeof(void *));
> +		kvfree(cache->entries);
> +		cache->entries = entries;
> +		cache->max_cached = nr;
> +	}
> +
> +	got = kmem_cache_alloc_bulk(cache->slab, gfp, nr - cache->nr_cached,
> +				    &cache->entries[cache->nr_cached]);
> +	if (got)
> +		io_cache_stash(cache, &cache->entries[cache->nr_cached], got);
> +}
> diff --git a/io_uring/alloc_cache.h b/io_uring/alloc_cache.h
> index 82d552c7517..ca6af52dd7d 100644
> --- a/io_uring/alloc_cache.h
> +++ b/io_uring/alloc_cache.h
> @@ -8,6 +8,7 @@
>   */
>  #define IO_ALLOC_CACHE_MAX	128
>  #define IO_ALLOC_CACHE_REFILL	32
> +#define IO_ALLOC_CACHE_PREFILL_MAX	4096
>  
>  void io_alloc_cache_free(struct io_alloc_cache *cache,
>  			 void (*free)(const void *));
> @@ -16,6 +17,7 @@ bool io_alloc_cache_init(struct io_alloc_cache *cache,
>  			 unsigned int init_bytes);
>  
>  void *io_cache_alloc_new(struct io_alloc_cache *cache, gfp_t gfp);
> +void io_alloc_cache_prefill(struct io_alloc_cache *cache, unsigned int nr);
>  
>  static inline bool io_alloc_cache_put(struct io_alloc_cache *cache,
>  				      void *entry)
> diff --git a/io_uring/rsrc.c b/io_uring/rsrc.c
> index 6413682ebe4..b7f78b6b514 100644
> --- a/io_uring/rsrc.c
> +++ b/io_uring/rsrc.c
> @@ -560,6 +560,10 @@ int io_sqe_files_register(struct io_ring_ctx *ctx, void __user *arg,
>  	if (!io_alloc_file_tables(ctx, &ctx->file_table, nr_args))
>  		return -ENOMEM;
>  
> +	/* sparse table: nodes are installed later, so cache them now */
> +	if (!fds)
> +		io_alloc_cache_prefill(&ctx->node_cache, nr_args);
> +
>  	for (i = 0; i < nr_args; i++) {
>  		struct io_rsrc_node *node;
>  		u64 tag = 0;
> -- 
> 2.43.0
>

-- 
Gabriel Krisman Bertazi