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

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

On Thu, Oct 31, 2024 at 9:34 PM Andrew Morton <akpm@linux-foundation.org> wrote:
>
> On Thu, 31 Oct 2024 15:03:34 -0400 Joshua Hahn <joshua.hahnjy@gmail.com> wrote:
>
> > 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?
>
> No problem.
> : 3.  Implementation Details:
> : 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.
> :
> : With this said, there are 2 problems:
> : (a) They are still not reported in memory.stat, which means the
> :     disparity between the memcg reports are still there.
> : (b) We cannot reasonably expect users to enable the hugeTLB controller
> :     just for the sake of hugeTLB usage reporting, especially since
> :     they don't have any use for hugeTLB usage enforcing [2].
> :
> : [1] https://lore.kernel.org/all/20231006184629.155543-1-nphamcs@gmail.com/
> : [2] Of course, we can't make a new patch for every feature that can be
> :     duplicated. However, since the existing solution of enabling the
> :     hugeTLB controller is an imperfect solution that still leaves a
> :     discrepancy between memory.stat and memory.curent, I think that it
> :     is reasonable to isolate the feature in this case.
>

Hello Andrew,

Thank you for your help as always. I apologize for not being clear in my
original request -- the "With this said, there are 2 problems:" paragraph is
part of the 2nd section (2. We already have a hugeTLB controller...) So the
outline will be:

This patch introduces...

1. Why is this patch necessary?\n
Currently, memcg hugeTLB accounting...

Aside from the consistency between...

2. We already have a hugeTLB controller. Why not use that?\n
It is true that hugeTLB tracks...

With this said, there are 2 problems:\n
  (a) They are still not...
  (b) We cannot reasonably...

3. Implementation Details\n
In the alloc / free hugetlb functions, ...

The global counter is added because...

[1] https://lore.kernel.org/ ...
[2] Of course, we can't make a new patch...

Thank you for your patience. I promise that this is the last change
to the patch message, I apologize for the frequent requests for
modifications. I hope you have a great day!
Joshua