On Thu, Oct 31, 2024 at 9:34 PM Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> wrote: > > On Thu, 31 Oct 2024 15:03:34 -0400 Joshua Hahn <joshua.hahnjy@xxxxxxxxx> 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@xxxxxxxxx/ > : [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
