Re: [PATCH v2 09/13] mm/slab: change struct slabobj_ext to a union

Hao Li <[email protected]> Thu, 23 Jul 2026 17:56:56 +0800
Newsgroups org.kernel.vger.cgroups,org.kernel.vger.linux-kernel,org.kvack.linux-mm
Message-ID <amHkbJsvOjcfUR3D@fedora>
On Thu, Jul 23, 2026 at 03:27:51PM +0900, Harry Yoo wrote:
> 
> 
> On 7/23/26 12:05 PM, Hao Li wrote:
> > On Mon, Jul 20, 2026 at 04:16:23PM +0200, Vlastimil Babka (SUSE) wrote:
> >> Currently, struct slabobj_ext can hold both objcg pointer and
> >> codetag_ref (when both are compile-enabled) and there is an array of as
> >> many slabobj_ext instances as there are objects in a slab.
> >>
> >> This makes the layout fixed so even if codetag_ref is unused (because
> >> memory allocation profiling is disabled), the space for them is
> >> allocated and wasted. Similarly, some caches (currently kmalloc_normal)
> >> do not ever need objcg pointers, leading to wasted memory with memory
> >> allocation profiling enabled.
> >>
> >> To make this more flexible, change the layout so that struct slabobj_ext
> >> becomes a union of objcg pointer and codetag_ref (to ensure uniform
> >> size; in practice both are the same size anyway). The slabobj_ext array
> >> then can have twice as many elements as before. For cache locality
> >> purposes, the effective memory layout is unchanged, so objcg and codetag
> >> ref for a given object are still adjacent.
> >>
> >> cache_obj_ext_size() returns the effective size of (0-2) struct
> >> slabobj_ext's for a cache, slab_obj_ext_size() for a slab. Currently
> >> both return a constant value derived from the config options, but will
> >> be made dynamic later. Replace all sizeof(slabobj_ext) usage with these.
> >>
> >> No functional change intended, the layout is still effectively static.
> >>
> >> Reviewed-by: Suren Baghdasaryan <[email protected]>
> >> Signed-off-by: Vlastimil Babka (SUSE) <[email protected]>
> >> ---
> >>  mm/slab.h | 49 +++++++++++++++++++++++++++++++++++++++----------
> >>  mm/slub.c | 19 +++++++++++--------
> >>  2 files changed, 50 insertions(+), 18 deletions(-)
> >>
> >> diff --git a/mm/slab.h b/mm/slab.h
> >> index e586798e4f16..f8446167e175 100644
> >> --- a/mm/slab.h
> >> +++ b/mm/slab.h
> >> @@ -550,18 +550,42 @@ static inline bool need_kmalloc_no_objext(void)
> >>  }
> >>  
> >>  /*
> >> - * Extended information for slab objects stored as an array in page->memcg_data
> >> - * if MEMCG_DATA_OBJEXTS is set.
> >> + * Extended information for slab objects stored as a pointer to an array in
> >> + * slab->obj_exts (aliasing page->memcg_data) if MEMCG_DATA_OBJEXTS is set.
> >>   */
> >>  struct slabobj_ext {
> >> +	/*
> >> +	 * All elements of the union should be pointer-sized to avoid memory
> >> +	 * waste
> >> +	 */
> >> +	union {
> >>  #ifdef CONFIG_MEMCG
> >> -	struct obj_cgroup *_objcg;
> >> +		struct obj_cgroup *_objcg;
> >>  #endif
> >>  #ifdef CONFIG_MEM_ALLOC_PROFILING
> >> -	union codetag_ref _ctref;
> >> +		union codetag_ref _ctref;
> >>  #endif
> >> +	};
> >>  } __aligned(8);
> > 
> > slabobj_ext is now acting as a generic struct, serving as either _objcg or
> > _ctref. However, because it's set to __aligned(8), I'm wondering if this might
> > waste half the memory on 32-bit?
> 
> We could loosen the alignment to
> max(__NR_OBJEXTS_FLAGS, alignof(void *))
> 
> and drop OBJEXT_FLAG_UNUSED.

Yes, this idea is insightful and I think it may need a standalone patch without
blocking current patchset.

> 
> ...not sure how much we care about this on 32bit though.

Yeah, 32bit is a rare arch today :)

-- 
Thanks,
Hao