mm/slub: serve slabobj_ext array from a strictly larger kmalloc cache

A production host in the Meta fleet (6.16 kernel, memory allocation profiling enabled) panicked with a kernel stack overflow while a kernel driver was freeing a resource:

  BUG: TASK stack guard page was hit
  Oops: stack guard page
  RIP: 0010:kfree+0x8/0x5d0
  Call Trace:
   __free_slab+0x66/0xc0
   kfree+0x3f0/0x5d0
   ... ( ~125x __free_slab <-> kfree ) ...
   <kernel driver freeing a resource>
   do_syscall_64

The crash dump shows a 125-deep __free_slab<->kfree recursion that overflowed the 16 KiB kernel stack.

What happened: a KMALLOC_NORMAL slab's obj_exts array (used by allocation profiling / memcg accounting) is itself kmalloc()'d from a KMALLOC_NORMAL cache,

so the "slab holds another slab's obj_exts array" relation can form cycles. With sizeof(struct slabobj_ext) == 16 and the host's geometry:

- kmalloc-512 has 64 objects/slab -> array is 64*16 == 1024 bytes,

    served from kmalloc-1k;

- kmalloc-1k has 32 objects/slab -> array is 32*16 == 512 bytes, served from kmalloc-512.

A kmalloc-512 slab and a kmalloc-1k slab therefore hold each other's obj_exts array. Discarding one frees the other's array, which empties and discards that slab, which frees the first's array, and so on:
__free_slab() -> free_slab_obj_exts() -> kfree() -> discard_slab() -> __free_slab() recurses along the cycle until the stack is exhausted.

The
dump confirms it: the recursion's slabs strictly alternate kmalloc-512 (obj_exts in kmalloc-1k) and kmalloc-1k (obj_exts in kmalloc-512), and mem_alloc_profiling_key was enabled.

Commit 280ea9c3154b ("mm/slab: avoid allocating slabobj_ext array from its own slab") is not sufficient: it bumps the allocation size only when the array would come from the *same* cache (object_size ==). At the geometry above neither cache is self-referential (512 != 1024 and 1024 != 512), so the bump never triggers and the kmalloc-512 <-> kmalloc-1k cross cycle remains.

Fix it structurally by removing cycles of every shape: serve the array from a cache strictly larger than the one it describes whenever it would otherwise come from the same or a smaller cache. Every reference edge then points from a smaller to a larger cache (here kmalloc-1k's array moves to kmalloc-2k), so the relation is a DAG and cannot contain a cycle.

No slab can be self- or cross-pinned, the tear-down recursion is bounded by the number of kmalloc size classes (it terminates at the large-kmalloc path, which carries no obj_exts), and profiling/accounting coverage is unchanged - the array is still allocated, only relocated.

Reproduced on next-20260623 at the same geometry: churning kmalloc-512/kmalloc-1k under vm.mem_profiling and then shrinking leaves kmalloc-512 with thousands of unreclaimable objects without this patch (8056) and at baseline with it (847).

Fixes 4b8736964640 ("mm/slab: add allocation accounting into slab allocation and free paths")

Signed-off-by Shakeel Butt <[email protected]>

Reported-by Danielle Costantino <[email protected]>

---
 mm/slub.c | 26 ++++++++++----------------
 1 file changed, 10 insertions(+), 16 deletions(-)

mm/slub.c +7 -8

index 9ec774dc7009..48e54d340865 100644
@@ -2124,15 +2124,14 @@ static inline void init_slab_obj_exts(struct slab *slab)
 }
 
 /*
- * Calculate the allocation size for slabobj_ext array.
+ * Size of the slabobj_ext array for @slab.
  *
- * When memory allocation profiling is enabled, the obj_exts array
- * could be allocated from the same slab cache it's being allocated for.
- * This would prevent the slab from ever being freed because it would
- * always contain at least one allocated object (its own obj_exts array).
- *
- * To avoid this, increase the allocation size when we detect the array
- * may come from the same cache, forcing it to use a different cache.
+ * The array is itself kmalloc()'d. If it came from the same or a smaller
+ * kmalloc cache than @s, the "slab holds another slab's array" relation could
+ * form a cycle (self, or e.g. kmalloc-512 <-> kmalloc-1k) that pins the slabs
+ * forever and recurses via free_slab_obj_exts() -> kfree() -> discard_slab()
+ * at teardown. Force it into a strictly larger cache to keep that relation a
+ * DAG (acyclic).
  */
 static inline size_t obj_exts_alloc_size(struct kmem_cache *s,
 					 struct slab *slab, gfp_t gfp)

@@ -2147,14 +2146,9 @@ static inline size_t obj_exts_alloc_size(struct kmem_cache *s,
 		return sz;
 

mm/slub.c +2 -0

+	if (obj_exts_cache->object_size <= s->object_size)
+		return s->object_size + 1;
 
 	return sz;
 }

Hi Shakeel,

Quoted 3 lines

What happened: a KMALLOC_NORMAL slab's obj_exts array (used by allocation profiling / memcg accounting) is itself kmalloc()'d from a KMALLOC_NORMAL cache,

Usually KMALLOC_NORMAL caches don't need obj_exts array, but yes, this could happen if memory allocation profiling is enabled.

Quoted 6 lines

