Re: [PATCH RFC 05/12] mm/slab: abstract slabobj_ext.objcg access

[Suren Baghdasaryan <surenb@google.com>]

On Wed, Jul 15, 2026 at 3:11 AM Vlastimil Babka (SUSE)
<vbabka@kernel.org> wrote:
>
> In preparation for changes to the structure, abstract access to the
> objcg field with a slab_obj_ext_objcgp() function.
> Rename the field to _objcg to make an unexpected direct access a compile
> error.
>
> No functional change intended.
>
> Signed-off-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>
> ---
>  mm/kfence/core.c |  2 +-
>  mm/memcontrol.c  | 23 +++++++++++++++--------
>  mm/slab.h        |  9 ++++++++-
>  mm/slub.c        |  2 +-
>  4 files changed, 25 insertions(+), 11 deletions(-)
>
> diff --git a/mm/kfence/core.c b/mm/kfence/core.c
> index 6577bd76954e..717e8baf7e5d 100644
> --- a/mm/kfence/core.c
> +++ b/mm/kfence/core.c
> @@ -1249,7 +1249,7 @@ void __kfence_free(void *addr)
>         struct kfence_metadata *meta = addr_to_metadata((unsigned long)addr);
>
>  #ifdef CONFIG_MEMCG
> -       KFENCE_WARN_ON(meta->obj_exts.objcg);
> +       KFENCE_WARN_ON(*slab_obj_ext_objcgp(&meta->obj_exts));
>  #endif
>         /*
>          * If the objects of the cache are SLAB_TYPESAFE_BY_RCU, defer freeing
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 4e427286a88a..6303a2b1a9d0 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -2865,6 +2865,7 @@ struct mem_cgroup *mem_cgroup_from_obj_slab(struct slab *slab, void *p)
>          */
>         unsigned long obj_exts;
>         struct slabobj_ext *obj_ext;
> +       struct obj_cgroup *objcg;
>
>         obj_exts = slab_obj_exts(slab);
>         if (!obj_exts)
> @@ -2872,9 +2873,8 @@ struct mem_cgroup *mem_cgroup_from_obj_slab(struct slab *slab, void *p)
>
>         get_slab_obj_exts(obj_exts);
>         obj_ext = slab_obj_ext(slab->slab_cache, slab, obj_exts, p);
> -       if (obj_ext->objcg) {
> -               struct obj_cgroup *objcg = obj_ext->objcg;
> -
> +       objcg = *slab_obj_ext_objcgp(obj_ext);
> +       if (objcg) {
>                 put_slab_obj_exts(obj_exts);
>                 return obj_cgroup_memcg(objcg);
>         }
> @@ -3577,6 +3577,7 @@ bool __memcg_slab_post_alloc_hook(struct kmem_cache *s, struct list_lru *lru,
>                 unsigned long obj_exts;
>                 struct slabobj_ext *obj_ext;
>                 struct obj_stock_pcp *stock;
> +               struct obj_cgroup **objcgp;
>
>                 slab = virt_to_slab(p[i]);
>
> @@ -3612,10 +3613,15 @@ bool __memcg_slab_post_alloc_hook(struct kmem_cache *s, struct list_lru *lru,
>                 unlock_stock(stock);
>
>                 obj_exts = slab_obj_exts(slab);
> +
>                 get_slab_obj_exts(obj_exts);
> +
>                 obj_ext = slab_obj_ext(s, slab, obj_exts, p[i]);
> +               objcgp = slab_obj_ext_objcgp(obj_ext);
> +
>                 obj_cgroup_get(objcg);
> -               obj_ext->objcg = objcg;
> +               *objcgp = objcg;
> +
>                 put_slab_obj_exts(obj_exts);
>         }
>
> @@ -3628,16 +3634,17 @@ void __memcg_slab_free_hook(struct kmem_cache *s, struct slab *slab,
>         size_t obj_size = obj_full_size(s);
>
>         for (int i = 0; i < objects; i++) {
> -               struct obj_cgroup *objcg;
> +               struct obj_cgroup **objcgp, *objcg;
>                 struct slabobj_ext *obj_ext;
>                 struct obj_stock_pcp *stock;
>
>                 obj_ext = slab_obj_ext(s, slab, obj_exts, p[i]);
> -               objcg = obj_ext->objcg;
> -               if (!objcg)
> +               objcgp = slab_obj_ext_objcgp(obj_ext);
> +               if (!*objcgp)
>                         continue;
>
> -               obj_ext->objcg = NULL;
> +               objcg = *objcgp;
> +               *objcgp = NULL;
>
>                 stock = trylock_stock();
>                 __refill_obj_stock(objcg, stock, obj_size, true);
> diff --git a/mm/slab.h b/mm/slab.h
> index 36d067d6e7c0..789bd292075f 100644
> --- a/mm/slab.h
> +++ b/mm/slab.h
> @@ -555,7 +555,7 @@ static inline bool need_kmalloc_no_objext(void)
>   */
>  struct slabobj_ext {
>  #ifdef CONFIG_MEMCG
> -       struct obj_cgroup *objcg;
> +       struct obj_cgroup *_objcg;
>  #endif
>  #ifdef CONFIG_MEM_ALLOC_PROFILING
>         union codetag_ref ref;
> @@ -661,6 +661,13 @@ slab_obj_ext(struct kmem_cache *s, struct slab *slab, unsigned long obj_exts,
>         return kasan_reset_tag(obj_ext);
>  }
>
> +#ifdef CONFIG_MEMCG
> +static inline struct obj_cgroup **slab_obj_ext_objcgp(struct slabobj_ext *obj_ext)

I understand why you need to return obj_cgroup** but maybe instead we
can have a separate setter function and just return a pointer here?
That would also help catch cases when someone wants to set
obj_ext._objcg that was optimized away (for that the setter would need
an additional parameter to identify whether we allocated optimized
objext vector).

> +{
> +       return &obj_ext->_objcg;
> +}
> +#endif
> +
>  int alloc_slab_obj_exts(struct slab *slab, struct kmem_cache *s,
>                         gfp_t gfp, unsigned int alloc_flags);
>
> diff --git a/mm/slub.c b/mm/slub.c
> index 5e3f53bcd0d3..48e10198a3ce 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -2523,7 +2523,7 @@ bool memcg_slab_post_charge(void *p, gfp_t flags)
>         if (obj_exts) {
>                 get_slab_obj_exts(obj_exts);
>                 obj_ext = slab_obj_ext(s, slab, obj_exts, p);
> -               if (unlikely(obj_ext->objcg)) {
> +               if (unlikely(*slab_obj_ext_objcgp(obj_ext))) {
>                         put_slab_obj_exts(obj_exts);
>                         return true;
>                 }
>
> --
> 2.55.0
>