Re: [PATCH v6 1/4] mm/slub: enable debugging memory wasting of kmalloc

[Feng Tang <feng.tang@intel.com>]

On Thu, Nov 03, 2022 at 06:35:53PM +0100, Vlastimil Babka wrote:

[...]

> >> But FYI I'm suggesting to drop CONFIG_TRACING=n variant:
> >> 
> >> https://lore.kernel.org/linux-mm/20221101222520.never.109-kees@kernel.org/T/#m20ecf14390e406247bde0ea9cce368f469c539ed
> >> 
> >> Any thoughts?
> > 
> > I'll get to it, also I think we were pondering that within your series too,
> > but I wanted to postpone in case somebody objects to the extra function call
> > it creates.
> > But that would be for 6.2 anyway while I'll collect the fix here for 6.1.
> 
> On second thought, the fix is making the inlined kmalloc_trace() expand to a
> call that had 2 parameters and now it has 5, which seems to me like a worse
> thing (code bloat) than the function call. With the other reasons to ditch
> the CONFIG_TRACING=n variant I'm inclined to just do it right now.

That's great! It will save much trouble, and reduce code complexity.

Btw, the patch below also has some compiling issue for some kconfig
(thanks to 0Day's kbuild bot).

Thanks,
Feng

> >>> 
> >>> How about the following fix?
> >>> 
> >>> Thanks,
> >>> Feng
> >>> 
> >>> ---
> >>> From 9f9fa9da8946fd44625f873c0f51167357075be1 Mon Sep 17 00:00:00 2001
> >>> From: Feng Tang <feng.tang@intel.com>
> >>> Date: Thu, 3 Nov 2022 21:32:10 +0800
> >>> Subject: [PATCH] mm/slub: Add missing orig_size parameter for wastage debug
> >>> 
> >>> commit 6edf2576a6cc ("mm/slub: enable debugging memory wasting of
> >>> kmalloc") was introduced for debugging kmalloc memory wastage,
> >>> and it missed to pass the original request size for kmalloc_trace()
> >>> and kmalloc_node_trace() in CONFIG_TRACING=n path.
> >>> 
> >>> Fix it by using __kmem_cache_alloc_node() with correct original
> >>> request size.
> >>> 
> >>> Fixes: 6edf2576a6cc ("mm/slub: enable debugging memory wasting of kmalloc")
> >>> Suggested-by: Vlastimil Babka <vbabka@suse.cz>
> >>> Signed-off-by: Feng Tang <feng.tang@intel.com>
> >>> ---
> >>>  include/linux/slab.h | 9 +++++++--
> >>>  1 file changed, 7 insertions(+), 2 deletions(-)
> >>> 
> >>> diff --git a/include/linux/slab.h b/include/linux/slab.h
> >>> index 90877fcde70b..9691afa569e1 100644
> >>> --- a/include/linux/slab.h
> >>> +++ b/include/linux/slab.h
> >>> @@ -469,6 +469,9 @@ void *__kmalloc_node(size_t size, gfp_t flags, int node) __assume_kmalloc_alignm
> >>>  							 __alloc_size(1);
> >>>  void *kmem_cache_alloc_node(struct kmem_cache *s, gfp_t flags, int node) __assume_slab_alignment
> >>>  									 __malloc;
> >>> +void *__kmem_cache_alloc_node(struct kmem_cache *s, gfp_t flags, int node,
> >>> +				size_t orig_size, unsigned long caller) __assume_slab_alignment
> >>> +									 __malloc;
> >>>  
> >>>  #ifdef CONFIG_TRACING
> >>>  void *kmalloc_trace(struct kmem_cache *s, gfp_t flags, size_t size)
> >>> @@ -482,7 +485,8 @@ void *kmalloc_node_trace(struct kmem_cache *s, gfp_t gfpflags,
> >>>  static __always_inline __alloc_size(3)
> >>>  void *kmalloc_trace(struct kmem_cache *s, gfp_t flags, size_t size)
> >>>  {
> >>> -	void *ret = kmem_cache_alloc(s, flags);
> >>> +	void *ret = __kmem_cache_alloc_node(s, flags, NUMA_NO_NODE,
> >>> +					    size, _RET_IP_);
> >>>  
> >>>  	ret = kasan_kmalloc(s, ret, size, flags);
> >>>  	return ret;
> >>> @@ -492,7 +496,8 @@ static __always_inline __alloc_size(4)
> >>>  void *kmalloc_node_trace(struct kmem_cache *s, gfp_t gfpflags,
> >>>  			 int node, size_t size)
> >>>  {
> >>> -	void *ret = kmem_cache_alloc_node(s, gfpflags, node);
> >>> +	void *ret = __kmem_cache_alloc_node(s, gfpflags, node,
> >>> +					    size, _RET_IP_);
> >>>  
> >>>  	ret = kasan_kmalloc(s, ret, size, gfpflags);
> >>>  	return ret;
> >>> -- 
> >>> 2.34.1
> >>> 
> >>> 
> >>> 
> >> 
> > 
> 
>