Fix it structurally by removing cycles of every shape: serve the array from a cache strictly larger than the one it describes whenever it would otherwise come from the same or a smaller cache. Every reference edge then points from a smaller to a larger cache (here kmalloc-1k's array moves to kmalloc-2k), so the relation is a DAG and cannot contain a cycle.

This will fix the problem.

But this will waste memory as we need smaller obj_exts array as the size gets larger.

We should probably create a new kmalloc type to avoid cycles instead? (needed only when memory profiling is enabled, though)

That would also prevent recursion even further.

Quoted 6 lines

No slab can be self- or cross-pinned, the tear-down recursion is bounded by the number of kmalloc size classes (it terminates at the large-kmalloc path, which carries no obj_exts), and profiling/accounting coverage is unchanged - the array is still allocated, only relocated.

Quoted 6 lines

Reproduced on next-20260623 at the same geometry: churning kmalloc-512/kmalloc-1k under vm.mem_profiling and then shrinking leaves kmalloc-512 with thousands of unreclaimable objects without this patch (8056) and at baseline with it (847).

Fixes 4b8736964640 ("mm/slab: add allocation accounting into slab allocation and free paths")

Perhaps Cc: stable? v6.12 and v6.18 are affected.

mm/slub.c static inline size_t obj_exts_alloc_size(struct kmem_cache *s, · line 2149

Quoted 3 lines

mm/slub.c +1 -2

-	if (s->object_size == obj_exts_cache->object_size)
-		return obj_exts_cache->object_size + 1;
+	/* compare object_size, not the cache pointer (partitioned kmalloc caches) */

This comment is no longer relevant, by the way.

"compare object_size instead of cache pointers because there can be
multiple caches of the same size" doesn't apply anymore.

Quoted 5 lines

mm/slub.c +2 -0

+	if (obj_exts_cache->object_size <= s->object_size)
+		return s->object_size + 1;
 
 	return sz;
 }

Signature · 2 lines

Cheers,
Harry / Hyeonggon

Quoted 3 lines · Harry Yoo

Hi Shakeel,

[...]

Quoted 6 lines

Quoted 3 lines

What happened: a KMALLOC_NORMAL slab's obj_exts array (used by allocation profiling / memcg accounting) is itself kmalloc()'d from a KMALLOC_NORMAL cache,

Usually KMALLOC_NORMAL caches don't need obj_exts array, but yes, this could happen if memory allocation profiling is enabled.

Yes, we have enabled memory allocation profiling fleet wide.

[...]

Quoted 12 lines

Quoted 6 lines

Fix it structurally by removing cycles of every shape: serve the array from a cache strictly larger than the one it describes whenever it would otherwise come from the same or a smaller cache. Every reference edge then points from a smaller to a larger cache (here kmalloc-1k's array moves to kmalloc-2k), so the relation is a DAG and cannot contain a cycle.

This will fix the problem.

But this will waste memory as we need smaller obj_exts array as the size gets larger.

Quoted 4 lines

We should probably create a new kmalloc type to avoid cycles instead? (needed only when memory profiling is enabled, though)

That would also prevent recursion even further.

Yes but I assume that would add kmem caches even for users not using memory profiling. Anyways, I think that is a separate discussion. Am I understanding correctly that you don't have any concerns with this approach?

Quoted 6 lines

Quoted 6 lines

No slab can be self- or cross-pinned, the tear-down recursion is bounded by the number of kmalloc size classes (it terminates at the large-kmalloc path, which carries no obj_exts), and profiling/accounting coverage is unchanged - the array is still allocated, only relocated.

Quoted 8 lines

Quoted 6 lines

Reproduced on next-20260623 at the same geometry: churning kmalloc-512/kmalloc-1k under vm.mem_profiling and then shrinking leaves kmalloc-512 with thousands of unreclaimable objects without this patch (8056) and at baseline with it (847).

Fixes 4b8736964640 ("mm/slab: add allocation accounting into slab allocation and free paths")

Perhaps Cc: stable? v6.12 and v6.18 are affected.

Ack.

[...]

Quoted 9 lines

mm/slub.c static inline size_t obj_exts_alloc_size(struct kmem_cache *s, · line 2149

Quoted 3 lines

mm/slub.c +1 -2

-	if (s->object_size == obj_exts_cache->object_size)
-		return obj_exts_cache->object_size + 1;
+	/* compare object_size, not the cache pointer (partitioned kmalloc caches) */

This comment is no longer relevant, by the way.

"compare object_size instead of cache pointers because there can be
multiple caches of the same size" doesn't apply anymore.

I will remove the comment in next version.

Thanks for the review.

Quoted 9 lines

Quoted 5 lines

mm/slub.c +2 -0

+	if (obj_exts_cache->object_size <= s->object_size)
+		return s->object_size + 1;
 
 	return sz;
 }

Signature · 2 lines

Cheers,
Harry / Hyeonggon

Quoted 11 lines

Quoted 11 lines

Quoted 5 lines

Fix it structurally by removing cycles of every shape: serve the array from a cache strictly larger than the one it describes whenever it would otherwise come from the same or a smaller cache. Every reference edge then points from a smaller to a larger cache (here kmalloc-1k's array moves to kmalloc-2k), so the relation is a DAG and cannot contain a cycle.

This will fix the problem.

But this will waste memory as we need smaller obj_exts array as the size gets larger.

Quoted 8 lines

Quoted 4 lines

We should probably create a new kmalloc type to avoid cycles instead? (needed only when memory profiling is enabled, though)

That would also prevent recursion even further.

Yes but I assume that would add kmem caches even for users not using memory profiling. Anyways, I think that is a separate discussion. Am I understanding correctly that you don't have any concerns with this approach?

Umm, the memory waste is a concern?

Minimally I'd now want to only do that size bumping when allocation profiling is enabled. Ideally that means both configured in and not booted with "never".

We probably should have done that already in 280ea9c3154b2. Because AFAIU memcg-only obj_exts array don't have this issue (or maybe they do have the
[1] issue? Harry?). But if memcg-only should keep avoiding the same size bucket, it can keep what it was doing and only memalloc profiling would do the strictly larger thing.

Suren's input would be also nice to have.

Thanks!

[1] https://lore.kernel.org/oe-lkp/[email protected]

On Fri, Jun 26, 2026 at 07:11:33PM +0200, Vlastimil Babka (SUSE) wrote:
[...]

Quoted 26 lines

Quoted 11 lines

Quoted 11 lines

Quoted 5 lines

Fix it structurally by removing cycles of every shape: serve the array from a cache strictly larger than the one it describes whenever it would otherwise come from the same or a smaller cache. Every reference edge then points from a smaller to a larger cache (here kmalloc-1k's array moves to kmalloc-2k), so the relation is a DAG and cannot contain a cycle.

This will fix the problem.

But this will waste memory as we need smaller obj_exts array as the size gets larger.

Quoted 8 lines

Quoted 4 lines

We should probably create a new kmalloc type to avoid cycles instead? (needed only when memory profiling is enabled, though)

That would also prevent recursion even further.

Yes but I assume that would add kmem caches even for users not using memory profiling. Anyways, I think that is a separate discussion. Am I understanding correctly that you don't have any concerns with this approach?

Umm, the memory waste is a concern?

Minimally I'd now want to only do that size bumping when allocation profiling is enabled. Ideally that means both configured in and not booted with "never".

Quoted 5 lines

We probably should have done that already in 280ea9c3154b2. Because AFAIU memcg-only obj_exts array don't have this issue (or maybe they do have the
[1] issue? Harry?). But if memcg-only should keep avoiding the same size bucket, it can keep what it was doing and only memalloc profiling would do the strictly larger thing.

memcg should not have this issue as normal kmalloc caches do not serve memcg charged objects.

So here we can do dedicated caches as Harry suggested or make this size bumping very specialized as Vlastimil suggested. What do we want long term? Orthogonally we do want this fix to be backported easily to older stable kernels. I will see how does this narrowed down size bumping looks like.

Quoted 7 lines

Suren's input would be also nice to have.

Thanks!

[1] https://lore.kernel.org/oe-lkp/[email protected]

Quoted 34 lines · Shakeel Butt

On Fri, Jun 26, 2026 at 07:11:33PM +0200, Vlastimil Babka (SUSE) wrote:
[...]

Quoted 31 lines

Quoted 11 lines

Quoted 11 lines

Quoted 5 lines

Fix it structurally by removing cycles of every shape: serve the array from a cache strictly larger than the one it describes whenever it would otherwise come from the same or a smaller cache. Every reference edge then points from a smaller to a larger cache (here kmalloc-1k's array moves to kmalloc-2k), so the relation is a DAG and cannot contain a cycle.

This will fix the problem.

But this will waste memory as we need smaller obj_exts array as the size gets larger.

Quoted 8 lines

Quoted 4 lines

We should probably create a new kmalloc type to avoid cycles instead? (needed only when memory profiling is enabled, though)

That would also prevent recursion even further.

Yes but I assume that would add kmem caches even for users not using memory profiling. Anyways, I think that is a separate discussion. Am I understanding correctly that you don't have any concerns with this approach?

Umm, the memory waste is a concern?

Minimally I'd now want to only do that size bumping when allocation profiling is enabled. Ideally that means both configured in and not booted with "never".

We probably should have done that already in 280ea9c3154b2. Because AFAIU memcg-only obj_exts array don't have this issue (or maybe they do have the
[1] issue? Harry?). But if memcg-only should keep avoiding the same size bucket, it can keep what it was doing and only memalloc profiling would do the strictly larger thing.

Quoted 2 lines

memcg should not have this issue as normal kmalloc caches do not serve memcg charged objects.

I am wrong here as I went back and see d8df600b67d7.

Quoted 3 lines

So here we can do dedicated caches as Harry suggested or make this size bumping very specialized as Vlastimil suggested. What do we want long term? Orthogonally

Quoted 3 lines

we do want this fix to be backported easily to older stable kernels. I will see how does this narrowed down size bumping looks like.

BTW I think we need something like the following, right?

	if (mem_alloc_profiling_enabled()) {
		if (obj_exts_cache->object_size <= s->object_size)
			return s->object_size + 1;
	} else {
		if (obj_exts_cache->object_size == s->object_size)
			return s->object_size + 1;
	}

Quoted 7 lines

Quoted 7 lines

Suren's input would be also nice to have.

Thanks!

[1] https://lore.kernel.org/oe-lkp/[email protected]

Quoted 35 lines · Shakeel Butt

Quoted 34 lines · Shakeel Butt

On Fri, Jun 26, 2026 at 07:11:33PM +0200, Vlastimil Babka (SUSE) wrote:
[...]

Quoted 31 lines

Quoted 11 lines

Quoted 11 lines

Quoted 5 lines

Fix it structurally by removing cycles of every shape: serve the array from a cache strictly larger than the one it describes whenever it would otherwise come from the same or a smaller cache. Every reference edge then points from a smaller to a larger cache (here kmalloc-1k's array moves to kmalloc-2k), so the relation is a DAG and cannot contain a cycle.

This will fix the problem.

But this will waste memory as we need smaller obj_exts array as the size gets larger.

Quoted 8 lines

Quoted 4 lines

We should probably create a new kmalloc type to avoid cycles instead? (needed only when memory profiling is enabled, though)

That would also prevent recursion even further.

Yes but I assume that would add kmem caches even for users not using memory profiling. Anyways, I think that is a separate discussion. Am I understanding correctly that you don't have any concerns with this approach?

Umm, the memory waste is a concern?

Minimally I'd now want to only do that size bumping when allocation profiling is enabled. Ideally that means both configured in and not booted with "never".

We probably should have done that already in 280ea9c3154b2. Because AFAIU memcg-only obj_exts array don't have this issue (or maybe they do have the
[1] issue? Harry?). But if memcg-only should keep avoiding the same size bucket, it can keep what it was doing and only memalloc profiling would do the strictly larger thing.

Quoted 4 lines

Quoted 2 lines

memcg should not have this issue as normal kmalloc caches do not serve memcg charged objects.

I am wrong here as I went back and see d8df600b67d7.

(8dafa9f5900c upstream)

Quoted 3 lines

Quoted 3 lines

So here we can do dedicated caches as Harry suggested or make this size bumping very specialized as Vlastimil suggested. What do we want long term? Orthogonally

Maybe long term we make kmem_buckets unconditional and use that.

Quoted 13 lines

Quoted 3 lines

we do want this fix to be backported easily to older stable kernels. I will see how does this narrowed down size bumping looks like.

BTW I think we need something like the following, right?

	if (mem_alloc_profiling_enabled()) {
		if (obj_exts_cache->object_size <= s->object_size)
			return s->object_size + 1;
	} else {
		if (obj_exts_cache->object_size == s->object_size)
			return s->object_size + 1;
	}

Quoted 36 lines · Vlastimil Babka (SUSE)

Quoted 35 lines · Shakeel Butt

Quoted 34 lines · Shakeel Butt

On Fri, Jun 26, 2026 at 07:11:33PM +0200, Vlastimil Babka (SUSE) wrote:
[...]

Quoted 31 lines

Quoted 11 lines

Quoted 11 lines

Quoted 5 lines

Fix it structurally by removing cycles of every shape: serve the array from a cache strictly larger than the one it describes whenever it would otherwise come from the same or a smaller cache. Every reference edge then points from a smaller to a larger cache (here kmalloc-1k's array moves to kmalloc-2k), so the relation is a DAG and cannot contain a cycle.

This will fix the problem.

But this will waste memory as we need smaller obj_exts array as the size gets larger.

Quoted 8 lines

Quoted 4 lines

We should probably create a new kmalloc type to avoid cycles instead? (needed only when memory profiling is enabled, though)

That would also prevent recursion even further.

Yes but I assume that would add kmem caches even for users not using memory profiling. Anyways, I think that is a separate discussion. Am I understanding correctly that you don't have any concerns with this approach?

Umm, the memory waste is a concern?

Minimally I'd now want to only do that size bumping when allocation profiling is enabled. Ideally that means both configured in and not booted with "never".

We probably should have done that already in 280ea9c3154b2. Because AFAIU memcg-only obj_exts array don't have this issue (or maybe they do have the
[1] issue? Harry?). But if memcg-only should keep avoiding the same size bucket, it can keep what it was doing and only memalloc profiling would do the strictly larger thing.

Quoted 4 lines

Quoted 4 lines

Quoted 2 lines

memcg should not have this issue as normal kmalloc caches do not serve memcg charged objects.

I am wrong here as I went back and see d8df600b67d7.

I was confused too :)

Quoted 21 lines

(8dafa9f5900c upstream)

Quoted 3 lines

Quoted 3 lines

So here we can do dedicated caches as Harry suggested or make this size bumping very specialized as Vlastimil suggested. What do we want long term? Orthogonally

Maybe long term we make kmem_buckets unconditional and use that.

Quoted 13 lines

Quoted 3 lines

we do want this fix to be backported easily to older stable kernels. I will see how does this narrowed down size bumping looks like.

BTW I think we need something like the following, right?

     if (mem_alloc_profiling_enabled()) {
             if (obj_exts_cache->object_size <= s->object_size)
                     return s->object_size + 1;
     } else {
             if (obj_exts_cache->object_size == s->object_size)
                     return s->object_size + 1;
     }

We should not add mem_alloc_profiling_enabled() check because, then we're not fixing this issue on SLUB_TINY, when the caller specifies __GFP_RECLAIMABLE|__GFP_ACCOUNT without memory allocation profiling.

if (!is_kmalloc_normal(s)) check already bails out when it doesn't need to bump the size.

So Shakeel's original code will work fine.

We're only pessimizing memory allocation profiling and SLUB_TINY && MEMCG users, but (as Vlastimil suggests off-list) it wouldn't make much sense to enable MEMCG on memory restricted systems anyway. (IIRC even raspberry pis don't enable the memory controller by default...)

I think it's okay to fix the bug first, but we need to address the memory wastage issue sooner or later if companies (Meta and Google I guess?) are deploying kernels with memory allocation profiling on in production systems.

Perhaps it's worth adding a comment like this, though:

/*
* Only bump the size when the object (not the obj_exts array) is
* allocated from KMALLOC_NORMAL, either by memory allocation profiling
* or memcg on SLUB_TINY with __GFP_RECLAIMABLE|__GFP_ACCOUNT.
* Otherwise, obj_exts allocations cannot form a cycle between
* kmalloc caches.
*/
if (!is_kmalloc_normal(s))

        return sz;

Thanks!

--
Cheers,
Harry / Hyeonggon

Quoted 81 lines · Harry Yoo

Quoted 36 lines · Vlastimil Babka (SUSE)

Quoted 35 lines · Shakeel Butt

Quoted 34 lines · Shakeel Butt

On Fri, Jun 26, 2026 at 07:11:33PM +0200, Vlastimil Babka (SUSE) wrote:
[...]

Quoted 31 lines

Quoted 11 lines

Quoted 11 lines

Quoted 5 lines

Fix it structurally by removing cycles of every shape: serve the array from a cache strictly larger than the one it describes whenever it would otherwise come from the same or a smaller cache. Every reference edge then points from a smaller to a larger cache (here kmalloc-1k's array moves to kmalloc-2k), so the relation is a DAG and cannot contain a cycle.

This will fix the problem.

But this will waste memory as we need smaller obj_exts array as the size gets larger.

Quoted 8 lines

Quoted 4 lines

We should probably create a new kmalloc type to avoid cycles instead? (needed only when memory profiling is enabled, though)

That would also prevent recursion even further.

Yes but I assume that would add kmem caches even for users not using memory profiling. Anyways, I think that is a separate discussion. Am I understanding correctly that you don't have any concerns with this approach?

Umm, the memory waste is a concern?

Minimally I'd now want to only do that size bumping when allocation profiling is enabled. Ideally that means both configured in and not booted with "never".

We probably should have done that already in 280ea9c3154b2. Because AFAIU memcg-only obj_exts array don't have this issue (or maybe they do have the
[1] issue? Harry?). But if memcg-only should keep avoiding the same size bucket, it can keep what it was doing and only memalloc profiling would do the strictly larger thing.

Quoted 4 lines

Quoted 4 lines

Quoted 2 lines

memcg should not have this issue as normal kmalloc caches do not serve memcg charged objects.

I am wrong here as I went back and see d8df600b67d7.

I was confused too :)

Quoted 21 lines

(8dafa9f5900c upstream)

Quoted 3 lines

Quoted 3 lines

So here we can do dedicated caches as Harry suggested or make this size bumping very specialized as Vlastimil suggested. What do we want long term? Orthogonally

Maybe long term we make kmem_buckets unconditional and use that.

Quoted 13 lines

Quoted 3 lines

we do want this fix to be backported easily to older stable kernels. I will see how does this narrowed down size bumping looks like.

BTW I think we need something like the following, right?

     if (mem_alloc_profiling_enabled()) {
             if (obj_exts_cache->object_size <= s->object_size)
                     return s->object_size + 1;
     } else {
             if (obj_exts_cache->object_size == s->object_size)
                     return s->object_size + 1;
     }

We should not add mem_alloc_profiling_enabled() check because, then we're not fixing this issue on SLUB_TINY, when the caller specifies __GFP_RECLAIMABLE|__GFP_ACCOUNT without memory allocation profiling.

if (!is_kmalloc_normal(s)) check already bails out when it doesn't need to bump the size.

So Shakeel's original code will work fine.

We're only pessimizing memory allocation profiling and SLUB_TINY && MEMCG users, but (as Vlastimil suggests off-list) it wouldn't make much sense to enable MEMCG on memory restricted systems anyway. (IIRC even raspberry pis don't enable the memory controller by default...)

Quoted 4 lines

I think it's okay to fix the bug first, but we need to address the memory wastage issue sooner or later if companies (Meta and Google I guess?) are deploying kernels with memory allocation profiling on in production systems.

Sorry for the delay folks. I just got a chance to read through this thread.

I think adding a new KMALLOC_TYPE would be the cleanest way to fix this recursion problem once and for all. This size bumping and the special case of SLUB_TINY are quite confusing.

We could define that> new KMALLOC_TYPE only if memory allocation profiling or SLUB_TINY are enabled to avoid new caches when not needed. Does not seem too complex
but maybe I'm missing something? WDYT?

If it is more complex than I imaging then I'm fine with Shakeel's approach as a temporary fix.

[ Adding Kees Cook for SLAB_BUCKETS conversation ]

The thread:
https://lore.kernel.org/linux-mm/[email protected]/

Quoted 82 lines · Suren Baghdasaryan

Quoted 81 lines · Harry Yoo

Quoted 36 lines · Vlastimil Babka (SUSE)

Quoted 35 lines · Shakeel Butt

Quoted 34 lines · Shakeel Butt

On Fri, Jun 26, 2026 at 07:11:33PM +0200, Vlastimil Babka (SUSE) wrote:
[...]

Quoted 31 lines

Quoted 11 lines

Quoted 11 lines

Quoted 5 lines

Fix it structurally by removing cycles of every shape: serve the array from a cache strictly larger than the one it describes whenever it would otherwise come from the same or a smaller cache. Every reference edge then points from a smaller to a larger cache (here kmalloc-1k's array moves to kmalloc-2k), so the relation is a DAG and cannot contain a cycle.

This will fix the problem.

But this will waste memory as we need smaller obj_exts array as the size gets larger.

Quoted 8 lines

Quoted 4 lines

We should probably create a new kmalloc type to avoid cycles instead? (needed only when memory profiling is enabled, though)

That would also prevent recursion even further.

Yes but I assume that would add kmem caches even for users not using memory profiling. Anyways, I think that is a separate discussion. Am I understanding correctly that you don't have any concerns with this approach?

Umm, the memory waste is a concern?

Minimally I'd now want to only do that size bumping when allocation profiling is enabled. Ideally that means both configured in and not booted with "never".

We probably should have done that already in 280ea9c3154b2. Because AFAIU memcg-only obj_exts array don't have this issue (or maybe they do have the
[1] issue? Harry?). But if memcg-only should keep avoiding the same size bucket, it can keep what it was doing and only memalloc profiling would do the strictly larger thing.

Quoted 4 lines

Quoted 4 lines

Quoted 2 lines

memcg should not have this issue as normal kmalloc caches do not serve memcg charged objects.

I am wrong here as I went back and see d8df600b67d7.

I was confused too :)

Quoted 21 lines

(8dafa9f5900c upstream)

Quoted 3 lines

Quoted 3 lines

So here we can do dedicated caches as Harry suggested or make this size bumping very specialized as Vlastimil suggested. What do we want long term? Orthogonally

Maybe long term we make kmem_buckets unconditional and use that.

Quoted 13 lines

Quoted 3 lines

we do want this fix to be backported easily to older stable kernels. I will see how does this narrowed down size bumping looks like.

BTW I think we need something like the following, right?

     if (mem_alloc_profiling_enabled()) {
             if (obj_exts_cache->object_size <= s->object_size)
                     return s->object_size + 1;
     } else {
             if (obj_exts_cache->object_size == s->object_size)
                     return s->object_size + 1;
     }

We should not add mem_alloc_profiling_enabled() check because, then we're not fixing this issue on SLUB_TINY, when the caller specifies __GFP_RECLAIMABLE|__GFP_ACCOUNT without memory allocation profiling.

if (!is_kmalloc_normal(s)) check already bails out when it doesn't need to bump the size.

So Shakeel's original code will work fine.

We're only pessimizing memory allocation profiling and SLUB_TINY && MEMCG users, but (as Vlastimil suggests off-list) it wouldn't make much sense to enable MEMCG on memory restricted systems anyway. (IIRC even raspberry pis don't enable the memory controller by default...)

Quoted 6 lines

Quoted 4 lines

I think it's okay to fix the bug first, but we need to address the memory wastage issue sooner or later if companies (Meta and Google I guess?) are deploying kernels with memory allocation profiling on in production systems.

Sorry for the delay folks. I just got a chance to read through this thread.

Hi Suren, no worries!

Quoted 3 lines

I think adding a new KMALLOC_TYPE would be the cleanest way to fix this recursion problem once and for all. This size bumping and the special case of SLUB_TINY are quite confusing.

As mentioned by Vlsatimil, in the long term, using SLAB_BUCKETS infrastructure would be more straightforward than new KMALLOC_TYPE because (I think) the kmalloc type is decided purely based on GFP flags and we need to somehow work around that. SLAB_BUCKETS provides a nice abstraction to do this.

Luckily, SLAB_BUCKETS is introduced in v6.11.
Unfortunately, SLAB_BUCKETS is optional.

Quoted 3 lines

We could define that> new KMALLOC_TYPE only if memory allocation profiling or SLUB_TINY are enabled to avoid new caches when not needed. Does not seem too complex
but maybe I'm missing something? WDYT?

I think we need some enhancements to achieve that with SLAB_BUCKETS

1. Rename SLAB_BUCKETS to SLAB_BUCKETS_HARDENING
(w/ SLAB_BUCKETS being a transitional config for _HARDENING)

2. Make the SLAB_BUCKETS infrastructure unconditional, but the decision is made at runtime:

1) actually creating a kmem_buckets vs.
2) falling back to kmalloc.

