This patch fully deprecates the mem_cgroup_{try, commit, cancel} charge
functions, as well as their hugetlb variants. Please note that this
patch relies on [1], which removes the last references (from memcg-v1)
to some of these functions.

Signed-off-by: Joshua Hahn <joshua.hahnjy@gmail.com>

[1] https://lore.kernel.org/linux-mm/20241025012304.2473312-1-shakeel.butt@linux.dev/

---
 include/linux/memcontrol.h | 22 -------------
 mm/memcontrol.c            | 65 ++------------------------------------
 2 files changed, 3 insertions(+), 84 deletions(-)

diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
index d90c1ac791f1..75f15c4efe73 100644
--- a/include/linux/memcontrol.h
+++ b/include/linux/memcontrol.h
@@ -649,8 +649,6 @@ static inline bool mem_cgroup_below_min(struct mem_cgroup *target,
 		page_counter_read(&memcg->memory);
 }
 
-void mem_cgroup_commit_charge(struct folio *folio, struct mem_cgroup *memcg);
-
 int __mem_cgroup_charge(struct folio *folio, struct mm_struct *mm, gfp_t gfp);
 
 /**
@@ -675,9 +673,6 @@ static inline int mem_cgroup_charge(struct folio *folio, struct mm_struct *mm,
 	return __mem_cgroup_charge(folio, mm, gfp);
 }
 
-int mem_cgroup_hugetlb_try_charge(struct mem_cgroup *memcg, gfp_t gfp,
-		long nr_pages);
-
 int mem_cgroup_charge_hugetlb(struct folio* folio, gfp_t gfp);
 
 int mem_cgroup_swapin_charge_folio(struct folio *folio, struct mm_struct *mm,
@@ -708,7 +703,6 @@ static inline void mem_cgroup_uncharge_folios(struct folio_batch *folios)
 	__mem_cgroup_uncharge_folios(folios);
 }
 
-void mem_cgroup_cancel_charge(struct mem_cgroup *memcg, unsigned int nr_pages);
 void mem_cgroup_replace_folio(struct folio *old, struct folio *new);
 void mem_cgroup_migrate(struct folio *old, struct folio *new);
 
@@ -1167,23 +1161,12 @@ static inline bool mem_cgroup_below_min(struct mem_cgroup *target,
 	return false;
 }
 
-static inline void mem_cgroup_commit_charge(struct folio *folio,
-		struct mem_cgroup *memcg)
-{
-}
-
 static inline int mem_cgroup_charge(struct folio *folio,
 		struct mm_struct *mm, gfp_t gfp)
 {
 	return 0;
 }
 
-static inline int mem_cgroup_hugetlb_try_charge(struct mem_cgroup *memcg,
-		gfp_t gfp, long nr_pages)
-{
-	return 0;
-}
-
 static inline int mem_cgroup_swapin_charge_folio(struct folio *folio,
 			struct mm_struct *mm, gfp_t gfp, swp_entry_t entry)
 {
@@ -1202,11 +1185,6 @@ static inline void mem_cgroup_uncharge_folios(struct folio_batch *folios)
 {
 }
 
-static inline void mem_cgroup_cancel_charge(struct mem_cgroup *memcg,
-		unsigned int nr_pages)
-{
-}
-
 static inline void mem_cgroup_replace_folio(struct folio *old,
 		struct folio *new)
 {
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 95ee77fe27af..17126d8d263d 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -2351,21 +2351,6 @@ int try_charge_memcg(struct mem_cgroup *memcg, gfp_t gfp_mask,
 	return 0;
 }
 
-/**
- * mem_cgroup_cancel_charge() - cancel an uncommitted try_charge() call.
- * @memcg: memcg previously charged.
- * @nr_pages: number of pages previously charged.
- */
-void mem_cgroup_cancel_charge(struct mem_cgroup *memcg, unsigned int nr_pages)
-{
-	if (mem_cgroup_is_root(memcg))
-		return;
-
-	page_counter_uncharge(&memcg->memory, nr_pages);
-	if (do_memsw_account())
-		page_counter_uncharge(&memcg->memsw, nr_pages);
-}
-
 static void commit_charge(struct folio *folio, struct mem_cgroup *memcg)
 {
 	VM_BUG_ON_FOLIO(folio_memcg_charged(folio), folio);
@@ -2379,18 +2364,6 @@ static void commit_charge(struct folio *folio, struct mem_cgroup *memcg)
 	folio->memcg_data = (unsigned long)memcg;
 }
 
