Re: [PATCH v3 06/13] mm/slab: abstract slabobj_ext.ref access

On 7/29/26 08:18, Harry Yoo wrote:
>> diff --git a/mm/slab.h b/mm/slab.h
>> index 8f352d9f4d91..fbad99da093c 100644
>> --- a/mm/slab.h
>> +++ b/mm/slab.h
>> @@ -726,6 +726,8 @@ static inline void slab_obj_ext_set_objcg(struct slabobj_ext *obj_ext,
>>  static inline union codetag_ref *
>>  slab_obj_ext_codetag_ref(struct slab *slab, struct slabobj_ext *obj_ext)
>>  {
>> +       VM_WARN_ON_ONCE(!slab_obj_ext_has_codetag());
>> +
>>         if (IS_ENABLED(CONFIG_MEMCG))
>>                 obj_ext += 1;
>>  
>> ... and for objcg we would need to start passing slab pointer to
>> slab_obj_ext_objcg() and slab_obj_ext_set_objcg(). Hmm...
> 
> But slab_obj_ext_codetag_ref() already takes the parameter to
> calculate offset. I think it's worth as it'll be optimized away on
> non-debug kerenels and there is no way to detect type confusion
> due to subtle errors in the objext layout.

OK, so updated to this. VM_WARN_ON_ONCE() added later.

From db9c3501c0f9747fda8fa92d4f640b6fac8050e0 Mon Sep 17 00:00:00 2001
From: "Vlastimil Babka (SUSE)" <[email protected]>
Date: Mon, 27 Jul 2026 14:53:59 +0200
Subject: [PATCH] mm/slab: abstract slabobj_ext.objcg access

In preparation for changes to the structure, abstract getting and
setting the objcg field with slab_obj_ext_objcg() and
slab_obj_ext_set_objcg().
Rename the field to _objcg to make an unexpected direct access a compile
error.

The helpers take a slab pointer, which is currently unused, but will be
used by a debug check later.
Since there is no slab pointer easily available in __kfence_free(), just
drop the debug check there. The whole memcg_kmem accounting in kfence is
to be removed later anyway.

Otherwise, no functional change intended.

Reviewed-by: Hao Li <[email protected]>
Reviewed-by: Suren Baghdasaryan <[email protected]>
Reviewed-by: Harry Yoo (Oracle) <[email protected]>
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Vlastimil Babka (SUSE) <[email protected]>
---
 mm/kfence/core.c |  3 ---
 mm/memcontrol.c  | 14 ++++++++------
 mm/slab.h        | 17 ++++++++++++++++-
 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..5608a37f2b4d 100644
--- a/mm/kfence/core.c
+++ b/mm/kfence/core.c
@@ -1248,9 +1248,6 @@ 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);
-#endif
 	/*
 	 * If the objects of the cache are SLAB_TYPESAFE_BY_RCU, defer freeing
 	 * the object, as the object page may be recycled for other-typed
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 4e427286a88a..68e98fb3350e 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_objcg(slab, obj_ext);
+	if (objcg) {
 		put_slab_obj_exts(obj_exts);
 		return obj_cgroup_memcg(objcg);
 	}
@@ -3614,8 +3614,10 @@ bool __memcg_slab_post_alloc_hook(struct kmem_cache *s, struct list_lru *lru,
 		obj_exts = slab_obj_exts(slab);
 		get_slab_obj_exts(obj_exts);
 		obj_ext = slab_obj_ext(s, slab, obj_exts, p[i]);
+
 		obj_cgroup_get(objcg);
-		obj_ext->objcg = objcg;
+		slab_obj_ext_set_objcg(slab, obj_ext, objcg);
+
 		put_slab_obj_exts(obj_exts);
 	}
 
@@ -3633,11 +3635,11 @@ void __memcg_slab_free_hook(struct kmem_cache *s, struct slab *slab,
 		struct obj_stock_pcp *stock;
 
 		obj_ext = slab_obj_ext(s, slab, obj_exts, p[i]);
-		objcg = obj_ext->objcg;
+		objcg = slab_obj_ext_objcg(slab, obj_ext);
 		if (!objcg)
 			continue;
 
-		obj_ext->objcg = NULL;
+		slab_obj_ext_set_objcg(slab, obj_ext, NULL);
 
 		stock = trylock_stock();
 		__refill_obj_stock(objcg, stock, obj_size, true);
diff --git a/mm/slab.h b/mm/slab.h
index 451b50b7f237..ea014311e29f 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;
@@ -662,6 +662,21 @@ 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_objcg(struct slab *slab, struct slabobj_ext *obj_ext)
+{
+	return obj_ext->_objcg;
+}
+
+static inline void
+slab_obj_ext_set_objcg(struct slab *slab, struct slabobj_ext *obj_ext,
+		       struct obj_cgroup *objcg)
+{
+	obj_ext->_objcg = 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 a74f1866c958..ed18860fa0fd 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -2526,7 +2526,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_objcg(slab, obj_ext))) {
 			put_slab_obj_exts(obj_exts);
 			return true;
 		}
-- 
2.55.0