Re: [PATCH] memcg: page_cgroup_ino() get memcg from compound_head(page)

[Waiman Long <longman@redhat.com>]

On 3/14/23 23:10, Yosry Ahmed wrote:
>>> The only other user today is print_page_owner_memcg(). I am not sure
>>> if it's doing the right thing by explicitly reading page->memcg_data,
>>> but it is already excluding pages that have page->memcg_data == 0,
>>> which should be the case for tail pages.
>> It is reading memcg_data directly to see if it is slab cache page. It is
>> currently skipping page that does not have memcg_data set.
> IIUC this skips tail pages, because they should always have
> page->memcg_data == 0, even if they are charged to a memcg. To
> correctly get their memcg we should read it from the
> compound_head()/page_folio().
The purpose of that function is mainly to report pages that have a 
reference to a memcg, especially the dead one. So by counting the 
occurrence of a particular cgroup name, we can have a rough idea of 
that. So only head page has relevance here and we can skip the tail pages.
>
> My 2c, we can check PageSlab() to print the extra message for slab
> pages, instead of reading memcg_data directly, which kinda breaks the
> abstraction created by the various helpers for reading a page memcg.
> Someone can easily change something in how memcg_data is interpreted
> in those helpers without realizing that page_owner is also reading it.

You are right. We should be using a helper if available. I will send a 
patch to fix that.

Thanks,
Longman