[PATCH] mm: memcg: fix slab over-accounting for in-object objcg metadata

From: Guopeng Zhang <[email protected]>

obj_full_size() adds the size of an obj_cgroup pointer to s->size to
account for slabobj_ext storage, which normally resides outside the
object.

Since commit a77d6d338685 ("mm/slab: place slabobj_ext metadata in
unused space within s->size"), slabobj_ext can reside in object padding
already covered by s->size. The extra charge then accounts for the same
memory twice.

The per-slab obj_exts_in_object bit introduced by commit b5bc35ace2c5
("mm/slab: replace slab.stride with obj_exts_in_object") identifies this
layout. Use it to omit the extra charge only for in-object metadata.
Unlike in-object metadata, slab-leftover and separately allocated metadata
are not covered by the per-object s->size charge, so retain the existing
charge for those cases.

Reported-by: Ran Xiaokai <[email protected]>
Link: https://lore.kernel.org/all/[email protected]/
Fixes: a77d6d338685 ("mm/slab: place slabobj_ext metadata in unused space within s->size")
Signed-off-by: Guopeng Zhang <[email protected]>
---
 mm/memcontrol.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index d1312441a02b..aef480f46891 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -3526,12 +3526,16 @@ void obj_cgroup_uncharge(struct obj_cgroup *objcg, size_t size)
 	refill_obj_stock(objcg, size, true);
 }
 
-static inline size_t obj_full_size(struct kmem_cache *s)
+static inline size_t obj_full_size(struct kmem_cache *s, struct slab *slab)
 {
 	/*
 	 * For each accounted object there is an extra space which is used
-	 * to store obj_cgroup membership. Charge it too.
+	 * to store obj_cgroup membership. Charge it too, unless it is stored
+	 * in object padding already covered by s->size.
 	 */
+	if (obj_exts_in_object(slab))
+		return s->size;
+
 	return s->size + sizeof(struct obj_cgroup *);
 }
 
@@ -3539,7 +3543,6 @@ bool __memcg_slab_post_alloc_hook(struct kmem_cache *s, struct list_lru *lru,
 				  gfp_t flags, unsigned int slab_alloc_flags,
 				  size_t size, void **p)
 {
-	size_t obj_size = obj_full_size(s);
 	struct obj_cgroup *objcg;
 	struct slab *slab;
 	size_t i;
@@ -3580,6 +3583,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;
+		size_t obj_size;
 
 		slab = virt_to_slab(p[i]);
 
@@ -3590,6 +3594,8 @@ bool __memcg_slab_post_alloc_hook(struct kmem_cache *s, struct list_lru *lru,
 				continue;
 		}
 
+		obj_size = obj_full_size(s, slab);
+
 		/*
 		 * if we fail and size is 1, memcg_alloc_abort_single() will
 		 * just free the object, which is ok as we have not assigned
@@ -3632,7 +3638,7 @@ bool __memcg_slab_post_alloc_hook(struct kmem_cache *s, struct list_lru *lru,
 void __memcg_slab_free_hook(struct kmem_cache *s, struct slab *slab,
 			    void **p, int objects, unsigned long obj_exts)
 {
-	size_t obj_size = obj_full_size(s);
+	size_t obj_size = obj_full_size(s, slab);
 
 	for (int i = 0; i < objects; i++) {
 		struct obj_cgroup *objcg;

base-commit: e7f630142df2afccce90555e4972e60008222311
-- 
2.43.0