Re: [PATCH] mm: memcg: remove direct use of __memcg_kmem_uncharge_page

[Shakeel Butt <shakeelb@google.com>]

On Wed, Dec 13, 2023 at 08:31:00AM -0800, Yosry Ahmed wrote:
> On Wed, Dec 13, 2023 at 8:27 AM Matthew Wilcox <willy@infradead.org> wrote:
> >
> > On Wed, Dec 13, 2023 at 08:24:04AM -0800, Yosry Ahmed wrote:
> > > I doubt an extra compound_head() will matter in that path, but if you
> > > feel strongly about it that's okay. It's a nice cleanup that's all.
> >
> > i don't even understand why you think it's a nice cleanup.
> 
> free_pages_prepare() is directly calling __memcg_kmem_uncharge_page()
> instead of memcg_kmem_uncharge_page(), and open-coding checks that
> already exist in both of them to avoid the unnecessary function call
> if possible. I think this should be the job of
> memcg_kmem_uncharge_page(), but it's currently missing the
> PageMemcgKmem() check (which is in __memcg_kmem_uncharge_page()).
> 
> So I think moving that check to the wrapper allows
> free_pages_prepare() to call memcg_kmem_uncharge_page() and without
> worrying about those memcg-specific checks.

There is a (performance) reason these open coded check are present in
page_alloc.c and that is very clear for __memcg_kmem_charge_page() but
not so much for __memcg_kmem_uncharge_page(). So, for uncharge path,
this seems ok. Now to resolve Willy's concern for the fork() path, I
think we can open code the checks there.

Willy, any concern with that approach?