Re: [PATCH RFC 08/12] mm/slab: change struct slabobj_ext to a union

[Suren Baghdasaryan <surenb@google.com>]

On Wed, Jul 15, 2026 at 3:11 AM Vlastimil Babka (SUSE)
<vbabka@kernel.org> 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 an 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.
>
> static_obj_ext_size() returns the effective size of (0-2) struct
> slabobj_ext's, depending on the config. slab_obj_ext_size() is currently
> static as well, but takes a slab pointer so it can be made dynamic
> later. Replace all sizeof(slabobj_ext) usage with these.
>
> No functional change intended, the layout is still effectively static.
>
> Signed-off-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>

This is much cleaner than what I was preparing. Nicely done!

Reviewed-by: Suren Baghdasaryan <surenb@google.com>

> ---
>  mm/slab.h | 41 +++++++++++++++++++++++++++++++++--------
>  mm/slub.c | 17 +++++++++--------
>  2 files changed, 42 insertions(+), 16 deletions(-)
>
> diff --git a/mm/slab.h b/mm/slab.h
> index 3ad9777ad600..359ab8caf61e 100644
> --- a/mm/slab.h
> +++ b/mm/slab.h
> @@ -554,14 +554,34 @@ static inline bool need_kmalloc_no_objext(void)
>   * if MEMCG_DATA_OBJEXTS is set.
>   */
>  struct slabobj_ext {

Perhaps we should add a comment here stating that every element of
this union should be pointer-sized?

> +       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);
>
> +static inline size_t static_obj_ext_size(void)
> +{
> +       size_t sz = 0;
> +
> +       if (IS_ENABLED(CONFIG_MEMCG))
> +               sz += 1;
> +
> +       if (IS_ENABLED(CONFIG_MEM_ALLOC_PROFILING))
> +               sz += 1;
> +
> +       return sizeof(struct slabobj_ext) * sz;
> +}
> +
> +static inline size_t slab_obj_ext_size(struct slab *slab)
> +{
> +       return static_obj_ext_size();
> +}
> +
>  #ifdef CONFIG_SLAB_OBJ_EXT
>
>  /*
> @@ -650,17 +670,18 @@ slab_obj_ext(struct kmem_cache *s, struct slab *slab, unsigned long obj_exts,
>  {
>         struct slabobj_ext *obj_ext;
>         unsigned int index;
> +       unsigned int stride;
>
>         VM_WARN_ON_ONCE(obj_exts != slab_obj_exts(slab));
>
>         index = obj_to_index(s, slab, obj);
>
> -       if (!obj_exts_in_object(slab)) {
> -               obj_ext = ((struct slabobj_ext *)obj_exts) + index;
> -       } else {
> -               unsigned int stride = s->size;
> -               obj_ext = (struct slabobj_ext *)(obj_exts + index * stride);
> -       }
> +       if (!obj_exts_in_object(slab))
> +               stride = slab_obj_ext_size(slab);
> +       else
> +               stride = s->size;
> +
> +       obj_ext = (struct slabobj_ext *)(obj_exts + index * stride);
>
>         return kasan_reset_tag(obj_ext);
>  }
> @@ -668,6 +689,7 @@ slab_obj_ext(struct kmem_cache *s, struct slab *slab, unsigned long obj_exts,
>  #ifdef CONFIG_MEMCG
>  static inline struct obj_cgroup **slab_obj_ext_objcgp(struct slabobj_ext *obj_ext)
>  {
> +       /* if objcg exists, it's first, so we don't need to do anything */
>         return &obj_ext->_objcg;
>  }
>  #endif
> @@ -676,6 +698,9 @@ static inline struct obj_cgroup **slab_obj_ext_objcgp(struct slabobj_ext *obj_ex
>  static inline union codetag_ref *
>  slab_obj_ext_codetag_ref(struct slab *slab, struct slabobj_ext *obj_ext)
>  {
> +       if (IS_ENABLED(CONFIG_MEMCG))
> +               obj_ext += 1;
> +
>         return &obj_ext->_ctref;
>  }
>  #endif
> diff --git a/mm/slub.c b/mm/slub.c
> index 98a14e5842a2..dd15af8abd62 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -803,7 +803,7 @@ static inline bool need_slab_obj_exts(struct kmem_cache *s)
>
>  static inline unsigned int obj_exts_size_in_slab(struct slab *slab)
>  {
> -       return sizeof(struct slabobj_ext) * slab->objects;
> +       return slab_obj_ext_size(slab) * slab->objects;
>  }
>
>  static inline unsigned long obj_exts_offset_in_slab(struct kmem_cache *s,
> @@ -1199,7 +1199,7 @@ static void print_trailer(struct kmem_cache *s, struct slab *slab, u8 *p)
>         off += kasan_metadata_size(s, false);
>
>         if (obj_exts_in_object(slab))
> -               off += sizeof(struct slabobj_ext);
> +               off += slab_obj_ext_size(slab);
>
>         if (off != size_from_object(s))
>                 /* Beginning of the filler is the free pointer */
> @@ -1404,7 +1404,7 @@ static int check_pad_bytes(struct kmem_cache *s, struct slab *slab, u8 *p)
>         off += kasan_metadata_size(s, false);
>
>         if (obj_exts_in_object(slab))
> -               off += sizeof(struct slabobj_ext);
> +               off += slab_obj_ext_size(slab);
>
>         if (size_from_object(s) == off)
>                 return 1;
> @@ -2089,6 +2089,8 @@ static inline bool mark_failed_objexts_alloc(struct slab *slab)
>  static inline void handle_failed_objexts_alloc(struct slab *slab,
>                 unsigned long obj_exts, struct slabobj_ext *vec)
>  {
> +       unsigned int stride = slab_obj_ext_size(slab) / sizeof(*vec);
> +
>         /*
>          * If vector previously failed to allocate then we have live
>          * objects with no tag reference. Mark all references in this
> @@ -2101,7 +2103,7 @@ static inline void handle_failed_objexts_alloc(struct slab *slab,
>                 union codetag_ref *ref = slab_obj_ext_codetag_ref(slab, vec);
>
>                 set_codetag_empty(ref);
> -               vec++;
> +               vec += stride;
>         }
>  }
>
> @@ -2127,7 +2129,7 @@ int alloc_slab_obj_exts(struct slab *slab, struct kmem_cache *s,
>         unsigned long new_exts;
>         unsigned long old_exts;
>         struct slabobj_ext *vec;
> -       size_t sz = sizeof(struct slabobj_ext) * slab->objects;
> +       size_t sz = slab_obj_ext_size(slab) * slab->objects;
>
>         gfp &= ~OBJCGS_CLEAR_MASK;
>         /*
> @@ -2270,8 +2272,7 @@ static void alloc_slab_obj_exts_early(struct kmem_cache *s, struct slab *slab)
>
>                 get_slab_obj_exts(obj_exts);
>                 for_each_object(addr, s, slab_address(slab), slab->objects)
> -                       memset(kasan_reset_tag(addr) + offset, 0,
> -                              sizeof(struct slabobj_ext));
> +                       memset(kasan_reset_tag(addr) + offset, 0, slab_obj_ext_size(slab));
>                 put_slab_obj_exts(obj_exts);
>
>  #ifdef CONFIG_MEMCG
> @@ -7930,7 +7931,7 @@ static int calculate_sizes(struct kmem_cache_args *args, struct kmem_cache *s)
>         aligned_size = ALIGN(size, s->align);
>  #if defined(CONFIG_SLAB_OBJ_EXT) && defined(CONFIG_64BIT)
>         if (slab_args_unmergeable(args, s->flags) &&
> -                       (aligned_size - size >= sizeof(struct slabobj_ext)))
> +                       (aligned_size - size >= static_obj_ext_size()))
>                 s->flags |= SLAB_OBJ_EXT_IN_OBJ;
>  #endif
>         size = aligned_size;
>
> --
> 2.55.0
>