On Wed, Oct 30, 2024 at 04:27:37PM +0100, Michal Hocko wrote: > On Wed 30-10-24 11:01:02, Johannes Weiner wrote: > > On Wed, Oct 30, 2024 at 12:35:25PM +0100, Michal Hocko wrote: > > > On Mon 28-10-24 14:05:05, Joshua Hahn wrote: > > > [...] > > > > Changelog > > > > v3: > > > > * Removed check for whether CGRP_ROOT_HUGETLB_ACCOUNTING is on, since > > > > this check is already handled by lruvec_stat_mod (and doing the > > > > check in hugetlb.c actually breaks the build if MEMCG is not > > > > enabled. > > > [...] > > > > diff --git a/mm/hugetlb.c b/mm/hugetlb.c > > > > index 190fa05635f4..fbb10e52d7ea 100644 > > > > --- a/mm/hugetlb.c > > > > +++ b/mm/hugetlb.c > > > > @@ -1925,6 +1925,7 @@ void free_huge_folio(struct folio *folio) > > > > pages_per_huge_page(h), folio); > > > > hugetlb_cgroup_uncharge_folio_rsvd(hstate_index(h), > > > > pages_per_huge_page(h), folio); > > > > + lruvec_stat_mod_folio(folio, NR_HUGETLB, -pages_per_huge_page(h)); > > > > mem_cgroup_uncharge(folio); > > > > if (restore_reserve) > > > > h->resv_huge_pages++; > > > > @@ -3093,6 +3094,7 @@ struct folio *alloc_hugetlb_folio(struct vm_area_struct *vma, > > > > > > > > if (!memcg_charge_ret) > > > > mem_cgroup_commit_charge(folio, memcg); > > > > + lruvec_stat_mod_folio(folio, NR_HUGETLB, pages_per_huge_page(h)); > > > > mem_cgroup_put(memcg); > > > > > > > > return folio; > > > > > > I do not see any specific checks for CGRP_ROOT_MEMORY_HUGETLB_ACCOUNTING > > > in these paths. I guess you wanted to say that you rely on > > > mem_cgroup_commit_charge setting memcg pointer which then __lruvec_stat_mod_folio > > > relies on when updating stats. > > > > Yes, this is what Shakeel pointed out here: > > > > https://lore.kernel.org/lkml/il346o3nahawquum3t5rzcuuntkdpyahidpm2ctmdibj3td7pm@2aqirlm5hrdh/ > > It belongs to the changelog. Joshua, can you please include something like this at the end: lruvec_stat_mod_folio() keys off of folio->memcg linkage, which is only set up if CGRP_ROOT_MEMORY_HUGETLB_ACCOUNTING is switched on. This ensures that memory.stat::hugetlb is in sync with the hugetlb share of memory.current.
