All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] slab: allow sheaves for SLAB_NOLEAKTRACE caches
@ 2026-11-12 10:04 Breno Leitao
  2026-11-12 16:40 ` Catalin Marinas
  2026-11-13 10:05 ` sashiko-bot
  0 siblings, 2 replies; 5+ messages in thread
From: Breno Leitao @ 2026-11-12 10:04 UTC (permalink / raw)
  To: Vlastimil Babka, Harry Yoo, Hao Li, Christoph Lameter,
	David Rientjes, Roman Gushchin, Andrew Morton
  Cc: linux-mm, linux-kernel, kernel-team, Catalin Marinas,
	Breno Leitao

This is a patch that makes kmemleak cheaper to run, by letting
kmemleak's own metadata cache use a _new_ sheaves type. The flag only
makes that possible without recursion.

The cost today:
  * every allocation kmemleak tracks needs one kmemleak_object from
    object_cache. An RCU callback frees it later.

Why that's slow:
  * object_cache has no sheaves, so each of those allocations and frees
    goes to the shared slab lists under n->list_lock.
     - On meta's fleet kmemleak kernels, list_lock is the second most
       contended lock, after kmemleak_lock.
     - About 88% of that wait is RCU callbacks freeing kmemleak objects.

Why sheaves aren't allowed:
  * a sheaf is a kmalloc object. Tracking it makes kmemleak allocate
    another kmemleak_object, which needs a sheaf, and so on.

When I reported the recursion in [1], I suggested keeping
SLAB_NOLEAKTRACE caches away from sheaves, and commit e47c897a2949
("slab: add sheaves to most caches") did that. It stopped the
recursion, but it took the sheaves away from the whole cache, although
only the kmemleak tracking of the sheaf itself recursed.

