Hi Michal, On Thu, Oct 31, 2024 at 4:12 AM Michal Hocko <mhocko@xxxxxxxx> 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@xxxxxxxx> > 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
