Re: [PATCH v3 15/17] memcg: Convert mem_cgroup_from_obj_folio() to mem_cgroup_from_obj_slab()

[Vlastimil Babka <vbabka@suse.cz>]

On 10/24/25 22:44, Matthew Wilcox (Oracle) wrote:
> In preparation for splitting struct slab from struct page and struct
> folio, convert the pointer to a slab rather than a folio.  This means
> we can end up passing a NULL slab pointer to mem_cgroup_from_obj_slab()
> if the pointer is not to a page allocated to slab, and we handle that
> appropriately by returning NULL.
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>  mm/memcontrol.c | 36 +++++++++++++-----------------------
>  1 file changed, 13 insertions(+), 23 deletions(-)
> 
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 1a95049d8b88..c3ba53b40375 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -2558,38 +2558,28 @@ static inline void mod_objcg_mlstate(struct obj_cgroup *objcg,
>  }
>  
>  static __always_inline
> -struct mem_cgroup *mem_cgroup_from_obj_folio(struct folio *folio, void *p)
> +struct mem_cgroup *mem_cgroup_from_obj_slab(struct slab *slab, void *p)
>  {
>  	/*
>  	 * Slab objects are accounted individually, not per-page.
>  	 * Memcg membership data for each individual object is saved in
>  	 * slab->obj_exts.
>  	 */
> -	if (folio_test_slab(folio)) {
> -		struct slabobj_ext *obj_exts;
> -		struct slab *slab;
> -		unsigned int off;
> -
> -		slab = folio_slab(folio);
> -		obj_exts = slab_obj_exts(slab);
> -		if (!obj_exts)
> -			return NULL;
> +	struct slabobj_ext *obj_exts;
> +	unsigned int off;
>  
> -		off = obj_to_index(slab->slab_cache, slab, p);
> -		if (obj_exts[off].objcg)
> -			return obj_cgroup_memcg(obj_exts[off].objcg);
> +	if (!slab)
> +		return NULL;
>  
> +	obj_exts = slab_obj_exts(slab);
> +	if (!obj_exts)
>  		return NULL;
> -	}
>  
> -	/*
> -	 * 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);

The changelog should perhaps mention this removal so it doesn't give people
pause. I think the comment describes an impossible scenario, you probably
concluded the same. I'm not sure if it became impossible unnoticed at some
point, or was bogus since the beginning. I'll pretend I did not try git
blame that.

> +	off = obj_to_index(slab->slab_cache, slab, p);
> +	if (obj_exts[off].objcg)
> +		return obj_cgroup_memcg(obj_exts[off].objcg);
> +
> +	return NULL;
>  }
>  
>  /*
> @@ -2606,7 +2596,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);
>  }
>  
>  static struct obj_cgroup *__get_obj_cgroup_from_memcg(struct mem_cgroup *memcg)