Commit 17c38c88294d ("slab: remove cpu (partial) slabs usage from
allocation paths"), from the same series, then left these caches with
no per-CPU caching at all.

In hindsight, skipping kmemleak only for the
sheaf itself, as the new SLAB_ALLOC_NO_LEAKTRACE does, would have been
the better fix.

Link: https://lore.kernel.org/all/aW_NK2NXVgtuzCVH@gmail.com/ [1]
Cc: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Breno Leitao <leitao@debian.org>
---
 mm/slab.h |  1 +
 mm/slub.c | 19 +++++++++++--------
 2 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/mm/slab.h b/mm/slab.h
index e94ac63d10dc03..7b20f10aa46a2e 100644
--- a/mm/slab.h
+++ b/mm/slab.h
@@ -23,6 +23,7 @@
 #define SLAB_ALLOC_NEW_SLAB	0x02 /* a flag for alloc_slab_obj_exts() */
 #define SLAB_ALLOC_NO_RECURSE	0x04 /* prevent kmalloc() recursion */
 #define SLAB_ALLOC_NO_OBJ_EXT	0x08 /* prevent obj_exts array allocation */
+#define SLAB_ALLOC_NO_LEAKTRACE	0x10 /* skip kmemleak tracking */
 
 #define SLAB_FREE_DEFAULT	0x00 /* no flags */
 #define SLAB_FREE_NOLOCK	0x01 /* spinning not allowed */
diff --git a/mm/slub.c b/mm/slub.c
index 8369853ddca721..f19e20b6f9a4fa 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -2850,6 +2850,13 @@ static struct slab_sheaf *__alloc_empty_sheaf(struct kmem_cache *s, gfp_t gfp,
 	if (s->flags & SLAB_KMALLOC)
 		alloc_flags |= SLAB_ALLOC_NO_RECURSE;
 
+	/*
+	 * kmemleak allocates its metadata from SLAB_NOLEAKTRACE caches, so
+	 * tracking their sheaves would recurse into kmemleak.
+	 */
+	if (s->flags & SLAB_NOLEAKTRACE)
+		alloc_flags |= SLAB_ALLOC_NO_LEAKTRACE;
+
 	sheaf_size = struct_size(sheaf, objects, capacity);
 	sheaf = kmalloc_flags(sheaf_size, gfp | __GFP_ZERO, alloc_flags, NUMA_NO_NODE);
 
@@ -4727,7 +4734,8 @@ bool slab_post_alloc_hook(struct kmem_cache *s, gfp_t flags, size_t size,
 		if (init && p[i] && !is_kfence_address(p[i]))
 			memset(p[i], 0, zero_size);
 
-		if (alloc_flags_allow_spinning(ac->alloc_flags))
+		if (alloc_flags_allow_spinning(ac->alloc_flags) &&
+		    !(ac->alloc_flags & SLAB_ALLOC_NO_LEAKTRACE))
 			kmemleak_alloc_recursive(p[i], s->object_size, 1,
 						 s->flags, init_flags);
 		kmsan_slab_alloc(s, p[i], init_flags);
@@ -7987,13 +7995,8 @@ static unsigned int calculate_sheaf_capacity(struct kmem_cache *s,
 	if (IS_ENABLED(CONFIG_SLUB_TINY) || s->flags & SLAB_DEBUG_FLAGS)
 		return 0;
 
-	/*
-	 * Bootstrap caches can't have sheaves for now (SLAB_NO_SHEAVES).
-	 * SLAB_NOLEAKTRACE caches (e.g., kmemleak's object_cache) must not
-	 * have sheaves to avoid recursion when sheaf allocation triggers
-	 * kmemleak tracking.
-	 */
-	if (s->flags & (SLAB_NO_SHEAVES | SLAB_NOLEAKTRACE))
+	/* Bootstrap caches can't have sheaves for now (SLAB_NO_SHEAVES). */
+	if (s->flags & SLAB_NO_SHEAVES)
 		return 0;
 
 	/*

---
base-commit: 630059a5a632377b9b7b487baccde92b7ae1dd47
change-id: 20261110-kmemleak_sheaves-b3531591ad21

Best regards,
--  
Breno Leitao <leitao@debian.org>



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] slab: allow sheaves for SLAB_NOLEAKTRACE caches
  2026-11-12 10:04 [PATCH] slab: allow sheaves for SLAB_NOLEAKTRACE caches Breno Leitao
@ 2026-11-12 16:40 ` Catalin Marinas
  2026-11-13 10:05 ` sashiko-bot
  1 sibling, 0 replies; 5+ messages in thread
From: Catalin Marinas @ 2026-11-12 16:40 UTC (permalink / raw)
  To: Breno Leitao
  Cc: Vlastimil Babka, Harry Yoo, Hao Li, Christoph Lameter,
	David Rientjes, Roman Gushchin, Andrew Morton, linux-mm,
	linux-kernel, kernel-team

On Thu, Nov 12, 2026 at 02:04:18AM -0800, Breno Leitao wrote:
> This is a patch that makes kmemleak cheaper to run, by letting
> kmemleak's own metadata cache use a _new_ sheaves type. The flag only
> makes that possible without recursion.
> 
> The cost today:
>   * every allocation kmemleak tracks needs one kmemleak_object from
>     object_cache. An RCU callback frees it later.
> 
> Why that's slow:
>   * object_cache has no sheaves, so each of those allocations and frees
>     goes to the shared slab lists under n->list_lock.
>      - On meta's fleet kmemleak kernels, list_lock is the second most
>        contended lock, after kmemleak_lock.
>      - About 88% of that wait is RCU callbacks freeing kmemleak objects.
> 
> Why sheaves aren't allowed:
>   * a sheaf is a kmalloc object. Tracking it makes kmemleak allocate
>     another kmemleak_object, which needs a sheaf, and so on.
> 
> When I reported the recursion in [1], I suggested keeping
> SLAB_NOLEAKTRACE caches away from sheaves, and commit e47c897a2949
> ("slab: add sheaves to most caches") did that. It stopped the
> recursion, but it took the sheaves away from the whole cache, although
> only the kmemleak tracking of the sheaf itself recursed.
> 
> Commit 17c38c88294d ("slab: remove cpu (partial) slabs usage from
> allocation paths"), from the same series, then left these caches with
> no per-CPU caching at all.
> 
> In hindsight, skipping kmemleak only for the
> sheaf itself, as the new SLAB_ALLOC_NO_LEAKTRACE does, would have been
> the better fix.
> 
> Link: https://lore.kernel.org/all/aW_NK2NXVgtuzCVH@gmail.com/ [1]
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Signed-off-by: Breno Leitao <leitao@debian.org>

Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] slab: allow sheaves for SLAB_NOLEAKTRACE caches
  2026-11-12 10:04 [PATCH] slab: allow sheaves for SLAB_NOLEAKTRACE caches Breno Leitao
  2026-11-12 16:40 ` Catalin Marinas
@ 2026-11-13 10:05 ` sashiko-bot
  2026-11-13 12:27   ` Catalin Marinas
  1 sibling, 1 reply; 5+ messages in thread
From: sashiko-bot @ 2026-11-13 10:05 UTC (permalink / raw)
  To: Breno Leitao, Vlastimil Babka, Harry Yoo, Hao Li,
	Christoph Lameter, David Rientjes, Roman Gushchin, Andrew Morton
  Cc: linux-mm, kernel-team, Catalin Marinas

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] mm/slub: SLAB_ALLOC_NO_LEAKTRACE leaks into nested sheaf
  allocations
--

commit 7da9d1d54f05d33de4e779dda137bfa95c2a403c
Author: Breno Leitao <leitao@debian.org>

slab: allow sheaves for SLAB_NOLEAKTRACE caches

This commit introduces SLAB_ALLOC_NO_LEAKTRACE so sheaves allocated for
SLAB_NOLEAKTRACE caches skip kmemleak tracking, allowing those caches to
use per-CPU sheaves without recursing into kmemleak.

Link: https://lore.kernel.org/all/aW_NK2NXVgtuzCVH@gmail.com/ [1]

> diff --git a/mm/slub.c b/mm/slub.c
> index 8369853ddca72..f19e20b6f9a4f 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -2850,6 +2850,13 @@ static struct slab_sheaf *__alloc_empty_sheaf(struct kmem_cache *s, gfp_t gfp,
>  	if (s->flags & SLAB_KMALLOC)
>  		alloc_flags |= SLAB_ALLOC_NO_RECURSE;
>  
> +	/*
> +	 * kmemleak allocates its metadata from SLAB_NOLEAKTRACE caches, so
> +	 * tracking their sheaves would recurse into kmemleak.
> +	 */
> +	if (s->flags & SLAB_NOLEAKTRACE)
> +		alloc_flags |= SLAB_ALLOC_NO_LEAKTRACE;
> +
>  	sheaf_size = struct_size(sheaf, objects, capacity);
>  	sheaf = kmalloc_flags(sheaf_size, gfp | __GFP_ZERO, alloc_flags, NUMA_NO_NODE);

[Severity: Medium]
Does SLAB_ALLOC_NO_LEAKTRACE leak into nested sheaf and slabobj_ext
allocations for the backing normal kmalloc cache?

When s is a SLAB_NOLEAKTRACE cache (such as kmemleak's object_cache),
s->flags & SLAB_KMALLOC is false, so __alloc_empty_sheaf() passes
SLAB_ALLOC_NO_LEAKTRACE to kmalloc_flags() without SLAB_ALLOC_NO_RECURSE.
If the backing normal kmalloc bucket needs a new empty sheaf in
alloc_from_pcs() -> __pcs_replace_empty_main() -> alloc_empty_sheaf(),
that nested sheaf allocation inherits SLAB_ALLOC_NO_LEAKTRACE and is not
registered with kmemleak.

Because kmemleak never scans that untracked normal kmalloc sheaf:
- If the sheaf is later used as pcs->rcu_free in __kfree_rcu_sheaf(),
  kvfree_call_rcu() returns before kmemleak_ignore(ptr) while freeing is
  deferred, so kmemleak_scan() misses pcs->rcu_free->objects[] and reports
  queued objects as false-positive leaks.
- Any tracked kmalloc sheaf linked between two untracked sheaves on
  barn->sheaves_full or barn->sheaves_empty is unreachable during
  kmemleak_scan() and is also reported as a false-positive leak.
- In slab_post_alloc_hook() -> alloc_tagging_slab_alloc_hook() ->
  prepare_slab_obj_exts_hook() -> alloc_slab_obj_exts(), the slabobj_ext
  vector is likewise allocated with SLAB_ALLOC_NO_LEAKTRACE still set,
  skipping kmemleak registration before calling kmemleak_not_leak(vec).

Setting SLAB_ALLOC_NO_RECURSE along with SLAB_ALLOC_NO_LEAKTRACE (or
clearing SLAB_ALLOC_NO_LEAKTRACE before nested allocations) would prevent
the flag from propagating into the normal kmalloc cache's own metadata.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261112-kmemleak_sheaves-v1-1-e40e3d73b253@debian.org?part=1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] slab: allow sheaves for SLAB_NOLEAKTRACE caches
  2026-11-13 10:05 ` sashiko-bot
@ 2026-11-13 12:27   ` Catalin Marinas
  2026-11-13 14:21     ` Breno Leitao
  0 siblings, 1 reply; 5+ messages in thread
From: Catalin Marinas @ 2026-11-13 12:27 UTC (permalink / raw)
  To: sashiko-reviews
  Cc: Breno Leitao, Vlastimil Babka, Harry Yoo, Hao Li,
	Christoph Lameter, David Rientjes, Roman Gushchin, Andrew Morton,
	linux-mm, kernel-team

On Fri, Nov 13, 2026 at 10:05:08AM +0000, sashiko-bot@kernel.org wrote:
> > diff --git a/mm/slub.c b/mm/slub.c
> > index 8369853ddca72..f19e20b6f9a4f 100644
> > --- a/mm/slub.c
> > +++ b/mm/slub.c
> > @@ -2850,6 +2850,13 @@ static struct slab_sheaf *__alloc_empty_sheaf(struct kmem_cache *s, gfp_t gfp,
> >  	if (s->flags & SLAB_KMALLOC)
> >  		alloc_flags |= SLAB_ALLOC_NO_RECURSE;
> >  
> > +	/*
> > +	 * kmemleak allocates its metadata from SLAB_NOLEAKTRACE caches, so
> > +	 * tracking their sheaves would recurse into kmemleak.
> > +	 */
> > +	if (s->flags & SLAB_NOLEAKTRACE)
> > +		alloc_flags |= SLAB_ALLOC_NO_LEAKTRACE;
> > +
> >  	sheaf_size = struct_size(sheaf, objects, capacity);
> >  	sheaf = kmalloc_flags(sheaf_size, gfp | __GFP_ZERO, alloc_flags, NUMA_NO_NODE);
> 
> [Severity: Medium]
> Does SLAB_ALLOC_NO_LEAKTRACE leak into nested sheaf and slabobj_ext
> allocations for the backing normal kmalloc cache?
> 
> When s is a SLAB_NOLEAKTRACE cache (such as kmemleak's object_cache),
> s->flags & SLAB_KMALLOC is false, so __alloc_empty_sheaf() passes
> SLAB_ALLOC_NO_LEAKTRACE to kmalloc_flags() without SLAB_ALLOC_NO_RECURSE.
> If the backing normal kmalloc bucket needs a new empty sheaf in
> alloc_from_pcs() -> __pcs_replace_empty_main() -> alloc_empty_sheaf(),
> that nested sheaf allocation inherits SLAB_ALLOC_NO_LEAKTRACE and is not
> registered with kmemleak.
> 
> Because kmemleak never scans that untracked normal kmalloc sheaf:
> - If the sheaf is later used as pcs->rcu_free in __kfree_rcu_sheaf(),
>   kvfree_call_rcu() returns before kmemleak_ignore(ptr) while freeing is
>   deferred, so kmemleak_scan() misses pcs->rcu_free->objects[] and reports
>   queued objects as false-positive leaks.
> - Any tracked kmalloc sheaf linked between two untracked sheaves on
>   barn->sheaves_full or barn->sheaves_empty is unreachable during
>   kmemleak_scan() and is also reported as a false-positive leak.
> - In slab_post_alloc_hook() -> alloc_tagging_slab_alloc_hook() ->
>   prepare_slab_obj_exts_hook() -> alloc_slab_obj_exts(), the slabobj_ext
>   vector is likewise allocated with SLAB_ALLOC_NO_LEAKTRACE still set,
>   skipping kmemleak registration before calling kmemleak_not_leak(vec).
> 
> Setting SLAB_ALLOC_NO_RECURSE along with SLAB_ALLOC_NO_LEAKTRACE (or
> clearing SLAB_ALLOC_NO_LEAKTRACE before nested allocations) would prevent
> the flag from propagating into the normal kmalloc cache's own metadata.

Sashiko is right here, this could lead to false positives. I don't think
clearing SLAB_ALLOC_NO_LEAKTRACE before nested allocations is correct
since that could reintroduce the original recursion. Adding
SLAB_ALLOC_NO_RECURSE alongside it should fix this.

IIUC, I think we need to include SLAB_NOLEAKTRACE in the
mark_obj_codetag_empty() condition in __free_empty_sheaf(), alongside
SLAB_KMALLOC, if we go with the NO_RECURSE option.

-- 
Catalin


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] slab: allow sheaves for SLAB_NOLEAKTRACE caches
  2026-11-13 12:27   ` Catalin Marinas
@ 2026-11-13 14:21     ` Breno Leitao
  0 siblings, 0 replies; 5+ messages in thread
From: Breno Leitao @ 2026-11-13 14:21 UTC (permalink / raw)
  To: Catalin Marinas
  Cc: sashiko-reviews, Vlastimil Babka, Harry Yoo, Hao Li,
	Christoph Lameter, David Rientjes, Roman Gushchin, Andrew Morton,
	linux-mm, kernel-team

On Fri, Nov 13, 2026 at 12:27:04PM +0000, Catalin Marinas wrote:
> On Fri, Nov 13, 2026 at 10:05:08AM +0000, sashiko-bot@kernel.org wrote:
> >
> > Setting SLAB_ALLOC_NO_RECURSE along with SLAB_ALLOC_NO_LEAKTRACE (or
> > clearing SLAB_ALLOC_NO_LEAKTRACE before nested allocations) would prevent
> > the flag from propagating into the normal kmalloc cache's own metadata.
> 
> Sashiko is right here, this could lead to false positives. I don't think
> clearing SLAB_ALLOC_NO_LEAKTRACE before nested allocations is correct
> since that could reintroduce the original recursion. Adding
> SLAB_ALLOC_NO_RECURSE alongside it should fix this.

ack. I will respin v2 and sets both flags for SLAB_NOLEAKTRACE caches.

> IIUC, I think we need to include SLAB_NOLEAKTRACE in the
> mark_obj_codetag_empty() condition in __free_empty_sheaf(), alongside
> SLAB_KMALLOC, if we go with the NO_RECURSE option.

Ack. without that hunk, CONFIG_MEM_ALLOC_PROFILING_DEBUG warns
"alloc_tag was not set" from __free_empty_sheaf().

Thanks for the review, respining v2.
--breno


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-11-13 14:21 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-12 10:04 [PATCH] slab: allow sheaves for SLAB_NOLEAKTRACE caches Breno Leitao
2026-11-12 16:40 ` Catalin Marinas
2026-11-13 10:05 ` sashiko-bot
2026-11-13 12:27   ` Catalin Marinas
2026-11-13 14:21     ` Breno Leitao

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.