-/**
- * mem_cgroup_commit_charge - commit a previously successful try_charge().
- * @folio: folio to commit the charge to.
- * @memcg: memcg previously charged.
- */
-void mem_cgroup_commit_charge(struct folio *folio, struct mem_cgroup *memcg)
-{
-	css_get(&memcg->css);
-	commit_charge(folio, memcg);
-	memcg1_commit_charge(folio, memcg);
-}
-
 static inline void __mod_objcg_mlstate(struct obj_cgroup *objcg,
 				       struct pglist_data *pgdat,
 				       enum node_stat_item idx, int nr)
@@ -4469,7 +4442,9 @@ static int charge_memcg(struct folio *folio, struct mem_cgroup *memcg,
 	if (ret)
 		goto out;
 
-	mem_cgroup_commit_charge(folio, memcg);
+	css_get(&memcg->css);
+	commit_charge(folio, memcg);
+	memcg1_commit_charge(folio, memcg);
 out:
 	return ret;
 }
@@ -4495,40 +4470,6 @@ bool memcg_accounts_hugetlb(void)
 #endif
 }
 
-/**
- * mem_cgroup_hugetlb_try_charge - try to charge the memcg for a hugetlb folio
- * @memcg: memcg to charge.
- * @gfp: reclaim mode.
- * @nr_pages: number of pages to charge.
- *
- * This function is called when allocating a huge page folio to determine if
- * the memcg has the capacity for it. It does not commit the charge yet,
- * as the hugetlb folio itself has not been obtained from the hugetlb pool.
- *
- * Once we have obtained the hugetlb folio, we can call
- * mem_cgroup_commit_charge() to commit the charge. If we fail to obtain the
- * folio, we should instead call mem_cgroup_cancel_charge() to undo the effect
- * of try_charge().
- *
- * Returns 0 on success. Otherwise, an error code is returned.
- */
-int mem_cgroup_hugetlb_try_charge(struct mem_cgroup *memcg, gfp_t gfp,
-			long nr_pages)
-{
-	/*
-	 * If hugetlb memcg charging is not enabled, do not fail hugetlb allocation,
-	 * 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) || !memcg_accounts_hugetlb())
-		return -EOPNOTSUPP;
-
-	if (try_charge(memcg, gfp, nr_pages))
-		return -ENOMEM;
-
-	return 0;
-}
-
 int mem_cgroup_charge_hugetlb(struct folio *folio, gfp_t gfp)
 {
 	struct mem_cgroup *memcg = get_mem_cgroup_from_current();
-- 
2.43.5

On Fri, Nov 8, 2024 at 1:30 PM Joshua Hahn <joshua.hahnjy@gmail.com> wrote:
>
> This patch fully deprecates the mem_cgroup_{try, commit, cancel} charge
> functions, as well as their hugetlb variants. Please note that this
> patch relies on [1], which removes the last references (from memcg-v1)
> to some of these functions.

Nit: We are not really "deprecating" them, we are removing them.
Deprecation is usually tied to user-visible APIs that we cannot just
remove, at least not right away. Please rephrase the subject and
commit log accordingly.

>
> Signed-off-by: Joshua Hahn <joshua.hahnjy@gmail.com>
>
> [1] https://lore.kernel.org/linux-mm/20241025012304.2473312-1-shakeel.butt@linux.dev/
>
> ---
>  include/linux/memcontrol.h | 22 -------------
>  mm/memcontrol.c            | 65 ++------------------------------------
>  2 files changed, 3 insertions(+), 84 deletions(-)
>
> diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
> index d90c1ac791f1..75f15c4efe73 100644
> --- a/include/linux/memcontrol.h
> +++ b/include/linux/memcontrol.h
> @@ -649,8 +649,6 @@ static inline bool mem_cgroup_below_min(struct mem_cgroup *target,
>                 page_counter_read(&memcg->memory);
>  }
>
> -void mem_cgroup_commit_charge(struct folio *folio, struct mem_cgroup *memcg);
> -
>  int __mem_cgroup_charge(struct folio *folio, struct mm_struct *mm, gfp_t gfp);
>
>  /**
> @@ -675,9 +673,6 @@ static inline int mem_cgroup_charge(struct folio *folio, struct mm_struct *mm,
>         return __mem_cgroup_charge(folio, mm, gfp);
>  }
>
> -int mem_cgroup_hugetlb_try_charge(struct mem_cgroup *memcg, gfp_t gfp,
> -               long nr_pages);
> -
>  int mem_cgroup_charge_hugetlb(struct folio* folio, gfp_t gfp);
>
>  int mem_cgroup_swapin_charge_folio(struct folio *folio, struct mm_struct *mm,
> @@ -708,7 +703,6 @@ static inline void mem_cgroup_uncharge_folios(struct folio_batch *folios)
>         __mem_cgroup_uncharge_folios(folios);
>  }
>
> -void mem_cgroup_cancel_charge(struct mem_cgroup *memcg, unsigned int nr_pages);
>  void mem_cgroup_replace_folio(struct folio *old, struct folio *new);
>  void mem_cgroup_migrate(struct folio *old, struct folio *new);
>
> @@ -1167,23 +1161,12 @@ static inline bool mem_cgroup_below_min(struct mem_cgroup *target,
>         return false;
>  }
>
> -static inline void mem_cgroup_commit_charge(struct folio *folio,
> -               struct mem_cgroup *memcg)
> -{
> -}
> -
>  static inline int mem_cgroup_charge(struct folio *folio,
>                 struct mm_struct *mm, gfp_t gfp)
>  {
>         return 0;
>  }
>
> -static inline int mem_cgroup_hugetlb_try_charge(struct mem_cgroup *memcg,
> -               gfp_t gfp, long nr_pages)
> -{
> -       return 0;
> -}
> -
>  static inline int mem_cgroup_swapin_charge_folio(struct folio *folio,
>                         struct mm_struct *mm, gfp_t gfp, swp_entry_t entry)
>  {
> @@ -1202,11 +1185,6 @@ static inline void mem_cgroup_uncharge_folios(struct folio_batch *folios)
>  {
>  }
>
> -static inline void mem_cgroup_cancel_charge(struct mem_cgroup *memcg,
> -               unsigned int nr_pages)
> -{
> -}
> -
>  static inline void mem_cgroup_replace_folio(struct folio *old,
>                 struct folio *new)
>  {
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 95ee77fe27af..17126d8d263d 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -2351,21 +2351,6 @@ int try_charge_memcg(struct mem_cgroup *memcg, gfp_t gfp_mask,
>         return 0;
>  }
>
> -/**
> - * mem_cgroup_cancel_charge() - cancel an uncommitted try_charge() call.
> - * @memcg: memcg previously charged.
> - * @nr_pages: number of pages previously charged.
> - */
> -void mem_cgroup_cancel_charge(struct mem_cgroup *memcg, unsigned int nr_pages)
> -{
> -       if (mem_cgroup_is_root(memcg))
> -               return;
> -
> -       page_counter_uncharge(&memcg->memory, nr_pages);
> -       if (do_memsw_account())
> -               page_counter_uncharge(&memcg->memsw, nr_pages);
> -}
> -
>  static void commit_charge(struct folio *folio, struct mem_cgroup *memcg)
>  {
>         VM_BUG_ON_FOLIO(folio_memcg_charged(folio), folio);
> @@ -2379,18 +2364,6 @@ static void commit_charge(struct folio *folio, struct mem_cgroup *memcg)
>         folio->memcg_data = (unsigned long)memcg;
>  }
>
> -/**
> - * mem_cgroup_commit_charge - commit a previously successful try_charge().
> - * @folio: folio to commit the charge to.
> - * @memcg: memcg previously charged.
> - */
> -void mem_cgroup_commit_charge(struct folio *folio, struct mem_cgroup *memcg)
> -{
> -       css_get(&memcg->css);
> -       commit_charge(folio, memcg);
> -       memcg1_commit_charge(folio, memcg);
> -}
> -
>  static inline void __mod_objcg_mlstate(struct obj_cgroup *objcg,
>                                        struct pglist_data *pgdat,
>                                        enum node_stat_item idx, int nr)
> @@ -4469,7 +4442,9 @@ static int charge_memcg(struct folio *folio, struct mem_cgroup *memcg,
>         if (ret)
>                 goto out;
>
> -       mem_cgroup_commit_charge(folio, memcg);
> +       css_get(&memcg->css);
> +       commit_charge(folio, memcg);
> +       memcg1_commit_charge(folio, memcg);
>  out:
>         return ret;
>  }
> @@ -4495,40 +4470,6 @@ bool memcg_accounts_hugetlb(void)
>  #endif
>  }
>
> -/**
> - * mem_cgroup_hugetlb_try_charge - try to charge the memcg for a hugetlb folio
> - * @memcg: memcg to charge.
> - * @gfp: reclaim mode.
> - * @nr_pages: number of pages to charge.
> - *
> - * This function is called when allocating a huge page folio to determine if
> - * the memcg has the capacity for it. It does not commit the charge yet,
> - * as the hugetlb folio itself has not been obtained from the hugetlb pool.
> - *
> - * Once we have obtained the hugetlb folio, we can call
> - * mem_cgroup_commit_charge() to commit the charge. If we fail to obtain the
> - * folio, we should instead call mem_cgroup_cancel_charge() to undo the effect
> - * of try_charge().
> - *
> - * Returns 0 on success. Otherwise, an error code is returned.
> - */
> -int mem_cgroup_hugetlb_try_charge(struct mem_cgroup *memcg, gfp_t gfp,
> -                       long nr_pages)
> -{
> -       /*
> -        * If hugetlb memcg charging is not enabled, do not fail hugetlb allocation,
> -        * 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) || !memcg_accounts_hugetlb())
> -               return -EOPNOTSUPP;
> -
> -       if (try_charge(memcg, gfp, nr_pages))
> -               return -ENOMEM;
> -
> -       return 0;
> -}
> -
>  int mem_cgroup_charge_hugetlb(struct folio *folio, gfp_t gfp)
>  {
>         struct mem_cgroup *memcg = get_mem_cgroup_from_current();
> --
> 2.43.5
>
>

On Fri, Nov 8, 2024 at 6:08 PM Yosry Ahmed <yosryahmed@google.com> wrote:
>
> On Fri, Nov 8, 2024 at 1:30 PM Joshua Hahn <joshua.hahnjy@gmail.com> wrote:
> >
> > This patch fully deprecates the mem_cgroup_{try, commit, cancel} charge
> > functions, as well as their hugetlb variants. Please note that this
> > patch relies on [1], which removes the last references (from memcg-v1)
> > to some of these functions.
>
> Nit: We are not really "deprecating" them, we are removing them.
> Deprecation is usually tied to user-visible APIs that we cannot just
> remove, at least not right away. Please rephrase the subject and
> commit log accordingly.
>
> >
> > Signed-off-by: Joshua Hahn <joshua.hahnjy@gmail.com>
> >
> > [1] https://lore.kernel.org/linux-mm/20241025012304.2473312-1-shakeel.butt@linux.dev/

Hi Yosry,

Thank you for letting me know. To be completely honest, I think
I have been misusing the word in that case. You are correct,
the goal was to try and not change any functionality from the
user perspective, so I think removing is a better word, as you
suggested. I will make this change in the v3!

Have a great day,
Joshua

On Fri, Nov 08, 2024 at 01:29:46PM -0800, Joshua Hahn wrote:
> This patch fully deprecates the mem_cgroup_{try, commit, cancel} charge
> functions, as well as their hugetlb variants. Please note that this
> patch relies on [1], which removes the last references (from memcg-v1)
> to some of these functions.
> 
> Signed-off-by: Joshua Hahn <joshua.hahnjy@gmail.com>
> 

Acked-by: Shakeel Butt <shakeel.butt@linux.dev>