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

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

On Sat, Nov 9, 2024 at 9:19 PM David Rientjes <rientjes@google.com> wrote:
>
> On Fri, 1 Nov 2024, Joshua Hahn wrote:
>
> > diff --git a/Documentation/admin-guide/cgroup-v2.rst b/Documentation/admin-guide/cgroup-v2.rst
> > index 69af2173555f..bd7e81c2aa2b 100644
> > --- a/Documentation/admin-guide/cgroup-v2.rst
> > +++ b/Documentation/admin-guide/cgroup-v2.rst
> > @@ -1646,6 +1646,11 @@ The following nested keys are defined.
> >         pgdemote_khugepaged
> >               Number of pages demoted by khugepaged.
> >
> > +       hugetlb
> > +             Amount of memory used by hugetlb pages. This metric only shows
> > +             up if hugetlb usage is accounted for in memory.current (i.e.
> > +             cgroup is mounted with the memory_hugetlb_accounting option).
> > +
> >    memory.numa_stat
> >       A read-only nested-keyed file which exists on non-root cgroups.
> >
>
> Definitely makes sense to include this.
>
> Any reason to not account different hugetlb page sizes separately in this
> stat, however?  IOW, should there be separate hugetlb_2048kB and
> hugetlb_1048576kB stats on x86?

Hello David, Thank you for reviewing my patch!

The reason that I opted not to include a breakdown of each hugetlb
size in memory.stat is only because I wanted to keep the addition that
this patch makes as minimal as possible, while still addressing
the goal of bridging the gap between memory.stat and memory.current.
Users who are curious about this breakdown can see how much memory
is used by each hugetlb size by enabling the hugetlb controller as well.

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.

I have not had a scenario where I had to look at the breakdown of the
hugetlb sizes (without the hugetlb controller), or a scenario where not
knowing this causes some sort of confusion. If others have had this
problem, I would love to hear about it, and perhaps work on a solution
that can address this point as well!

I hope you have a great day!
Joshua