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

[Vlastimil Babka <vbabka@suse.cz>]

On 11/13/25 17:14, 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.
>> -	 */

I've only noticed this comment and thought it describes something impossible
to happen (maybe it was possible in the past) so seemed fine to delete it.

>> -	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.

Didn't notice this one.

> E.g. !vmap kernel stack pages -> mod_lruvec_kmem_state -> mem_cgroup_from_obj_slab
> 
> 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);

page_memcg_check() maybe instead? we shouldn't get a tail page here, no?