Re: [PATCH v2 23/33] mm/memcg: Convert slab objcgs from struct page to struct slab

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 12/14/21 15:43, Johannes Weiner wrote:
> On Wed, Dec 01, 2021 at 07:15:00PM +0100, Vlastimil Babka wrote:
>> page->memcg_data is used with MEMCG_DATA_OBJCGS flag only for slab pages
>> so convert all the related infrastructure to struct slab.
>> 
>> To avoid include cycles, move the inline definitions of slab_objcgs() and
>> slab_objcgs_check() from memcontrol.h to mm/slab.h.
>> 
>> This is not just mechanistic changing of types and names. Now in
>> mem_cgroup_from_obj() we use PageSlab flag to decide if we interpret the page
>> as slab, instead of relying on MEMCG_DATA_OBJCGS bit checked in
>> page_objcgs_check() (now slab_objcgs_check()). Similarly in
>> memcg_slab_free_hook() where we can encounter kmalloc_large() pages (here the
>> PageSlab flag check is implied by virt_to_slab()).
> 
> Yup, this is great.
> 
>> @@ -2865,24 +2865,31 @@ int memcg_alloc_page_obj_cgroups(struct page *page, struct kmem_cache *s,
>>   */
>>  struct mem_cgroup *mem_cgroup_from_obj(void *p)
>>  {
>> -	struct page *page;
>> +	struct folio *folio;
>>  
>>  	if (mem_cgroup_disabled())
>>  		return NULL;
>>  
>> -	page = virt_to_head_page(p);
>> +	folio = virt_to_folio(p);
>>  
>>  	/*
>>  	 * Slab objects are accounted individually, not per-page.
>>  	 * Memcg membership data for each individual object is saved in
>>  	 * the page->obj_cgroups.
>>  	 */
>> -	if (page_objcgs_check(page)) {
>> +	if (folio_test_slab(folio)) {
>> +		struct obj_cgroup **objcgs;
>>  		struct obj_cgroup *objcg;
>> +		struct slab *slab;
>>  		unsigned int off;
>>  
>> -		off = obj_to_index(page->slab_cache, page_slab(page), p);
>> -		objcg = page_objcgs(page)[off];
>> +		slab = folio_slab(folio);
>> +		objcgs = slab_objcgs_check(slab);
> 
> AFAICS the change to the _check() variant was accidental.
> 
> folio_test_slab() makes sure it's a slab page, so the legit options
> for memcg_data are NULL or |MEMCG_DATA_OBJCGS; using slab_objcgs()
> here would include the proper asserts, like page_objcgs() used to.

Well spotted. On closer look, it's actually the same in
memcg_slab_free_hook() where I also added a folio_test_slab() as part of
virt_to_slab(). In fact it means page_objcgs_check() can be just deleted
instead of replaced by slab_objcgs_check(). Thanks!





[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]     [Monitors]

  Powered by Linux