| Message ID | 20241108212946.2642085-2-joshua.hahnjy@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | memcg/hugetlb: Rework memcg hugetlb charging | expand |
On Fri, Nov 08, 2024 at 01:29:44PM -0800, Joshua Hahn wrote: > This patch isolates the check for whether memcg accounts hugetlb. > This condition can only be true if the memcg mount option > memory_hugetlb_accounting is on, which includes hugetlb usage > in memory.current. > > Signed-off-by: Joshua Hahn <joshua.hahnjy@gmail.com> > > --- > mm/memcontrol.c | 17 ++++++++++++++--- > 1 file changed, 14 insertions(+), 3 deletions(-) > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index f3a9653cef0e..97f63ec9c9fb 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -1425,6 +1425,9 @@ unsigned long memcg_page_state_local_output(struct mem_cgroup *memcg, int item) > memcg_page_state_output_unit(item); > } > > +/* Forward declaration */ > +bool memcg_accounts_hugetlb(void); No need for forward declaration. Just define it here and make it static. > + > static void memcg_stat_format(struct mem_cgroup *memcg, struct seq_buf *s) > { > int i; > @@ -1446,7 +1449,7 @@ static void memcg_stat_format(struct mem_cgroup *memcg, struct seq_buf *s) > > #ifdef CONFIG_HUGETLB_PAGE > if (unlikely(memory_stats[i].idx == NR_HUGETLB) && > - !(cgrp_dfl_root.flags & CGRP_ROOT_MEMORY_HUGETLB_ACCOUNTING)) > + !memcg_accounts_hugetlb()) > continue; > #endif > size = memcg_page_state_output(memcg, memory_stats[i].idx); > @@ -4483,6 +4486,15 @@ int __mem_cgroup_charge(struct folio *folio, struct mm_struct *mm, gfp_t gfp) > return ret; > } > > +bool memcg_accounts_hugetlb(void) > +{ > +#ifdef CONFIG_HUGETLB_PAGE > + return cgrp_dfl_root.flags & CGRP_ROOT_MEMORY_HUGETLB_ACCOUNTING; > +#else > + return false; > +#endif > +} > + > /** > * mem_cgroup_hugetlb_try_charge - try to charge the memcg for a hugetlb folio > * @memcg: memcg to charge. > @@ -4508,8 +4520,7 @@ int mem_cgroup_hugetlb_try_charge(struct mem_cgroup *memcg, gfp_t gfp, > * but do not attempt to commit charge later (or cancel on error) either. > */ > if (mem_cgroup_disabled() || !memcg || > - !cgroup_subsys_on_dfl(memory_cgrp_subsys) || > - !(cgrp_dfl_root.flags & CGRP_ROOT_MEMORY_HUGETLB_ACCOUNTING)) > + !cgroup_subsys_on_dfl(memory_cgrp_subsys) || !memcg_accounts_hugetlb()) > return -EOPNOTSUPP; > > if (try_charge(memcg, gfp, nr_pages)) > -- > 2.43.5 >
On Fri, Nov 8, 2024 at 2:21 PM Shakeel Butt <shakeel.butt@linux.dev> wrote: > > On Fri, Nov 08, 2024 at 01:29:44PM -0800, Joshua Hahn wrote: > > This patch isolates the check for whether memcg accounts hugetlb. > > This condition can only be true if the memcg mount option > > memory_hugetlb_accounting is on, which includes hugetlb usage > > in memory.current. > > > > Signed-off-by: Joshua Hahn <joshua.hahnjy@gmail.com> > > > > --- > > mm/memcontrol.c | 17 ++++++++++++++--- > > 1 file changed, 14 insertions(+), 3 deletions(-) > > > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > > index f3a9653cef0e..97f63ec9c9fb 100644 > > --- a/mm/memcontrol.c > > +++ b/mm/memcontrol.c > > @@ -1425,6 +1425,9 @@ unsigned long memcg_page_state_local_output(struct mem_cgroup *memcg, int item) > > memcg_page_state_output_unit(item); > > } > > > > +/* Forward declaration */ > > +bool memcg_accounts_hugetlb(void); > > No need for forward declaration. Just define it here and make it static. Also please pull the #ifdef outside the function definition, e.g. #ifdef CONFIG_HUGETLB_PAGE static bool memcg_accounts_hugetlb(void) { return cgrp_dfl_root.flags & CGRP_ROOT_MEMORY_HUGETLB_ACCOUNTING; } #else /* CONFIG_HUGETLB_PAGE */ static bool memcg_accounts_hugetlb(void) { return false; } { return false; } #endif /* CONFIG_HUGETLB_PAGE */ > > > + > > static void memcg_stat_format(struct mem_cgroup *memcg, struct seq_buf *s) > > { > > int i; > > @@ -1446,7 +1449,7 @@ static void memcg_stat_format(struct mem_cgroup *memcg, struct seq_buf *s) > > > > #ifdef CONFIG_HUGETLB_PAGE > > if (unlikely(memory_stats[i].idx == NR_HUGETLB) && > > - !(cgrp_dfl_root.flags & CGRP_ROOT_MEMORY_HUGETLB_ACCOUNTING)) > > + !memcg_accounts_hugetlb()) > > continue; > > #endif > > size = memcg_page_state_output(memcg, memory_stats[i].idx); > > @@ -4483,6 +4486,15 @@ int __mem_cgroup_charge(struct folio *folio, struct mm_struct *mm, gfp_t gfp) > > return ret; > > } > > > > +bool memcg_accounts_hugetlb(void) > > +{ > > +#ifdef CONFIG_HUGETLB_PAGE > > + return cgrp_dfl_root.flags & CGRP_ROOT_MEMORY_HUGETLB_ACCOUNTING; > > +#else > > + return false; > > +#endif > > +} > > + > > /** > > * mem_cgroup_hugetlb_try_charge - try to charge the memcg for a hugetlb folio > > * @memcg: memcg to charge. > > @@ -4508,8 +4520,7 @@ int mem_cgroup_hugetlb_try_charge(struct mem_cgroup *memcg, gfp_t gfp, > > * but do not attempt to commit charge later (or cancel on error) either. > > */ > > if (mem_cgroup_disabled() || !memcg || > > - !cgroup_subsys_on_dfl(memory_cgrp_subsys) || > > - !(cgrp_dfl_root.flags & CGRP_ROOT_MEMORY_HUGETLB_ACCOUNTING)) > > + !cgroup_subsys_on_dfl(memory_cgrp_subsys) || !memcg_accounts_hugetlb()) > > return -EOPNOTSUPP; > > > > if (try_charge(memcg, gfp, nr_pages)) > > -- > > 2.43.5 > > >
diff --git a/mm/memcontrol.c b/mm/memcontrol.c index f3a9653cef0e..97f63ec9c9fb 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -1425,6 +1425,9 @@ unsigned long memcg_page_state_local_output(struct mem_cgroup *memcg, int item) memcg_page_state_output_unit(item); } +/* Forward declaration */ +bool memcg_accounts_hugetlb(void); + static void memcg_stat_format(struct mem_cgroup *memcg, struct seq_buf *s) { int i; @@ -1446,7 +1449,7 @@ static void memcg_stat_format(struct mem_cgroup *memcg, struct seq_buf *s) #ifdef CONFIG_HUGETLB_PAGE if (unlikely(memory_stats[i].idx == NR_HUGETLB) && - !(cgrp_dfl_root.flags & CGRP_ROOT_MEMORY_HUGETLB_ACCOUNTING)) + !memcg_accounts_hugetlb()) continue; #endif size = memcg_page_state_output(memcg, memory_stats[i].idx); @@ -4483,6 +4486,15 @@ int __mem_cgroup_charge(struct folio *folio, struct mm_struct *mm, gfp_t gfp) return ret; } +bool memcg_accounts_hugetlb(void) +{ +#ifdef CONFIG_HUGETLB_PAGE + return cgrp_dfl_root.flags & CGRP_ROOT_MEMORY_HUGETLB_ACCOUNTING; +#else + return false; +#endif +} + /** * mem_cgroup_hugetlb_try_charge - try to charge the memcg for a hugetlb folio * @memcg: memcg to charge. @@ -4508,8 +4520,7 @@ int mem_cgroup_hugetlb_try_charge(struct mem_cgroup *memcg, gfp_t gfp, * but do not attempt to commit charge later (or cancel on error) either. */ if (mem_cgroup_disabled() || !memcg || - !cgroup_subsys_on_dfl(memory_cgrp_subsys) || - !(cgrp_dfl_root.flags & CGRP_ROOT_MEMORY_HUGETLB_ACCOUNTING)) + !cgroup_subsys_on_dfl(memory_cgrp_subsys) || !memcg_accounts_hugetlb()) return -EOPNOTSUPP; if (try_charge(memcg, gfp, nr_pages))
This patch isolates the check for whether memcg accounts hugetlb. This condition can only be true if the memcg mount option memory_hugetlb_accounting is on, which includes hugetlb usage in memory.current. Signed-off-by: Joshua Hahn <joshua.hahnjy@gmail.com> --- mm/memcontrol.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-)