3. kmem_buckets_create() creates kmem_buckets only when SLAB_BUCKETS_HARDENING is enabled.

4. SLUB decides (not) to create kmem_buckets for internal use during the boot process. Use the kmem_buckets for obj_exts array allocation.

Side note: this would unconditionally add the kmem_buckets parameter to the kmalloc slowpath. Probably it'd be worth introducing a dedicated entrypoint for kmem_buckets instead.

Quoted 3 lines

If it is more complex than I imaging then I'm fine with Shakeel's approach as a temporary fix.

Since above requires quite some changes, I'd say let's proeed with the fix (since it's one line of code change that fixes a bug), and then see how we can make SLAB_BUCKETS changes as minimal as possible for backporting?

--
Cheers,
Harry / Hyeonggon

Quoted 2 lines · Harry Yoo

Quoted 3 lines · Suren Baghdasaryan

Quoted 2 lines · Harry Yoo

[...]

Thanks all for great discussion. Let me summarize the conclusion and please correct me if I missed something.

Let's keep the original one-line fix (serve the obj_exts array from a strictly larger cache, making the relation a DAG).

We will NOT gate it on> mem_alloc_profiling_enabled() as I floated earlier

-- per Harry, is_kmalloc_normal(s) is already the right condition,

and gating on> profiling would miss the SLUB_TINY + __GFP_RECLAIMABLE|__GFP_ACCOUNT memcg case.

So the bump stays unconditional for is_kmalloc_normal() caches.

This over-allocates the array for larger caches, but only for profiling and SLUB_TINY+MEMCG users (the latter unrealistic). Acceptable for a small, backportable fix.

Quoted 35 lines · Harry Yoo

Quoted 3 lines

I think adding a new KMALLOC_TYPE would be the cleanest way to fix this recursion problem once and for all. This size bumping and the special case of SLUB_TINY are quite confusing.

As mentioned by Vlsatimil, in the long term, using SLAB_BUCKETS infrastructure would be more straightforward than new KMALLOC_TYPE because (I think) the kmalloc type is decided purely based on GFP flags and we need to somehow work around that. SLAB_BUCKETS provides a nice abstraction to do this.

Luckily, SLAB_BUCKETS is introduced in v6.11.
Unfortunately, SLAB_BUCKETS is optional.

Quoted 3 lines

We could define that> new KMALLOC_TYPE only if memory allocation profiling or SLUB_TINY are enabled to avoid new caches when not needed. Does not seem too complex
but maybe I'm missing something? WDYT?

I think we need some enhancements to achieve that with SLAB_BUCKETS

1. Rename SLAB_BUCKETS to SLAB_BUCKETS_HARDENING
(w/ SLAB_BUCKETS being a transitional config for _HARDENING)

2. Make the SLAB_BUCKETS infrastructure unconditional, but the decision is made at runtime:

1) actually creating a kmem_buckets vs.
2) falling back to kmalloc.

