Re: [PATCH v4 14/16] memcg: Convert mem_cgroup_from_obj_folio() to mem_cgroup_from_obj_slab()

[Matthew Wilcox <willy@infradead.org>]

On Thu, Nov 13, 2025 at 11:14:24AM -0500, Johannes Weiner wrote:
> On Thu, Nov 13, 2025 at 12:09:28AM +0000, Matthew Wilcox (Oracle) wrote:
> > -	/*
> > -	 * folio_memcg_check() is used here, because in theory we can encounter
> > -	 * a folio where the slab flag has been cleared already, but
> > -	 * slab->obj_exts has not been freed yet
> > -	 * folio_memcg_check() will guarantee that a proper memory
> > -	 * cgroup pointer or NULL will be returned.
> > -	 */
> > -	return folio_memcg_check(folio);
> > +	off = obj_to_index(slab->slab_cache, slab, p);
> > +	if (obj_exts[off].objcg)
> > +		return obj_cgroup_memcg(obj_exts[off].objcg);
> > +
> > +	return NULL;
> >  }
> >  
> >  /*
> > @@ -2637,7 +2627,7 @@ struct mem_cgroup *mem_cgroup_from_slab_obj(void *p)
> >  	if (mem_cgroup_disabled())
> >  		return NULL;
> >  
> > -	return mem_cgroup_from_obj_folio(virt_to_folio(p), p);
> > +	return mem_cgroup_from_obj_slab(virt_to_slab(p), p);
> 
> The name undoubtedly sucks, but there is a comment above this function
> that this can be used on non-slab kernel pages as well.

Oh, I see.  Usercopy calls this kind of thing 'heap object', so
perhaps eventually rename this function to mem_cgroup_from_heap_obj()?

> E.g. !vmap kernel stack pages -> mod_lruvec_kmem_state -> mem_cgroup_from_obj_slab

That actually seems to be the only user ... and I wasn't testing on a
!VMAP_STACK build, so I wouldn't've caught it.

> How about:
> 
> 	if ((slab = virt_to_slap(p)))
> 		return mem_cgroup_from_obj_slab(slab, p);
> 	return folio_memcg_check(virt_to_folio(p), p);

Mild updates, here's my counteroffer:

commit 6ca8243530e4
Author: Matthew Wilcox (Oracle) <willy@infradead.org>
Date:   Thu Nov 13 11:30:59 2025 -0500

    fix-memcg-slab

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 597673e272a9..a2e6f409c5e8 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -2599,9 +2599,6 @@ struct mem_cgroup *mem_cgroup_from_obj_slab(struct slab *slab, void *p)
 	struct slabobj_ext *obj_exts;
 	unsigned int off;
 
-	if (!slab)
-		return NULL;
-
 	obj_exts = slab_obj_exts(slab);
 	if (!obj_exts)
 		return NULL;
@@ -2624,10 +2621,15 @@ struct mem_cgroup *mem_cgroup_from_obj_slab(struct slab *slab, void *p)
  */
 struct mem_cgroup *mem_cgroup_from_slab_obj(void *p)
 {
+	struct slab *slab;
+
 	if (mem_cgroup_disabled())
 		return NULL;
 
-	return mem_cgroup_from_obj_slab(virt_to_slab(p), p);
+	slab = virt_to_slab(p);
+	if (slab)
+		return mem_cgroup_from_obj_slab(slab, p);
+	return folio_memcg_check(virt_to_folio(p));
 }
 
 static struct obj_cgroup *__get_obj_cgroup_from_memcg(struct mem_cgroup *memcg)