Hi Yosry, thank you for taking the time to review my patch. It seems
like I made a
lot of silly spelling / grammar / style mistakes in this patch, I'll
be more mindful of
these in the future.
On Wed, Oct 23, 2024 at 5:15 PM Yosry Ahmed <yosryahmed@xxxxxxxxxx> wrote:
>
> Hi Joshua,
> [...]
> > 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,
>
> Why '_B'?
I added _B because I am measuring the statistics in bytes (not pages).
IIRC some stat items use _B at the end to denote that the unit is in bytes,
so I put it at the end in the spirit of maintaining consistency. However, if
is not the case, I will remove it in the next version.
> [...]
> > #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)
>
> I think we already have a couple of these checks and this patch adds a
> few more, perhaps we should add a helper at this point to improve
> readability? Maybe something like memcg_accounts_hugetlb()?
> [...]
As far as I can tell, these checks only come up in mem_cgroup_hugetlb_try_charge
and cgroup_show_options. Shakeel already requested a cleanup of the try charge
function in the v1 thread, so I think the best course of action is to
add the helper
function in this patch (series) and use that helper function in
another patch series to
clean up the remaining functions.
I'll be sure to add the other grammar / style changes that you
mentioned above in
v3 as well. Thank you again for your feedback!
Joshua
