[PATCH v2] mm/slub: disable user tracing for kmemleak caches by default

[PATCH v2] mm/slub: disable user tracing for kmemleak caches by default

[Johannes Berg]

From: Johannes Berg <johannes.berg@intel.com>

If kmemleak is enabled, it uses a kmem cache for its own objects.
These objects are used to hold information kmemleak uses, including
a stack trace. If slub_debug is also turned on, each of them has
*another* stack trace, so the overhead adds up, and on my tests (on
ARCH=um, admittedly) 2/3rds of the allocations end up being doing
the stack tracing.

Turn off SLAB_STORE_USER if SLAB_NOLEAKTRACE was given, to avoid
storing the essentially same data twice.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
Perhaps instead it should go the other way around, and kmemleak
could even use/access the stack trace that's already in there ...
But I don't really care too much, I can just turn off slub debug
for the kmemleak caches via the command line anyway :-)

v2:
 - strip SLAB_STORE_USER only coming from slub_debug so that the
   command line args always take effect

---
 mm/slub.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/mm/slub.c b/mm/slub.c
index 34dcc09e2ec9..a66c9948c529 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1412,6 +1412,15 @@ slab_flags_t kmem_cache_flags(unsigned int object_size,
 	size_t len;
 	char *next_block;
 	slab_flags_t block_flags;
+	slab_flags_t slub_debug_local = slub_debug;
+
+	/*
+	 * If the slab cache is for debugging (e.g. kmemleak) then
+	 * don't store user (stack trace) information by default,
+	 * but let the user enable it via the command line below.
+	 */
+	if (flags & SLAB_NOLEAKTRACE)
+		slub_debug_local &= ~SLAB_STORE_USER;
 
 	len = strlen(name);
 	next_block = slub_debug_string;
@@ -1446,7 +1455,7 @@ slab_flags_t kmem_cache_flags(unsigned int object_size,
 		}
 	}
 
-	return flags | slub_debug;
+	return flags | slub_debug_local;
 }
 #else /* !CONFIG_SLUB_DEBUG */
 static inline void setup_object_debug(struct kmem_cache *s,
-- 
2.26.2

Re: [PATCH v2] mm/slub: disable user tracing for kmemleak caches by default

[Vlastimil Babka]

On 1/13/21 9:51 PM, Johannes Berg wrote:
> From: Johannes Berg <johannes.berg@intel.com>
> 
> If kmemleak is enabled, it uses a kmem cache for its own objects.
> These objects are used to hold information kmemleak uses, including
> a stack trace. If slub_debug is also turned on, each of them has
> *another* stack trace, so the overhead adds up, and on my tests (on
> ARCH=um, admittedly) 2/3rds of the allocations end up being doing
> the stack tracing.
> 
> Turn off SLAB_STORE_USER if SLAB_NOLEAKTRACE was given, to avoid
> storing the essentially same data twice.
> 
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>

Acked-by: Vlastimil Babka <vbabka@suse.cz>

> ---
> Perhaps instead it should go the other way around, and kmemleak
> could even use/access the stack trace that's already in there ...
> But I don't really care too much, I can just turn off slub debug
> for the kmemleak caches via the command line anyway :-)
> 
> v2:
>  - strip SLAB_STORE_USER only coming from slub_debug so that the
>    command line args always take effect
> 
> ---
>  mm/slub.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/slub.c b/mm/slub.c
> index 34dcc09e2ec9..a66c9948c529 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -1412,6 +1412,15 @@ slab_flags_t kmem_cache_flags(unsigned int object_size,
>  	size_t len;
>  	char *next_block;
>  	slab_flags_t block_flags;
> +	slab_flags_t slub_debug_local = slub_debug;
> +
> +	/*
> +	 * If the slab cache is for debugging (e.g. kmemleak) then
> +	 * don't store user (stack trace) information by default,
> +	 * but let the user enable it via the command line below.
> +	 */
> +	if (flags & SLAB_NOLEAKTRACE)
> +		slub_debug_local &= ~SLAB_STORE_USER;
>  
>  	len = strlen(name);
>  	next_block = slub_debug_string;
> @@ -1446,7 +1455,7 @@ slab_flags_t kmem_cache_flags(unsigned int object_size,
>  		}
>  	}
>  
> -	return flags | slub_debug;
> +	return flags | slub_debug_local;
>  }
>  #else /* !CONFIG_SLUB_DEBUG */
>  static inline void setup_object_debug(struct kmem_cache *s,
> 

Re: [PATCH v2] mm/slub: disable user tracing for kmemleak caches by default

[Catalin Marinas]

On Wed, Jan 13, 2021 at 09:51:14PM +0100, Johannes Berg wrote:
> From: Johannes Berg <johannes.berg@intel.com>
> 
> If kmemleak is enabled, it uses a kmem cache for its own objects.
> These objects are used to hold information kmemleak uses, including
> a stack trace. If slub_debug is also turned on, each of them has
> *another* stack trace, so the overhead adds up, and on my tests (on
> ARCH=um, admittedly) 2/3rds of the allocations end up being doing
> the stack tracing.
> 
> Turn off SLAB_STORE_USER if SLAB_NOLEAKTRACE was given, to avoid
> storing the essentially same data twice.
> 
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>

I think that's the simplest.

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

> Perhaps instead it should go the other way around, and kmemleak
> could even use/access the stack trace that's already in there ...
> But I don't really care too much, I can just turn off slub debug
> for the kmemleak caches via the command line anyway :-)

This stack trace doesn't seem to be accessible in a unified way across
the sl*b allocators. Given that kmemleak already needs to track this
information for other objects (vmalloc, certain page allocations), it's
probably more hassle to handle it differently for slab objects (I don't
say it's impossible, just not sure it's worth).

-- 
Catalin

Re: [PATCH v2] mm/slub: disable user tracing for kmemleak caches by default

[David Rientjes]

On Wed, 13 Jan 2021, Johannes Berg wrote:

> From: Johannes Berg <johannes.berg@intel.com>
> 
> If kmemleak is enabled, it uses a kmem cache for its own objects.
> These objects are used to hold information kmemleak uses, including
> a stack trace. If slub_debug is also turned on, each of them has
> *another* stack trace, so the overhead adds up, and on my tests (on
> ARCH=um, admittedly) 2/3rds of the allocations end up being doing
> the stack tracing.
> 
> Turn off SLAB_STORE_USER if SLAB_NOLEAKTRACE was given, to avoid
> storing the essentially same data twice.
> 
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>

Acked-by: David Rientjes <rientjes@google.com>

> ---
> Perhaps instead it should go the other way around, and kmemleak
> could even use/access the stack trace that's already in there ...
> But I don't really care too much, I can just turn off slub debug
> for the kmemleak caches via the command line anyway :-)
> 

I think making the change to kmem_cache_flags() is likely the simplest.