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

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

 



On Mon, Mar 13, 2023 at 12:44 PM Andrew Morton
<akpm@xxxxxxxxxxxxxxxxxxxx> wrote:
>
> On Mon, 13 Mar 2023 08:34:52 +0000 Yosry Ahmed <yosryahmed@xxxxxxxxxx> wrote:
>
> > From: Hugh Dickins <hughd@xxxxxxxxxx>
> >
> > In a kernel with added WARN_ON_ONCE(PageTail) in page_memcg_check(), we
> > observed a warning from page_cgroup_ino() when reading
> > /proc/kpagecgroup.
>
> If this is the only known situation in which page_memcg_check() is
> passed a tail page, why does page_memcg_check() have
>
>         if (PageTail(page))
>                 return NULL;
>
> ?  Can we remove this to simplify, streamline and clarify?

I guess it's a safety check so that we don't end up trying to cast a
tail page to a folio. My opinion is to go one step further and change
page_memcg_check() to do return the memcg of the head page, i.e:

static inline struct mem_cgroup *page_memcg_check(struct page *page)
{
    return folio_memcg_check(page_folio(page));
}

This makes it consistent with page_memcg(), and makes sure future
users are getting the "correct" memcg for whatever page they pass in.
I am interested to hear other folks' opinions here.

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.

>
>





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

  Powered by Linux