Re: [PATCH RFC 12/12] mm/slab: stop allocating objcg pointers when unnecessary

On Wed, Jul 15, 2026 at 3:11 AM Vlastimil Babka (SUSE)
<[email protected]> wrote:
>
> Start using the slab_needs_objcg() helper to calculate slabobj_ext size.
> Caches that we know to never need objcg pointers (currently
> KMALLOC_NORMAL caches) will thus stop wasting memory on them when memory
> allocation profiling is enabled.
>
> For things to work properly, we need to also add slab_needs_objcg()
> checks to mem_cgroup_from_obj_slab() and memcg_slab_free_hook(), because
> when obj_exts array exists for a slab only due to mem_alloc profiling,
> we would otherwise attempt to access a non-existing objcg pointer in
> that slab.
>
> The function __memcg_slab_post_alloc_hook() should not be possible to
> call for a slab where slab_needs_objcg() is false, but add a DEBUG_VM
> check there to prevent breaking this assumption accidentally.
>
> Signed-off-by: Vlastimil Babka (SUSE) <[email protected]>

The change looks nice and simple. I'll take some more time to check if
we missed anything.
Only one nit below.

> ---
>  mm/memcontrol.c |  6 ++++++
>  mm/slab.h       | 12 ++++++++++--
>  mm/slub.c       |  3 +++
>  3 files changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 6303a2b1a9d0..09659722ec85 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -2871,6 +2871,9 @@ struct mem_cgroup *mem_cgroup_from_obj_slab(struct slab *slab, void *p)
>         if (!obj_exts)
>                 return NULL;
>
> +       if (!slab_needs_objcg(slab))
> +               return NULL;
> +
>         get_slab_obj_exts(obj_exts);
>         obj_ext = slab_obj_ext(slab->slab_cache, slab, obj_exts, p);
>         objcg = *slab_obj_ext_objcgp(obj_ext);
> @@ -3581,6 +3584,9 @@ bool __memcg_slab_post_alloc_hook(struct kmem_cache *s, struct list_lru *lru,
>
>                 slab = virt_to_slab(p[i]);
>
> +               if (IS_ENABLED(CONFIG_DEBUG_VM) && WARN_ON_ONCE(!slab_needs_objcg(slab)))
> +                       continue;
> +
>                 if (!slab_obj_exts(slab) &&
>                     alloc_slab_obj_exts(slab, s, flags, slab_alloc_flags)) {
>                         continue;
> diff --git a/mm/slab.h b/mm/slab.h
> index 948d075cdbef..072cc2506756 100644
> --- a/mm/slab.h
> +++ b/mm/slab.h
> @@ -622,7 +622,15 @@ static inline size_t static_obj_ext_size(void)
>
>  static inline size_t slab_obj_ext_size(struct slab *slab)
>  {
> -       return static_obj_ext_size();

Maybe now we should rename static_obj_ext_size() to
static_obj_ext_max_size() as it reflects the max possible size of
slabobj_ext?

> +       size_t sz = 0;
> +
> +       if (slab_needs_objcg(slab))
> +               sz += 1;
> +
> +       if (slab_obj_ext_has_codetag())
> +               sz += 1;
> +
> +       return sizeof(struct slabobj_ext) * sz;
>  }
>
>  #ifdef CONFIG_SLAB_OBJ_EXT
> @@ -741,7 +749,7 @@ 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))
> +       if (slab_needs_objcg(slab))
>                 obj_ext += 1;
>
>         return &obj_ext->_ctref;
> diff --git a/mm/slub.c b/mm/slub.c
> index 771d73abacb6..09c4931e5435 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -2503,6 +2503,9 @@ void memcg_slab_free_hook(struct kmem_cache *s, struct slab *slab, void **p,
>         if (likely(!obj_exts))
>                 return;
>
> +       if (!slab_needs_objcg(slab))
> +               return;
> +
>         get_slab_obj_exts(obj_exts);
>         __memcg_slab_free_hook(s, slab, p, objects, obj_exts);
>         put_slab_obj_exts(obj_exts);
>
> --
> 2.55.0
>