On Wed, Oct 23, 2024 at 6:17 PM Shakeel Butt <shakeel.butt@xxxxxxxxx> wrote:
>
> On Wed, Oct 23, 2024 at 01:34:33PM GMT, Joshua Hahn wrote:
> > include/linux/mmzone.h | 3 +++
> > mm/hugetlb.c | 4 ++++
> > mm/memcontrol.c | 11 +++++++++++
> > mm/vmstat.c | 3 +++
> > 4 files changed, 21 insertions(+)
> >
> > diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
> > index 17506e4a2835..d3ba49a974b2 100644
> > --- a/include/linux/mmzone.h
> > +++ b/include/linux/mmzone.h
> > @@ -215,6 +215,9 @@ enum node_stat_item {
> > #ifdef CONFIG_NUMA_BALANCING
> > PGPROMOTE_SUCCESS, /* promote successfully */
> > PGPROMOTE_CANDIDATE, /* candidate pages to promote */
> > +#endif
> > +#ifdef CONFIG_HUGETLB_PAGE
> > + HUGETLB_B,
>
> As Yosry pointed out, this is in pages, not bytes. There is already
> functionality to display this bin ytes for the readers of the memory
> stats.
Ah I see. I misunderstood what the _B meant, I didn't realize that it's
used to denote what the units of accounting were, not for exporting. I just
checked the functions that Yosry mentioned, and it makes a lot more sense.
(I also should have known, the method that I'm using to do the accounting
is called "pages"_per_huge_page)
> Also you will need to update Documentation/admin-guide/cgroup-v2.rst to
> include the hugetlb stats.
Thank you for pointing this out, I'll update the doc to include hugetlb stats.
> > #endif
> > /* PGDEMOTE_*: pages demoted */
> > PGDEMOTE_KSWAPD,
> > diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> > index 190fa05635f4..055bc91858e4 100644
> > --- a/mm/hugetlb.c
> > +++ b/mm/hugetlb.c
> > @@ -1925,6 +1925,8 @@ 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);
> > + if (cgrp_dfl_root.flags & CGRP_ROOT_MEMORY_HUGETLB_ACCOUNTING)
> > + lruvec_stat_mod_folio(folio, HUGETLB_B, -pages_per_huge_page(h));
>
> Please note that by you are adding this stat not only in memcg but also
> in global and per-node vmstat. This check will break those interfaces
> when this mount option is not used. You only need the check at the
> charging time. The uncharging and stats update functions will do the
> right thing as they check memcg_data attached to the folio.
Sounds good. I'll go back to make sure which other parts of the code
I'm touching
with this patch. Thank you for your feedback!
Joshua