3. kmem_buckets_create() creates kmem_buckets only when SLAB_BUCKETS_HARDENING is enabled.

4. SLUB decides (not) to create kmem_buckets for internal use during the boot process. Use the kmem_buckets for obj_exts array allocation.

Quoted 3 lines

Side note: this would unconditionally add the kmem_buckets parameter to the kmalloc slowpath. Probably it'd be worth introducing a dedicated entrypoint for kmem_buckets instead.

Yeah, this sounds quite complex.

Maybe we could use the new> kmalloc_flags() introduced by Vlastimil in [1] to avoid using GFP
flags to indicate that we want to use this new KMALLOC_TYPE? That seems simpler,

though it's not backportable because kmalloc_flags() is> brand new.

[1] https://lore.kernel.org/all/[email protected]/

Quoted 4 lines

Quoted 3 lines

If it is more complex than I imaging then I'm fine with Shakeel's approach as a temporary fix.

Quoted 4 lines

Since above requires quite some changes, I'd say let's proeed with the fix (since it's one line of code change that fixes a bug), and then see how we can make SLAB_BUCKETS changes as minimal as possible for backporting?

I was thinking Shakeel's approach for backports and kmalloc_flags()+KMALLOC_TYPE going forward.

Just throwing this as an> option. I haven't looked closely into SLAB_BUCKETS yet, so that might
be indeed a better direction.

Quoted 36 lines · Suren Baghdasaryan

Quoted 35 lines · Harry Yoo

Quoted 3 lines

I think adding a new KMALLOC_TYPE would be the cleanest way to fix this recursion problem once and for all. This size bumping and the special case of SLUB_TINY are quite confusing.

As mentioned by Vlsatimil, in the long term, using SLAB_BUCKETS infrastructure would be more straightforward than new KMALLOC_TYPE because (I think) the kmalloc type is decided purely based on GFP flags and we need to somehow work around that. SLAB_BUCKETS provides a nice abstraction to do this.

Luckily, SLAB_BUCKETS is introduced in v6.11.
Unfortunately, SLAB_BUCKETS is optional.

Quoted 3 lines

We could define that> new KMALLOC_TYPE only if memory allocation profiling or SLUB_TINY are enabled to avoid new caches when not needed. Does not seem too complex
but maybe I'm missing something? WDYT?

I think we need some enhancements to achieve that with SLAB_BUCKETS

1. Rename SLAB_BUCKETS to SLAB_BUCKETS_HARDENING
(w/ SLAB_BUCKETS being a transitional config for _HARDENING)

2. Make the SLAB_BUCKETS infrastructure unconditional, but the decision is made at runtime:

1) actually creating a kmem_buckets vs.
2) falling back to kmalloc.

3. kmem_buckets_create() creates kmem_buckets only when SLAB_BUCKETS_HARDENING is enabled.

4. SLUB decides (not) to create kmem_buckets for internal use during the boot process. Use the kmem_buckets for obj_exts array allocation.

Quoted 5 lines

Quoted 3 lines

Side note: this would unconditionally add the kmem_buckets parameter to the kmalloc slowpath. Probably it'd be worth introducing a dedicated entrypoint for kmem_buckets instead.

Yeah, this sounds quite complex.

I think it's not that complex, but quite some churn, yeah :)

Quoted 4 lines

Maybe we could use the new> kmalloc_flags() introduced by Vlastimil in [1] to avoid using GFP
flags to indicate that we want to use this new KMALLOC_TYPE? That seems simpler,

That indeed would be smaller changes.

Quoted 1 line

though it's not backportable because kmalloc_flags() is> brand new.

Right, I didn't seriously consider that option as I was (mistakenly) assuming you or Shakeel would want to backport it.

Quoted 11 lines

[1] https://lore.kernel.org/all/[email protected]/

Quoted 8 lines

Quoted 3 lines

If it is more complex than I imaging then I'm fine with Shakeel's approach as a temporary fix.

Since above requires quite some changes, I'd say let's proeed with the fix (since it's one line of code change that fixes a bug), and then see how we can make SLAB_BUCKETS changes as minimal as possible for backporting?

Quoted 2 lines

I was thinking Shakeel's approach for backports and kmalloc_flags()+KMALLOC_TYPE going forward.

Oh, I misread it then.
I was assuming it's critical enough to bother backporting.

Quoted 4 lines

Just throwing this as an> option. I haven't looked closely into SLAB_BUCKETS yet, so that might
be indeed a better direction.

--
Cheers,
Harry / Hyeonggon

Quoted 74 lines · Harry Yoo

Quoted 36 lines · Suren Baghdasaryan

Quoted 35 lines · Harry Yoo

Quoted 3 lines

I think adding a new KMALLOC_TYPE would be the cleanest way to fix this recursion problem once and for all. This size bumping and the special case of SLUB_TINY are quite confusing.

As mentioned by Vlsatimil, in the long term, using SLAB_BUCKETS infrastructure would be more straightforward than new KMALLOC_TYPE because (I think) the kmalloc type is decided purely based on GFP flags and we need to somehow work around that. SLAB_BUCKETS provides a nice abstraction to do this.

Luckily, SLAB_BUCKETS is introduced in v6.11.
Unfortunately, SLAB_BUCKETS is optional.

Quoted 3 lines

We could define that> new KMALLOC_TYPE only if memory allocation profiling or SLUB_TINY are enabled to avoid new caches when not needed. Does not seem too complex
but maybe I'm missing something? WDYT?

I think we need some enhancements to achieve that with SLAB_BUCKETS

1. Rename SLAB_BUCKETS to SLAB_BUCKETS_HARDENING
(w/ SLAB_BUCKETS being a transitional config for _HARDENING)

2. Make the SLAB_BUCKETS infrastructure unconditional, but the decision is made at runtime:

1) actually creating a kmem_buckets vs.
2) falling back to kmalloc.

3. kmem_buckets_create() creates kmem_buckets only when SLAB_BUCKETS_HARDENING is enabled.

4. SLUB decides (not) to create kmem_buckets for internal use during the boot process. Use the kmem_buckets for obj_exts array allocation.

Quoted 5 lines

Quoted 3 lines

Side note: this would unconditionally add the kmem_buckets parameter to the kmalloc slowpath. Probably it'd be worth introducing a dedicated entrypoint for kmem_buckets instead.

Yeah, this sounds quite complex.

I think it's not that complex, but quite some churn, yeah :)

Quoted 4 lines

Maybe we could use the new> kmalloc_flags() introduced by Vlastimil in [1] to avoid using GFP
flags to indicate that we want to use this new KMALLOC_TYPE? That seems simpler,

That indeed would be smaller changes.

Quoted 1 line

though it's not backportable because kmalloc_flags() is> brand new.

Right, I didn't seriously consider that option as I was (mistakenly) assuming you or Shakeel would want to backport it.

Quoted 13 lines

[1] https://lore.kernel.org/all/[email protected]/

Quoted 8 lines

Quoted 3 lines

If it is more complex than I imaging then I'm fine with Shakeel's approach as a temporary fix.

Since above requires quite some changes, I'd say let's proeed with the fix (since it's one line of code change that fixes a bug), and then see how we can make SLAB_BUCKETS changes as minimal as possible for backporting?

I was thinking Shakeel's approach for backports and kmalloc_flags()+KMALLOC_TYPE going forward.

Quoted 2 lines

Oh, I misread it then.
I was assuming it's critical enough to bother backporting.

Yes, it's worth backporting, so we can merge Shakeel's change as is and then once Vlastimil's patch is merged we can implement the new KMALLOC_TYPE as a replacement.

Quoted 9 lines

Quoted 5 lines

Just throwing this as an> option. I haven't looked closely into SLAB_BUCKETS yet, so that might
be indeed a better direction.

--
Cheers,
Harry / Hyeonggon

Quoted 73 lines · Suren Baghdasaryan

Quoted 71 lines · Harry Yoo

Quoted 36 lines · Suren Baghdasaryan

Quoted 35 lines · Harry Yoo

Quoted 3 lines

I think adding a new KMALLOC_TYPE would be the cleanest way to fix this recursion problem once and for all. This size bumping and the special case of SLUB_TINY are quite confusing.

As mentioned by Vlsatimil, in the long term, using SLAB_BUCKETS infrastructure would be more straightforward than new KMALLOC_TYPE because (I think) the kmalloc type is decided purely based on GFP flags and we need to somehow work around that. SLAB_BUCKETS provides a nice abstraction to do this.

Luckily, SLAB_BUCKETS is introduced in v6.11.
Unfortunately, SLAB_BUCKETS is optional.

Quoted 3 lines

We could define that> new KMALLOC_TYPE only if memory allocation profiling or SLUB_TINY are enabled to avoid new caches when not needed. Does not seem too complex
but maybe I'm missing something? WDYT?

I think we need some enhancements to achieve that with SLAB_BUCKETS

1. Rename SLAB_BUCKETS to SLAB_BUCKETS_HARDENING
(w/ SLAB_BUCKETS being a transitional config for _HARDENING)

2. Make the SLAB_BUCKETS infrastructure unconditional, but the decision is made at runtime:

1) actually creating a kmem_buckets vs.
2) falling back to kmalloc.

