Re: [PATCH v3 1/1] memcg/hugetlb: Adding hugeTLB counters to memcg

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

Hi Michal,

On Thu, Oct 31, 2024 at 4:12 AM Michal Hocko <mhocko@suse.com> wrote:
>
> I would also add the following
>
> The global counter is added because vmstats is the preferred framework
> for cgroup stats. It makes stat items consistent between global and
> cgroup. It provides a per-node breakdown as well which is useful. It
> avoids proliferating cgroup-specific hooks in generic MM code.
>
> Acked-by: Michal Hocko <mhocko@suse.com>
> Thanks!
> --
> Michal Hocko
> SUSE Labs

Thank you for your feedback and review. I think this makes sense,
I will add a new paragraph to the implementation details section!

Andrew -- I am sorry to ask again, but do you think you can replace
the 3rd section in the patch (3. Implementation Details) with the
following paragraphs? Thank you so much!

In the alloc / free hugetlb functions, we call lruvec_stat_mod_folio
regardless of whether memcg accounts hugetlb. mem_cgroup_commit_charge
which is called from alloc_hugetlb_folio will set memcg for the folio
only if the CGRP_ROOT_MEMORY_HUGETLB_ACCOUNTING cgroup mount option is
used, so lruvec_stat_mod_folio accounts per-memcg hugetlb counters
only if the feature is enabled. Regardless of whether memcg accounts
for hugetlb, the newly added global counter is updated and shown
in /proc/vmstat.

The global counter is added because vmstats is the preferred framework
for cgroup stats. It makes stat items consistent between global and
cgroups. It also provides a per-node breakdown, which is useful.
Because it does not use cgroup-specific hooks, we also keep generic
MM code separate from memcg code.

Thank you Johannes & Michal for your continued feedback and interest
in my work, and thank you Andrew for reviewing and allowing me to
fix the patch messages.

I hope you all have a great rest of your day!
Joshua