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