3. kmem_buckets_create() creates kmem_buckets only when SLAB_BUCKETS_HARDENING is enabled.

4. SLUB decides (not) to create kmem_buckets for internal use during the boot process. Use the kmem_buckets for obj_exts array allocation.

Quoted 5 lines

Quoted 3 lines

Side note: this would unconditionally add the kmem_buckets parameter to the kmalloc slowpath. Probably it'd be worth introducing a dedicated entrypoint for kmem_buckets instead.

Yeah, this sounds quite complex.

I think it's not that complex, but quite some churn, yeah :)

Quoted 4 lines

Maybe we could use the new> kmalloc_flags() introduced by Vlastimil in [1] to avoid using GFP
flags to indicate that we want to use this new KMALLOC_TYPE? That seems simpler,

That indeed would be smaller changes.

Quoted 1 line

though it's not backportable because kmalloc_flags() is> brand new.

Right, I didn't seriously consider that option as I was (mistakenly) assuming you or Shakeel would want to backport it.

Quoted 11 lines

[1] https://lore.kernel.org/all/[email protected]/

Quoted 8 lines

Quoted 3 lines

If it is more complex than I imaging then I'm fine with Shakeel's approach as a temporary fix.

Since above requires quite some changes, I'd say let's proeed with the fix (since it's one line of code change that fixes a bug), and then see how we can make SLAB_BUCKETS changes as minimal as possible for backporting?

Quoted 5 lines

Quoted 5 lines

Quoted 2 lines

I was thinking Shakeel's approach for backports and kmalloc_flags()+KMALLOC_TYPE going forward.

Oh, I misread it then.
I was assuming it's critical enough to bother backporting.

Quoted 1 line

Yes, it's worth backporting, so we can merge Shakeel's change as is

Quoted 1 line

and then once Vlastimil's patch is merged we can implement the new

Quoted 1 line

KMALLOC_TYPE as a replacement.

And Shakeel's patch is easily backportable.

Quoted 74 lines · Suren Baghdasaryan

Quoted 73 lines · Suren Baghdasaryan

Quoted 71 lines · Harry Yoo

Quoted 36 lines · Suren Baghdasaryan

Quoted 35 lines · Harry Yoo

Quoted 3 lines

I think adding a new KMALLOC_TYPE would be the cleanest way to fix this recursion problem once and for all. This size bumping and the special case of SLUB_TINY are quite confusing.

As mentioned by Vlsatimil, in the long term, using SLAB_BUCKETS infrastructure would be more straightforward than new KMALLOC_TYPE because (I think) the kmalloc type is decided purely based on GFP flags and we need to somehow work around that. SLAB_BUCKETS provides a nice abstraction to do this.

Luckily, SLAB_BUCKETS is introduced in v6.11.
Unfortunately, SLAB_BUCKETS is optional.

Quoted 3 lines

We could define that> new KMALLOC_TYPE only if memory allocation profiling or SLUB_TINY are enabled to avoid new caches when not needed. Does not seem too complex
but maybe I'm missing something? WDYT?

I think we need some enhancements to achieve that with SLAB_BUCKETS

1. Rename SLAB_BUCKETS to SLAB_BUCKETS_HARDENING
(w/ SLAB_BUCKETS being a transitional config for _HARDENING)

2. Make the SLAB_BUCKETS infrastructure unconditional, but the decision is made at runtime:

1) actually creating a kmem_buckets vs.
2) falling back to kmalloc.

3. kmem_buckets_create() creates kmem_buckets only when SLAB_BUCKETS_HARDENING is enabled.

4. SLUB decides (not) to create kmem_buckets for internal use during the boot process. Use the kmem_buckets for obj_exts array allocation.

Quoted 5 lines

Quoted 3 lines

Side note: this would unconditionally add the kmem_buckets parameter to the kmalloc slowpath. Probably it'd be worth introducing a dedicated entrypoint for kmem_buckets instead.

Yeah, this sounds quite complex.

I think it's not that complex, but quite some churn, yeah :)

Quoted 4 lines

Maybe we could use the new> kmalloc_flags() introduced by Vlastimil in [1] to avoid using GFP
flags to indicate that we want to use this new KMALLOC_TYPE? That seems simpler,

That indeed would be smaller changes.

Quoted 1 line

though it's not backportable because kmalloc_flags() is> brand new.

Right, I didn't seriously consider that option as I was (mistakenly) assuming you or Shakeel would want to backport it.

Quoted 11 lines

[1] https://lore.kernel.org/all/[email protected]/

Quoted 8 lines

Quoted 3 lines

If it is more complex than I imaging then I'm fine with Shakeel's approach as a temporary fix.

