Re: [PATCH v4 1/1] memcg/hugetlb: Add hugeTLB counters to memcg

[Joshua Hahn <joshua.hahnjy@gmail.com>]

On Wed, 13 Nov 2024 14:42:29 -0800 (PST) David Rientjes <rientjes@google.com> wrote:

Hi David,

Sorry for the late response on this thread. To be completely transparent
with you, I am not someone who inspects hugetlb usage on a regular
basis, so I may not have the most relevant insights when it comes to
how much utility there would be from breaking down the usage by size.

With that said, I believe that over the past couple of days, there have
been some responses on this thread regarding how others use hugetlb. As you
know, I share Johannes's opinion that if there are people who would benefit
from splitting up the hugetlb usage across different page sizes, it should
happen in the hugetlb controller. 

> On Mon, 11 Nov 2024, David Rientjes wrote:
> > While the patch may be minimal, this is solidifying a kernel API that 
> > users will start to count on.  Users who may be interested in their 
> > hugetlb usage may not have control over the configuration of their kernel?

This is a good point. With that said, I believe that this is an instance
of a feature where both of our proposed ideas can co-exist; we can have the
total hugetlb usage reported in memcg for now, and if there is a consensus 
/ majority that would like to see the breakdown as well, we can introduce
it in a future patch without breaking the utility of this patch.

To quickly address a potential concern of bloating the already large memcg
stat: including both the total and breakdown wouldn't be the first time
a stat and its breakdown are both included: there is a precedent with this
in slab_(un)reclaimable and slab. 

> > Does it make sense to provide a breakdown in memory.stat so that users can 
> > differentiate between mapping one 1GB hugetlb page and 512 2MB hugetlb 
> > pages, which are different global resources?
> > 
> > > It's true that this is the case as well for total hugeltb usage, but
> > > I felt that not including hugetlb memory usage in memory.stat when it
> > > is accounted by memory.current would cause confusion for the users
> > > not being able to see that memory.current = sum of memory.stat. On the
> > > other hand, seeing the breakdown of how much each hugetlb size felt more
> > > like an optimization, and not a solution that bridges a confusion.
> > > 
> > 
> > If broken down into hugetlb_2048kB and hugetlb_1048576kB on x86, for 
> > example, users could still do sum of memory.stat, no?>

This is true! I still think it would be nice to include the total anyways,
since for a lot of people who use this statistic (Nhat's response in this
thread and Shakeel's response in the v3 of this patch), all they want is
a quick check to see how much memory is being used by hugetlb so they can
reason about memory dynamics. Again, I think that if we are to include
a breakdown in a future patch, it can coexist with this one.

> Friendly ping on this, would there be any objections to splitting the 
> memory.stat metrics out to be per hugepage size?

Sorry for the late reponse again. I think that if you had examples of use
cases where having the differnt page sizes, it would help me better
understand a motivation for including the breakdown (I would be happy
to write the patch for the breakdown as well if there is a consensus!)

Thank you for your thoughts, have a great day!
Joshua