Since above requires quite some changes, I'd say let's proeed with the fix (since it's one line of code change that fixes a bug), and then see how we can make SLAB_BUCKETS changes as minimal as possible for backporting?

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

Quoted 2 lines

I was thinking Shakeel's approach for backports and kmalloc_flags()+KMALLOC_TYPE going forward.

Oh, I misread it then.
I was assuming it's critical enough to bother backporting.

Ah, here I meant backporting either the kmalloc_flags()+KMALLOC_TYPE or SLAB_BUCKETS approach.

Quoted 1 line

Quoted 1 line

Yes, it's worth backporting, so we can merge Shakeel's change as is

Right.

Quoted 1 line

Quoted 1 line

and then once Vlastimil's patch is merged we can implement the new

Vlastimil's patch has already landed mainline, by the way :)

Quoted 4 lines

Quoted 2 lines

KMALLOC_TYPE as a replacement.

And Shakeel's patch is easily backportable.

Yes, of course!

--
Cheers,
Harry / Hyeonggon

Quoted 14 lines · Harry Yoo

Quoted 2 lines · Suren Baghdasaryan

Quoted 1 line · Suren Baghdasaryan

Ah, here I meant backporting either the kmalloc_flags()+KMALLOC_TYPE or SLAB_BUCKETS approach.

Quoted 1 line

Quoted 1 line

Yes, it's worth backporting, so we can merge Shakeel's change as is

Right.

Quoted 3 lines

Quoted 1 line

Quoted 1 line

and then once Vlastimil's patch is merged we can implement the new

Vlastimil's patch has already landed mainline, by the way :)

Nice! I suggest posting Shakeel's patch CC'ing stable for backports and then following up with the fix using KMALLOC_TYPE. Vlastimil, WDYT?

Quoted 11 lines

Quoted 4 lines

Quoted 2 lines

KMALLOC_TYPE as a replacement.

And Shakeel's patch is easily backportable.

Yes, of course!

--
Cheers,
Harry / Hyeonggon

Quoted 19 lines · Suren Baghdasaryan

Quoted 17 lines · Harry Yoo

Quoted 2 lines · Suren Baghdasaryan

Quoted 1 line · Suren Baghdasaryan

Ah, here I meant backporting either the kmalloc_flags()+KMALLOC_TYPE or SLAB_BUCKETS approach.

Quoted 1 line

Quoted 1 line

Yes, it's worth backporting, so we can merge Shakeel's change as is

Right.

Quoted 1 line

Quoted 1 line

and then once Vlastimil's patch is merged we can implement the new

Vlastimil's patch has already landed mainline, by the way :)

Quoted 3 lines

Nice! I suggest posting Shakeel's patch CC'ing stable for backports and then following up with the fix using KMALLOC_TYPE. Vlastimil, WDYT?

Sounded like a plan, but then I realized I misunderstood the amount of the wastage. E.g. on my system kmalloc-8k with 4 objects per slab would have obj_ext size of 64, but now it's 16k? That's ridiculous.

I think it will> even self-amplify to some extent? kmalloc-8 would

so its obj_ext is 8k. It will not recursively create an obj_ext for the> obj_ext, but other 8k allocation in the same kmalloc-8k slab could then trigger it, right?

We could say it's for a debugging feature, but also it's running in production fleets (and Android?), so probably not that easy to dismiss.

Sudden memory increase in a LTS due to this backport doesn't sound nice to me.

Quoted 24 lines · Vlastimil Babka (SUSE)

Quoted 22 lines · Suren Baghdasaryan

Quoted 10 lines · Harry Yoo

Quoted 2 lines · Suren Baghdasaryan

Quoted 1 line · Suren Baghdasaryan

Ah, here I meant backporting either the kmalloc_flags()+KMALLOC_TYPE or SLAB_BUCKETS approach.

Quoted 7 lines

Quoted 1 line

Quoted 1 line

Yes, it's worth backporting, so we can merge Shakeel's change as is

Right.

Quoted 1 line

Quoted 1 line

and then once Vlastimil's patch is merged we can implement the new

Vlastimil's patch has already landed mainline, by the way :)

Nice! I suggest posting Shakeel's patch CC'ing stable for backports and then following up with the fix using KMALLOC_TYPE. Vlastimil, WDYT?

Quoted 3 lines

Sounded like a plan, but then I realized I misunderstood the amount of the wastage. E.g. on my system kmalloc-8k with 4 objects per slab would have obj_ext size of 64, but now it's 16k? That's ridiculous.

Right.

...which is why I was assuming either the KMALLOC_TYPE or SLAB_BUCKETS approach would be backported as a follow-up. Err, should have communicated clearly, apologies.

Quoted 2 lines

I think it will> even self-amplify to some extent? kmalloc-8 would

have 512 objects per slab,

Quoted 2 lines

so its obj_ext is 8k. It will not recursively create an obj_ext for the> obj_ext, but other 8k allocation in the same kmalloc-8k slab could then trigger it, right?

True, assuming that by 'self-amplifying' you meant this patch creates more kmalloc-8k objects, and also now kmalloc-8k wastes memory memory.

Quoted 2 lines

We could say it's for a debugging feature, but also it's running in production fleets (and Android?), so probably not that easy to dismiss.

I think a key factor is when it's enabled in production.

kconfigs says Android selects MEM_ALLOC_PROFILING, but not MEM_ALLOC_PROFILING_ENABLED_BY_DEFAULT.

I assumed that turning it on by default in the entire fleet would be bit hard to justify... (please correct me, if it's not the case)

Quoted 27 lines · Harry Yoo

Quoted 24 lines · Vlastimil Babka (SUSE)

Quoted 22 lines · Suren Baghdasaryan

Quoted 10 lines · Harry Yoo

Quoted 2 lines · Suren Baghdasaryan

Quoted 1 line · Suren Baghdasaryan

Ah, here I meant backporting either the kmalloc_flags()+KMALLOC_TYPE or SLAB_BUCKETS approach.

Quoted 7 lines

Quoted 1 line

Quoted 1 line

Yes, it's worth backporting, so we can merge Shakeel's change as is

Right.

Quoted 1 line

Quoted 1 line

and then once Vlastimil's patch is merged we can implement the new

Vlastimil's patch has already landed mainline, by the way :)

Nice! I suggest posting Shakeel's patch CC'ing stable for backports and then following up with the fix using KMALLOC_TYPE. Vlastimil, WDYT?

Quoted 5 lines

Quoted 3 lines

Sounded like a plan, but then I realized I misunderstood the amount of the wastage. E.g. on my system kmalloc-8k with 4 objects per slab would have obj_ext size of 64, but now it's 16k? That's ridiculous.

Right.

Yeah I should have given more thought on wastage.

Quoted 5 lines

...which is why I was assuming either the KMALLOC_TYPE or SLAB_BUCKETS approach would be backported as a follow-up. Err, should have communicated clearly, apologies.

Harry, do you want to take a stab at prototyping these? If these look simple enough, we can request backports of this.

Quoted 10 lines

Quoted 3 lines

I think it will> even self-amplify to some extent? kmalloc-8 would

have 512 objects per slab,

Quoted 2 lines

so its obj_ext is 8k. It will not recursively create an obj_ext for the> obj_ext, but other 8k allocation in the same kmalloc-8k slab could then trigger it, right?

True, assuming that by 'self-amplifying' you meant this patch creates more kmalloc-8k objects, and also now kmalloc-8k wastes memory memory.

I am not sure I understand what self-amplifying means here. Shouldn't 8k allocations served by the same kmalloc-8k slab will share the obj_exts array?

Quoted 11 lines

Quoted 2 lines

We could say it's for a debugging feature, but also it's running in production fleets (and Android?), so probably not that easy to dismiss.

I think a key factor is when it's enabled in production.

kconfigs says Android selects MEM_ALLOC_PROFILING, but not MEM_ALLOC_PROFILING_ENABLED_BY_DEFAULT.

I assumed that turning it on by default in the entire fleet would be bit hard to justify... (please correct me, if it's not the case)

Actually we have memory profiling enabled by default across Meta fleet. So, the issue is very real. At the moment, we are seeing this issue on a specific type of machine and we have disabled memory profiling for those machines.

Internally we did discuss to simply disable memory allocation profiling for kmalloc-normal caches but to me that was a big hammer and thus suggested the current approach.

Quoted 33 lines · Shakeel Butt

Quoted 31 lines · Harry Yoo

Quoted 27 lines · Vlastimil Babka (SUSE)

Quoted 15 lines · Suren Baghdasaryan

Quoted 14 lines · Harry Yoo

Quoted 2 lines · Suren Baghdasaryan

Quoted 1 line · Suren Baghdasaryan

Ah, here I meant backporting either the kmalloc_flags()+KMALLOC_TYPE or SLAB_BUCKETS approach.

Quoted 1 line

Quoted 1 line

Yes, it's worth backporting, so we can merge Shakeel's change as is

Right.

Quoted 7 lines

Quoted 3 lines

Quoted 1 line

Quoted 1 line

and then once Vlastimil's patch is merged we can implement the new

Vlastimil's patch has already landed mainline, by the way :)

Nice! I suggest posting Shakeel's patch CC'ing stable for backports and then following up with the fix using KMALLOC_TYPE. Vlastimil, WDYT?

Sounded like a plan, but then I realized I misunderstood the amount of the wastage. E.g. on my system kmalloc-8k with 4 objects per slab would have obj_ext size of 64, but now it's 16k? That's ridiculous.

Quoted 3 lines

Quoted 1 line

Right.

Yeah I should have given more thought on wastage.

Ugh! I didn't realize the wastage was that high.

Quoted 6 lines

Quoted 3 lines

...which is why I was assuming either the KMALLOC_TYPE or SLAB_BUCKETS approach would be backported as a follow-up. Err, should have communicated clearly, apologies.

Harry, do you want to take a stab at prototyping these? If these look simple enough, we can request backports of this.

I'll also give it some thought to see if there is maybe a different way to fix this that would be easy to backport.

Quoted 8 lines

Quoted 8 lines

Quoted 2 lines

We could say it's for a debugging feature, but also it's running in production fleets (and Android?), so probably not that easy to dismiss.

I think a key factor is when it's enabled in production.

kconfigs says Android selects MEM_ALLOC_PROFILING, but not MEM_ALLOC_PROFILING_ENABLED_BY_DEFAULT.

Quoted 6 lines

Quoted 3 lines

I assumed that turning it on by default in the entire fleet would be bit hard to justify... (please correct me, if it's not the case)

Actually we have memory profiling enabled by default across Meta fleet. So, the issue is very real.

Quoted 5 lines

At the moment, we are seeing this issue on a specific
type of machine and we have disabled memory profiling for those machines. Internally we did discuss to simply disable memory allocation profiling for kmalloc-normal caches but to me that was a big hammer and thus suggested the current approach.

Quoted 34 lines · Suren Baghdasaryan

Quoted 33 lines · Shakeel Butt

Quoted 31 lines · Harry Yoo

Quoted 27 lines · Vlastimil Babka (SUSE)

Quoted 15 lines · Suren Baghdasaryan

Quoted 14 lines · Harry Yoo

Quoted 2 lines · Suren Baghdasaryan

Quoted 1 line · Suren Baghdasaryan

Ah, here I meant backporting either the kmalloc_flags()+KMALLOC_TYPE or SLAB_BUCKETS approach.

Quoted 1 line

Quoted 1 line

Yes, it's worth backporting, so we can merge Shakeel's change as is

Right.

Quoted 7 lines

Quoted 3 lines

Quoted 1 line

Quoted 1 line

and then once Vlastimil's patch is merged we can implement the new

Vlastimil's patch has already landed mainline, by the way :)

Nice! I suggest posting Shakeel's patch CC'ing stable for backports and then following up with the fix using KMALLOC_TYPE. Vlastimil, WDYT?

Sounded like a plan, but then I realized I misunderstood the amount of the wastage. E.g. on my system kmalloc-8k with 4 objects per slab would have obj_ext size of 64, but now it's 16k? That's ridiculous.

Quoted 5 lines

Quoted 3 lines

Quoted 1 line

Right.

Yeah I should have given more thought on wastage.

Ugh! I didn't realize the wastage was that high.

Ouch!

Quoted 6 lines

Quoted 6 lines

Quoted 3 lines

...which is why I was assuming either the KMALLOC_TYPE or SLAB_BUCKETS approach would be backported as a follow-up. Err, should have communicated clearly, apologies.

Harry, do you want to take a stab at prototyping these? If these look simple enough, we can request backports of this.

Ack, let's see what would be the minimal changes to resolve this.

Quoted 2 lines

I'll also give it some thought to see if there is maybe a different way to fix this that would be easy to backport.

Thanks, let's discuss!

Quoted 9 lines

Quoted 9 lines

Quoted 9 lines

Quoted 3 lines

We could say it's for a debugging feature, but also it's running in production fleets (and Android?), so probably not that easy to dismiss.

I think a key factor is when it's enabled in production.

kconfigs says Android selects MEM_ALLOC_PROFILING, but not MEM_ALLOC_PROFILING_ENABLED_BY_DEFAULT.

Quoted 6 lines

Quoted 6 lines

Quoted 3 lines

I assumed that turning it on by default in the entire fleet would be bit hard to justify... (please correct me, if it's not the case)

Actually we have memory profiling enabled by default across Meta fleet. So, the issue is very real.

I see, thanks for clarifying.

Quoted 5 lines

Quoted 5 lines

At the moment, we are seeing this issue on a specific
type of machine and we have disabled memory profiling for those machines. Internally we did discuss to simply disable memory allocation profiling for kmalloc-normal caches but to me that was a big hammer and thus suggested the current approach.

--
Cheers,
Harry / Hyeonggon

Quoted 4 lines · Harry Yoo

Quoted 2 lines

I'll also give it some thought to see if there is maybe a different way to fix this that would be easy to backport.

Thanks, let's discuss!

So, below patch is based on stable linux-7.1.y branch and I think it's the simplest backportable way to fix this issue. Build-tested only, so use just as a prototype.

Not sure if making buckets parameter unconditional from CONFIG_SLAB_BUCKETS has side-effects...

Attaching the patch as well since I'm sure gmail will butcher the tabs.

The fix for 7.2 should be much simpler using a new flag in kmalloc_flags() for obj_ext vector allocations and serving these from a dedicated cache.

Please let me know what you think:

---
 include/linux/slab.h | 42 +++++++++++++++++++++---------
 mm/slub.c            | 61 +++++++++++---------------------------------
 2 files changed, 45 insertions(+), 58 deletions(-)

include/linux/slab.h +0 -0

index 2b5ab488e96b..2d4a6e3064a3 100644

include/linux/slab.h +4 -0

 /*
  * Whenever changing this, take care of that kmalloc_type() and
  * create_kmalloc_caches() still work as intended.

@@ -646,6 +650,9 @@ enum kmalloc_cache_type {
 #endif
 #ifdef CONFIG_MEMCG
         KMALLOC_CGROUP,
+#endif
+#ifdef OBJ_EXT_VEC_CACHE
+        KMALLOC_OBJ_EXT_VEC,
 #endif
         NR_KMALLOC_TYPES
 };

@@ -654,6 +661,18 @@ typedef struct kmem_cache *
 kmem_buckets[KMALLOC_SHIFT_HIGH + 1];
 
 extern kmem_buckets kmalloc_caches[NR_KMALLOC_TYPES];
 
+#ifdef OBJ_EXT_VEC_CACHE

include/linux/slab.h +0 -0

 /*
  * Define gfp bits that should not be set for KMALLOC_NORMAL.
  */

We can do that in pre-7.2 kernels, by teaching kmalloc_type() and kmalloc_slab() select the new KMALLOC_TYPE based on __GFP_NO_OBJ_EXT?

e.g.) Select the new KMALLOC_TYPE when KMALLOC_NOT_NORMAL_BITS is not set AND __GFP_NO_OBJ_EXT is set.

This doesn't require kmem_buckets and should be much simpler.

Quoted 5 lines · Harry Yoo

We can do that in pre-7.2 kernels, by teaching kmalloc_type() and kmalloc_slab() select the new KMALLOC_TYPE based on __GFP_NO_OBJ_EXT?

e.g.) Select the new KMALLOC_TYPE when KMALLOC_NOT_NORMAL_BITS is not set AND __GFP_NO_OBJ_EXT is set.

Uh, this is bit subtle though.

In some cases KMALLOC_DMA == KMALLOC_NORMAL,
KMALLOC_CGROUP == KMALLOC_NORMAL,
or KMALLOC_RECLAIM == KMALLOC_NORMAL.

Just checking KMALLOC_NOT_NORMAL_BITS is misleading.

Quoted 15 lines · Harry Yoo

Quoted 5 lines · Harry Yoo

We can do that in pre-7.2 kernels, by teaching kmalloc_type() and kmalloc_slab() select the new KMALLOC_TYPE based on __GFP_NO_OBJ_EXT?

e.g.) Select the new KMALLOC_TYPE when KMALLOC_NOT_NORMAL_BITS is not set AND __GFP_NO_OBJ_EXT is set.

Uh, this is bit subtle though.

In some cases KMALLOC_DMA == KMALLOC_NORMAL,
KMALLOC_CGROUP == KMALLOC_NORMAL,
or KMALLOC_RECLAIM == KMALLOC_NORMAL.

Quoted 1 line

Just checking KMALLOC_NOT_NORMAL_BITS is misleading.

Here's a prototype for slab/for-next. Backporting it requires handling __GFP_NO_OBJ_EXT instead of SLAB_ALLOC_NO_RECURSE, but shouldn't be too difficult. Now writing changelog and going through testing...

Quoted 16 lines · Harry Yoo

Quoted 14 lines · Harry Yoo

Quoted 5 lines · Harry Yoo

We can do that in pre-7.2 kernels, by teaching kmalloc_type() and kmalloc_slab() select the new KMALLOC_TYPE based on __GFP_NO_OBJ_EXT?

e.g.) Select the new KMALLOC_TYPE when KMALLOC_NOT_NORMAL_BITS is not set AND __GFP_NO_OBJ_EXT is set.

Uh, this is bit subtle though.

In some cases KMALLOC_DMA == KMALLOC_NORMAL,
KMALLOC_CGROUP == KMALLOC_NORMAL,
or KMALLOC_RECLAIM == KMALLOC_NORMAL.

Just checking KMALLOC_NOT_NORMAL_BITS is misleading.

Quoted 3 lines

Here's a prototype for slab/for-next. Backporting it requires handling __GFP_NO_OBJ_EXT instead of SLAB_ALLOC_NO_RECURSE, but shouldn't be too difficult. Now writing changelog and going through testing...

Now thinking about the performance implication of sheaf allocations for kmalloc caches being served from KMALLOC_NO_RECURSE caches won't have sheaves at all

Quoted 9 lines · Harry Yoo

Quoted 5 lines · Harry Yoo

We can do that in pre-7.2 kernels, by teaching kmalloc_type() and kmalloc_slab() select the new KMALLOC_TYPE based on __GFP_NO_OBJ_EXT?

e.g.) Select the new KMALLOC_TYPE when KMALLOC_NOT_NORMAL_BITS is not set AND __GFP_NO_OBJ_EXT is set.

Quoted 7 lines

Uh, this is bit subtle though.

In some cases KMALLOC_DMA == KMALLOC_NORMAL,
KMALLOC_CGROUP == KMALLOC_NORMAL,
or KMALLOC_RECLAIM == KMALLOC_NORMAL.

Just checking KMALLOC_NOT_NORMAL_BITS is misleading.

Here's a prototype for slab/for-next. Backporting it requires handling __GFP_NO_OBJ_EXT instead of SLAB_ALLOC_NO_RECURSE, but shouldn't be too difficult. Now writing changelog and going through testing...

include/linux/slab.h +2 -0

index 51f03f18c9a7..91a71537a2fe 100644
@@ -684,6 +684,26 @@ static inline unsigned int arch_slab_minalign(void)
 #define KMALLOC_PARTITION_CACHES_NR    0
 #endif
 
+/*
+ * SLUB needs a separate kmalloc type, KMALLOC_NO_RECURSE, when

internal slab
+ * metadata of kmalloc objects can be allocated from the same kmalloc type.
+ */

include/linux/slab.h +19 -0

+#if defined(CONFIG_MEM_ALLOC_PROFILING)
+/*
+ * Memory allocation profiling can allocate internal slab metadata
+ * for any slab cache.
+ */
+#define HAS_KMALLOC_NO_RECURSE
+#elif defined(CONFIG_SLUB_TINY) && defined(CONFIG_MEMCG)
+/*
+ * Accounted slab objects are usually allocated from KMALLOC_CGROUP.
+ * On SLUB_TINY, those can be allocated from KMALLOC_NORMAL because
+ * KMALLOC_RECLAIM aliases with KMALLOC_CGROUP and has higher priority than
+ * KMALLOC_CGROUP.
+ */
+#define HAS_KMALLOC_NO_RECURSE
+#endif
+
 /*
  * Whenever changing this, take care of that kmalloc_type() and
  * create_kmalloc_caches() still work as intended.

@@ -702,6 +722,9 @@ enum kmalloc_cache_type {
 #endif
        KMALLOC_PARTITION_START = KMALLOC_NORMAL,
        KMALLOC_PARTITION_END = KMALLOC_PARTITION_START +
 KMALLOC_PARTITION_CACHES_NR,
+#ifdef HAS_KMALLOC_NO_RECURSE
+       KMALLOC_NO_RECURSE,
+#endif
 #ifdef CONFIG_SLUB_TINY
        KMALLOC_RECLAIM = KMALLOC_NORMAL,

#else

include/linux/slab.h +10 -0

@@ -716,6 +739,16 @@ enum kmalloc_cache_type {
        NR_KMALLOC_TYPES
 };
 
+#if !defined(HAS_KMALLOC_NO_RECURSE) && defined(CONFIG_SLAB_OBJ_EXT)
+/*
+ * kmalloc_flags() with SLAB_ALLOC_NO_RECURSE should not use KMALLOC_NORMAL
+ * if any of these alias with KMALLOC_NORMAL.
+ */
+static_assert(KMALLOC_DMA != KMALLOC_NORMAL);
+static_assert(KMALLOC_CGROUP != KMALLOC_NORMAL);
+static_assert(KMALLOC_RECLAIM != KMALLOC_NORMAL);
+#endif
+
 typedef struct kmem_cache * kmem_buckets[KMALLOC_SHIFT_HIGH + 1];
 
 extern kmem_buckets kmalloc_caches[NR_KMALLOC_TYPES];

mm/slab.h +2 -1

index 281a65233795..ba0560111488 100644
@@ -386,12 +386,21 @@ static inline unsigned int
 size_index_elem(unsigned int bytes)
  * KMALLOC_MAX_CACHE_SIZE and the caller must check that.
  */
 static inline struct kmem_cache *
-kmalloc_slab(size_t size, kmem_buckets *b, gfp_t flags, kmalloc_token_t
 token)
+kmalloc_slab(size_t size, kmem_buckets *b, gfp_t flags, kmalloc_token_t
 token,
+            unsigned int alloc_flags)

mm/slab.h +1 -1

        if (!b)
-               b = &kmalloc_caches[kmalloc_type(flags, token)];
+               b = &kmalloc_caches[type];
        if (size <= 192)

                index = kmalloc_size_index[size_index_elem(size)];
        else

mm/slab_common.c +5 -1

index b6426d7ceec9..8541f4a9cfda 100644
@@ -783,11 +783,15 @@ u8 kmalloc_size_index[24] __ro_after_init = {
 size_t kmalloc_size_roundup(size_t size)
 {
        if (size && size <= KMALLOC_MAX_CACHE_SIZE) {
+               struct kmem_cache *s;
+
                /*
                 * The flags don't matter since size_index is common to all.
                 * Neither does the caller for just getting ->object_size.
                 */
-               return kmalloc_slab(size, NULL, GFP_KERNEL,
 __kmalloc_token(0))->object_size;
+               s = kmalloc_slab(size, NULL, GFP_KERNEL, __kmalloc_token(0),
+                                SLAB_ALLOC_DEFAULT);
+               return s->object_size;
        }
 

        /* Above the smaller buckets, size is a multiple of page size. */

mm/slab_common.c +6 -0

@@ -843,6 +847,12 @@ EXPORT_SYMBOL(kmalloc_size_roundup);
 #define KMALLOC_PARTITION_NAME(N, sz)
 #endif
 
+#ifdef HAS_KMALLOC_NO_RECURSE
+#define KMALLOC_NO_RECURSE_NAME(sz) .name[KMALLOC_NO_RECURSE] =
 "kmalloc-no-recurse-" #sz,
+#else
+#define KMALLOC_NO_RECURSE_NAME(sz)
+#endif
+
 #define INIT_KMALLOC_INFO(__size, __short_size)                        \
 {                                                              \

        .name[KMALLOC_NORMAL]  = "kmalloc-" #__short_size,      \

mm/slab_common.c +6 -0

@@ -850,6 +860,7 @@ EXPORT_SYMBOL(kmalloc_size_roundup);
        KMALLOC_CGROUP_NAME(__short_size)                       \
        KMALLOC_DMA_NAME(__short_size)                          \
        KMALLOC_PARTITION_NAME(KMALLOC_PARTITION_CACHES_NR, __short_size)       \
+       KMALLOC_NO_RECURSE_NAME(__short_size)                   \
        .size = __size,                                         \
 }
 

@@ -966,6 +977,11 @@ new_kmalloc_cache(int idx, enum kmalloc_cache_type
 type)
                flags |= SLAB_NO_MERGE;
 #endif
 
+#ifdef HAS_KMALLOC_NO_RECURSE
+       if (type == KMALLOC_NO_RECURSE)
+               flags |= SLAB_NO_OBJ_EXT;
+#endif
+
        /*
         * If CONFIG_MEMCG is enabled, disable cache merging for

* KMALLOC_NORMAL caches.

mm/slub.c +0 -26

index 9f754cf1c187..a5745759f0af 100644
@@ -2123,42 +2123,6 @@ static inline void init_slab_obj_exts(struct slab
 *slab)
        slab->obj_exts = 0;
 }
 
-/*
- * Calculate the allocation size for slabobj_ext array.
- *
- * When memory allocation profiling is enabled, the obj_exts array
- * could be allocated from the same slab cache it's being allocated for.
- * This would prevent the slab from ever being freed because it would
- * always contain at least one allocated object (its own obj_exts array).
- *
- * To avoid this, increase the allocation size when we detect the array
- * may come from the same cache, forcing it to use a different cache.
- */
-static inline size_t obj_exts_alloc_size(struct kmem_cache *s,
-                                        struct slab *slab, gfp_t gfp)
-{
-       size_t sz = sizeof(struct slabobj_ext) * slab->objects;
-       struct kmem_cache *obj_exts_cache;
-
-       if (sz > KMALLOC_MAX_CACHE_SIZE)
-               return sz;
-
-       if (!is_kmalloc_normal(s))
-               return sz;
-
-       obj_exts_cache = kmalloc_slab(sz, NULL, gfp, __kmalloc_token(0));
-       /*
-        * We can't simply compare s with obj_exts_cache, because partitioned

kmalloc
- * caches have multiple caches per size, selected by caller address or
type.
- * Since caller address or type may differ between kmalloc_slab() and
actual

mm/slub.c +2 -12

-        * allocation, bump size when sizes are equal.
-        */
-       if (s->object_size == obj_exts_cache->object_size)
-               return obj_exts_cache->object_size + 1;
-
-       return sz;
-}
-
 int alloc_slab_obj_exts(struct slab *slab, struct kmem_cache *s,
                        gfp_t gfp, unsigned int alloc_flags)
 {

@@ -2168,15 +2132,13 @@ int alloc_slab_obj_exts(struct slab *slab,
 struct kmem_cache *s,
        unsigned long new_exts;
        unsigned long old_exts;
        struct slabobj_ext *vec;
-       size_t sz;
+       size_t sz = sizeof(struct slabobj_ext) * slab->objects;
 
        gfp &= ~OBJCGS_CLEAR_MASK;
        /* Prevent recursive extension vector allocation */
        alloc_flags |= SLAB_ALLOC_NO_RECURSE;
        alloc_flags &= ~SLAB_ALLOC_NEW_SLAB;
 
-       sz = obj_exts_alloc_size(s, slab, gfp);
-
        /* This will use kmalloc_nolock() if alloc_flags say so */
        vec = kmalloc_flags(sz, gfp | __GFP_ZERO, alloc_flags, slab_nid(slab));

@@ -5330,7 +5292,7 @@ void *__do_kmalloc_node(kmem_buckets *b, gfp_t
 flags, int node,
        if (unlikely(!size))
                return ZERO_SIZE_PTR;
 
-       s = kmalloc_slab(size, b, flags, token);
+       s = kmalloc_slab(size, b, flags, token, ac->alloc_flags);
 
        ret = slab_alloc_node(s, flags, node, ac);

        ret = kasan_kmalloc(s, ret, size, flags);

mm/slub.c +3 -1

@@ -5395,7 +5357,9 @@ static void
 *__kmalloc_nolock_noprof(DECL_TOKEN_PARAMS(size, token), gfp_t gfp_f
 retry:
        if (unlikely(size > KMALLOC_MAX_CACHE_SIZE))
                return NULL;
-       s = kmalloc_slab(size, NULL, gfp_flags, PASS_TOKEN_PARAM(token));
+
+       s = kmalloc_slab(size, NULL, gfp_flags, PASS_TOKEN_PARAM(token),
+                        ac->alloc_flags);
 
        if (!(s->flags & __CMPXCHG_DOUBLE) && !kmem_cache_debug(s))

                /*

--
Cheers,
Harry / Hyeonggon

Quoted 98 lines · Harry Yoo

Quoted 7 lines · Harry Yoo

Quoted 5 lines · Harry Yoo

We can do that in pre-7.2 kernels, by teaching kmalloc_type() and kmalloc_slab() select the new KMALLOC_TYPE based on __GFP_NO_OBJ_EXT?

e.g.) Select the new KMALLOC_TYPE when KMALLOC_NOT_NORMAL_BITS is not set AND __GFP_NO_OBJ_EXT is set.

Quoted 7 lines

Uh, this is bit subtle though.

In some cases KMALLOC_DMA == KMALLOC_NORMAL,
KMALLOC_CGROUP == KMALLOC_NORMAL,
or KMALLOC_RECLAIM == KMALLOC_NORMAL.

Just checking KMALLOC_NOT_NORMAL_BITS is misleading.

Here's a prototype for slab/for-next. Backporting it requires handling __GFP_NO_OBJ_EXT instead of SLAB_ALLOC_NO_RECURSE, but shouldn't be too difficult. Now writing changelog and going through testing...

include/linux/slab.h +2 -0

index 51f03f18c9a7..91a71537a2fe 100644
@@ -684,6 +684,26 @@ static inline unsigned int arch_slab_minalign(void)
 #define KMALLOC_PARTITION_CACHES_NR    0
 #endif
 
+/*
+ * SLUB needs a separate kmalloc type, KMALLOC_NO_RECURSE, when

internal slab
+ * metadata of kmalloc objects can be allocated from the same kmalloc type.
+ */

include/linux/slab.h +19 -0

+#if defined(CONFIG_MEM_ALLOC_PROFILING)
+/*
+ * Memory allocation profiling can allocate internal slab metadata
+ * for any slab cache.
+ */
+#define HAS_KMALLOC_NO_RECURSE
+#elif defined(CONFIG_SLUB_TINY) && defined(CONFIG_MEMCG)
+/*
+ * Accounted slab objects are usually allocated from KMALLOC_CGROUP.
+ * On SLUB_TINY, those can be allocated from KMALLOC_NORMAL because
+ * KMALLOC_RECLAIM aliases with KMALLOC_CGROUP and has higher priority than
+ * KMALLOC_CGROUP.
+ */
+#define HAS_KMALLOC_NO_RECURSE
+#endif
+
 /*
  * Whenever changing this, take care of that kmalloc_type() and
  * create_kmalloc_caches() still work as intended.

@@ -702,6 +722,9 @@ enum kmalloc_cache_type {
 #endif
        KMALLOC_PARTITION_START = KMALLOC_NORMAL,
        KMALLOC_PARTITION_END = KMALLOC_PARTITION_START +
 KMALLOC_PARTITION_CACHES_NR,
+#ifdef HAS_KMALLOC_NO_RECURSE
+       KMALLOC_NO_RECURSE,
+#endif
 #ifdef CONFIG_SLUB_TINY
        KMALLOC_RECLAIM = KMALLOC_NORMAL,

#else

include/linux/slab.h +10 -0

@@ -716,6 +739,16 @@ enum kmalloc_cache_type {
        NR_KMALLOC_TYPES
 };
 
+#if !defined(HAS_KMALLOC_NO_RECURSE) && defined(CONFIG_SLAB_OBJ_EXT)
+/*
+ * kmalloc_flags() with SLAB_ALLOC_NO_RECURSE should not use KMALLOC_NORMAL
+ * if any of these alias with KMALLOC_NORMAL.
+ */
+static_assert(KMALLOC_DMA != KMALLOC_NORMAL);
+static_assert(KMALLOC_CGROUP != KMALLOC_NORMAL);
+static_assert(KMALLOC_RECLAIM != KMALLOC_NORMAL);
+#endif
+
 typedef struct kmem_cache * kmem_buckets[KMALLOC_SHIFT_HIGH + 1];
 
 extern kmem_buckets kmalloc_caches[NR_KMALLOC_TYPES];

mm/slab.h +4 -1

index 281a65233795..ba0560111488 100644
@@ -386,12 +386,21 @@ static inline unsigned int
 size_index_elem(unsigned int bytes)
  * KMALLOC_MAX_CACHE_SIZE and the caller must check that.
  */
 static inline struct kmem_cache *
-kmalloc_slab(size_t size, kmem_buckets *b, gfp_t flags, kmalloc_token_t
 token)
+kmalloc_slab(size_t size, kmem_buckets *b, gfp_t flags, kmalloc_token_t
 token,
+            unsigned int alloc_flags)
 {
        unsigned int index;
+       enum kmalloc_cache_type type = kmalloc_type(flags, token);
+

mm/slab.h static inline unsigned int · line 403

Quoted 6 lines

mm/slab.h +6 -0

+#ifdef HAS_KMALLOC_NO_RECURSE
+       if (type >= KMALLOC_PARTITION_START &&
+                       type <= KMALLOC_PARTITION_END &&
+                       (alloc_flags & SLAB_ALLOC_NO_RECURSE))
+               type = KMALLOC_NO_RECURSE;
+#endif

Overall your approach LGTM, just here why not simply:

Minimally I'd now want to only do that size bumping when allocation profiling is enabled. Ideally that means both configured in and not booted with "never".

We probably should have done that already in 280ea9c3154b2.

Because AFAIU
memcg-only obj_exts array don't have this issue
(or maybe they do have the > [1] issue? Harry?).

Quoted 5 lines · Vlastimil Babka (SUSE)

Minimally I'd now want to only do that size bumping when allocation profiling is enabled. Ideally that means both configured in and not booted with "never".

We probably should have done that already in 280ea9c3154b2.

I think we did already.

obj_exts allocation triggered by memcg has __GFP_ACCOUNT, kmalloc type is KMALLOC_CGROUP, and so is_kmalloc_normal() should return false?

Perhaps a comment above !is_kmalloc_normal() check would be nice.

Quoted 3 lines

Because AFAIU
memcg-only obj_exts array don't have this issue
(or maybe they do have the > [1] issue? Harry?).

memcg-only obj_exts array doesn't have this issue and [1].

Quoted 1 line

Because obj_exts are not accounted and so we can't allocate obj_exts

from KMALLOC_CGROUP.