linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* [v3 PATCH 0/3] memcg/hugetlb: Rework memcg hugetlb charging
@ 2024-12-11 20:39 Joshua Hahn
  2024-12-11 20:39 ` [v3 PATCH 1/3] memcg/hugetlb: Introduce memcg_accounts_hugetlb Joshua Hahn
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Joshua Hahn @ 2024-12-11 20:39 UTC (permalink / raw)
  To: shakeel.butt
  Cc: hannes, mhocko, roman.gushchin, muchun.song, akpm, sj, cgroups,
	linux-mm, linux-kernel, kernel-team

This series cleans up memcg's hugetlb charging logic by deprecating the
current memcg hugetlb try-charge + {commit, cancel} logic present in
alloc_hugetlb_folio. A single function mem_cgroup_charge_hugetlb takes
its place instead. This makes the code more maintainable by simplifying
the error path and reduces memcg's footprint in hugetlb logic.

This patch introduces a few changes in the hugetlb folio allocation
error path:
(a) Instead of having multiple return points, we consolidate them to
    two: one for reaching the memcg limit or running out of memory
    (-ENOMEM) and one for hugetlb allocation fails / limit being
    reached (-ENOSPC).
(b) Previously, the memcg limit was checked before the folio is acquired,
    meaning the hugeTLB folio isn't acquired if the limit is reached.
    This patch performs the charging after the folio is reached, meaning
    if memcg's limit is reached, the acquired folio is freed right away.

This patch builds on two earlier patch series: [2] which adds memcg
hugeTLB counters, and [3] which deprecates charge moving and removes the
last references to mem_cgroup_cancel_charge. The request for this cleanup
can be found in [2].

Suggested-by: Shakeel Butt <shakeel.butt@linux.dev>
Signed-off-by: Joshua Hahn <joshua.hahnjy@gmail.com>

[1] https://lore.kernel.org/all/20231006184629.155543-1-nphamcs@gmail.com/
[2] https://lore.kernel.org/all/20241101204402.1885383-1-joshua.hahnjy@gmail.com/
[3] https://lore.kernel.org/linux-mm/20241025012304.2473312-1-shakeel.butt@linux.dev/

---
Changelog
v3:
  * Fixed build error where mem_cgroup_charge_hugetlb was defined inside
    an #ifdef CONFIG_MEMCG. It is now defined in the #else case as well.
  * memcg_accounts_hugetlb is no longer forward-declared, just
    statically defined earlier.
  * The #ifdef is moved outside memcg_accounts_hugetlb's definition.
  * lruvec_stat_mod_folio() is now called only except when memcg
    charging fails. That is, even when memcg is not supported or memcg
    does not account hugetlb, we still update the (global) stats.
    * For this to happen, memcg_charge_hugetlb no longer returns
      -EOPNOTSUPP, just returns 0 when it is not supported.
  * kernel-doc comment added for mem_cgroup_charge_hugetlb
  * s/deprecate/remove in commit messages for clarity.
  * Thank you Shakeel, Yosry, and SJ for your input!
v2:
  * Removed declaration of memcg_accounts_hugetlb from memcontrol.h
  * Moved second call to memcg_accounts_hugetlb from 2nd patch to 1st
  * Changed error behavior in alloc_hugetlb_folio: v1 included a bug
    that uncharged hugetlb_cgroup twice when memecg's limit was reached
  * mem_cgroup_charge_hugetlb no longer called with hugetlb_lock held
  * Moved mem_cgroup_hugetlb_{try, charge} deprecation to patch 3
  * mem_cgroup_charge_hugetlb always decrements memcg's refcount
  * Fully cleaned up mem_cgroup_{cancel,commit}_charge
  * Fixed typos

Joshua Hahn (3):
  memcg/hugetlb: Introduce memcg_accounts_hugetlb
  memcg/hugetlb: Introduce mem_cgroup_charge_hugetlb
  memcg/hugetlb: Remove memcg hugetlb try-commit-cancel protocol

 include/linux/memcontrol.h | 21 ++-------
 mm/hugetlb.c               | 35 ++++++---------
 mm/memcontrol.c            | 92 ++++++++++++++++----------------------
 3 files changed, 56 insertions(+), 92 deletions(-)

-- 
base-commit: 9f16d5e6f220661f73b36a4be1b21575651d8833
2.43.5



^ permalink raw reply	[flat|nested] 10+ messages in thread

* [v3 PATCH 1/3] memcg/hugetlb: Introduce memcg_accounts_hugetlb
  2024-12-11 20:39 [v3 PATCH 0/3] memcg/hugetlb: Rework memcg hugetlb charging Joshua Hahn
@ 2024-12-11 20:39 ` Joshua Hahn
  2024-12-11 22:05   ` Shakeel Butt
  2024-12-14  1:28   ` Nhat Pham
  2024-12-11 20:39 ` [v3 PATCH 2/3] memcg/hugetlb: Introduce mem_cgroup_charge_hugetlb Joshua Hahn
  2024-12-11 20:39 ` [PATCH 3/3] memcg/hugetlb: Remove memcg hugetlb try-commit-cancel protocol Joshua Hahn
  2 siblings, 2 replies; 10+ messages in thread
From: Joshua Hahn @ 2024-12-11 20:39 UTC (permalink / raw)
  To: shakeel.butt
  Cc: hannes, mhocko, roman.gushchin, muchun.song, akpm, sj, cgroups,
	linux-mm, linux-kernel, kernel-team

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 7b3503d12aaf..b25eab9c933e 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -1448,6 +1448,18 @@ unsigned long memcg_page_state_local_output(struct mem_cgroup *memcg, int item)
 		memcg_page_state_output_unit(item);
 }
 
+#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;
+}
+#endif /* CONFIG_HUGETLB_PAGE */
+
 static void memcg_stat_format(struct mem_cgroup *memcg, struct seq_buf *s)
 {
 	int i;
@@ -1469,7 +1481,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);
@@ -4540,8 +4552,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



^ permalink raw reply	[flat|nested] 10+ messages in thread

* [v3 PATCH 2/3] memcg/hugetlb: Introduce mem_cgroup_charge_hugetlb
  2024-12-11 20:39 [v3 PATCH 0/3] memcg/hugetlb: Rework memcg hugetlb charging Joshua Hahn
  2024-12-11 20:39 ` [v3 PATCH 1/3] memcg/hugetlb: Introduce memcg_accounts_hugetlb Joshua Hahn
@ 2024-12-11 20:39 ` Joshua Hahn
  2024-12-11 22:06   ` Shakeel Butt
  2024-12-14  1:29   ` Nhat Pham
  2024-12-11 20:39 ` [PATCH 3/3] memcg/hugetlb: Remove memcg hugetlb try-commit-cancel protocol Joshua Hahn
  2 siblings, 2 replies; 10+ messages in thread
From: Joshua Hahn @ 2024-12-11 20:39 UTC (permalink / raw)
  To: shakeel.butt
  Cc: hannes, mhocko, roman.gushchin, muchun.song, akpm, sj, cgroups,
	linux-mm, linux-kernel, kernel-team

This patch introduces mem_cgroup_charge_hugetlb which combines the logic
of mem_cgroup_hugetlb_try_charge / mem_cgroup_hugetlb_commit_charge and
removes the need for mem_cgroup_hugetlb_cancel_charge. It also reduces
the footprint of memcg in hugetlb code and consolidates all memcg
related error paths into one.

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

---
 include/linux/memcontrol.h |  7 +++++++
 mm/hugetlb.c               | 35 ++++++++++++++---------------------
 mm/memcontrol.c            | 34 ++++++++++++++++++++++++++++++++++
 3 files changed, 55 insertions(+), 21 deletions(-)

diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
index 5502aa8e138e..f4271cfdba92 100644
--- a/include/linux/memcontrol.h
+++ b/include/linux/memcontrol.h
@@ -649,6 +649,8 @@ static inline int mem_cgroup_charge(struct folio *folio, struct mm_struct *mm,
 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,
 				  gfp_t gfp, swp_entry_t entry);
 
@@ -1152,6 +1154,11 @@ static inline int mem_cgroup_hugetlb_try_charge(struct mem_cgroup *memcg,
 	return 0;
 }
 
+static inline int mem_cgroup_charge_hugetlb(struct folio* folio, gfp_t gfp)
+{
+        return 0;
+}
+
 static inline int mem_cgroup_swapin_charge_folio(struct folio *folio,
 			struct mm_struct *mm, gfp_t gfp, swp_entry_t entry)
 {
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index ea2ed8e301ef..f0f0ffae30a3 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -2967,21 +2967,13 @@ struct folio *alloc_hugetlb_folio(struct vm_area_struct *vma,
 	struct hugepage_subpool *spool = subpool_vma(vma);
 	struct hstate *h = hstate_vma(vma);
 	struct folio *folio;
-	long map_chg, map_commit, nr_pages = pages_per_huge_page(h);
+	long map_chg, map_commit;
 	long gbl_chg;
-	int memcg_charge_ret, ret, idx;
+	int ret, idx;
 	struct hugetlb_cgroup *h_cg = NULL;
-	struct mem_cgroup *memcg;
 	bool deferred_reserve;
 	gfp_t gfp = htlb_alloc_mask(h) | __GFP_RETRY_MAYFAIL;
 
-	memcg = get_mem_cgroup_from_current();
-	memcg_charge_ret = mem_cgroup_hugetlb_try_charge(memcg, gfp, nr_pages);
-	if (memcg_charge_ret == -ENOMEM) {
-		mem_cgroup_put(memcg);
-		return ERR_PTR(-ENOMEM);
-	}
-
 	idx = hstate_index(h);
 	/*
 	 * Examine the region/reserve map to determine if the process
@@ -2989,12 +2981,8 @@ struct folio *alloc_hugetlb_folio(struct vm_area_struct *vma,
 	 * code of zero indicates a reservation exists (no change).
 	 */
 	map_chg = gbl_chg = vma_needs_reservation(h, vma, addr);
-	if (map_chg < 0) {
-		if (!memcg_charge_ret)
-			mem_cgroup_cancel_charge(memcg, nr_pages);
-		mem_cgroup_put(memcg);
+	if (map_chg < 0)
 		return ERR_PTR(-ENOMEM);
-	}
 
 	/*
 	 * Processes that did not create the mapping will have no
@@ -3092,10 +3080,18 @@ struct folio *alloc_hugetlb_folio(struct vm_area_struct *vma,
 		}
 	}
 
-	if (!memcg_charge_ret)
-		mem_cgroup_commit_charge(folio, memcg);
+	ret = mem_cgroup_charge_hugetlb(folio, gfp);
+	/*
+	 * Unconditionally increment NR_HUGETLB here. If it turns out that
+	 * mem_cgroup_charge_hugetlb failed, then immediately free the page and
+	 * decrement NR_HUGETLB.
+	 */
 	lruvec_stat_mod_folio(folio, NR_HUGETLB, pages_per_huge_page(h));
-	mem_cgroup_put(memcg);
+
+	if (ret == -ENOMEM) {
+		free_huge_folio(folio);
+		return ERR_PTR(-ENOMEM);
+	}
 
 	return folio;
 
@@ -3110,9 +3106,6 @@ struct folio *alloc_hugetlb_folio(struct vm_area_struct *vma,
 		hugepage_subpool_put_pages(spool, 1);
 out_end_reservation:
 	vma_end_reservation(h, vma, addr);
-	if (!memcg_charge_ret)
-		mem_cgroup_cancel_charge(memcg, nr_pages);
-	mem_cgroup_put(memcg);
 	return ERR_PTR(-ENOSPC);
 }
 
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index b25eab9c933e..c903e260a830 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -4561,6 +4561,40 @@ int mem_cgroup_hugetlb_try_charge(struct mem_cgroup *memcg, gfp_t gfp,
 	return 0;
 }
 
+/**
+ * mem_cgroup_charge_hugetlb - charge the memcg for a hugetlb folio
+ * @folio: folio being charged
+ * @gfp: reclaim mode
+ *
+ * This function is called when allocating a huge page folio, after the page has
+ * already been obtained and charged to the appropriate hugetlb cgroup
+ * controller (if it is enabled).
+ *
+ * Returns ENOMEM if the memcg is already full.
+ * Returns 0 if either the charge was successful, or if we skip the charging.
+ */
+int mem_cgroup_charge_hugetlb(struct folio *folio, gfp_t gfp)
+{
+	struct mem_cgroup *memcg = get_mem_cgroup_from_current();
+	int ret = 0;
+
+	/*
+	 * Even memcg does not account for hugetlb, we still want to update
+	 * system-level stats via lruvec_stat_mod_folio. Return 0, and skip
+	 * charging the memcg.
+	 */
+	if (mem_cgroup_disabled() || !memcg_accounts_hugetlb() ||
+		!memcg || !cgroup_subsys_on_dfl(memory_cgrp_subsys))
+		goto out;
+
+	if (charge_memcg(folio, memcg, gfp))
+		ret = -ENOMEM;
+
+out:
+	mem_cgroup_put(memcg);
+	return ret;
+}
+
 /**
  * mem_cgroup_swapin_charge_folio - Charge a newly allocated folio for swapin.
  * @folio: folio to charge.
-- 
2.43.5



^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 3/3] memcg/hugetlb: Remove memcg hugetlb try-commit-cancel protocol
  2024-12-11 20:39 [v3 PATCH 0/3] memcg/hugetlb: Rework memcg hugetlb charging Joshua Hahn
  2024-12-11 20:39 ` [v3 PATCH 1/3] memcg/hugetlb: Introduce memcg_accounts_hugetlb Joshua Hahn
  2024-12-11 20:39 ` [v3 PATCH 2/3] memcg/hugetlb: Introduce mem_cgroup_charge_hugetlb Joshua Hahn
@ 2024-12-11 20:39 ` Joshua Hahn
  2024-12-14  1:30   ` Nhat Pham
  2 siblings, 1 reply; 10+ messages in thread
From: Joshua Hahn @ 2024-12-11 20:39 UTC (permalink / raw)
  To: shakeel.butt
  Cc: hannes, mhocko, roman.gushchin, muchun.song, akpm, sj, cgroups,
	linux-mm, linux-kernel, kernel-team

This patch fully removes the mem_cgroup_{try, commit, cancel}_charge
functions, as well as their hugetlb variants.

Signed-off-by: Joshua Hahn <joshua.hahnjy@gmail.com>
Acked-by: Shakeel Butt <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 d1ee98dc3a38..e1a9998d8add 100644
--- a/include/linux/memcontrol.h
+++ b/include/linux/memcontrol.h
@@ -620,8 +620,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);
 
 /**
@@ -646,9 +644,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,
@@ -679,7 +674,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);
 
@@ -1137,23 +1131,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)
 {
@@ -1172,11 +1155,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 c903e260a830..7ddbb2d12eb9 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -2383,21 +2383,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);
@@ -2411,18 +2396,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)
@@ -4510,7 +4483,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;
 }
@@ -4527,40 +4502,6 @@ int __mem_cgroup_charge(struct folio *folio, struct mm_struct *mm, gfp_t gfp)
 	return ret;
 }
 
-/**
- * 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;
-}
-
 /**
  * mem_cgroup_charge_hugetlb - charge the memcg for a hugetlb folio
  * @folio: folio being charged
-- 
2.43.5



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [v3 PATCH 1/3] memcg/hugetlb: Introduce memcg_accounts_hugetlb
  2024-12-11 20:39 ` [v3 PATCH 1/3] memcg/hugetlb: Introduce memcg_accounts_hugetlb Joshua Hahn
@ 2024-12-11 22:05   ` Shakeel Butt
  2024-12-14  1:28   ` Nhat Pham
  1 sibling, 0 replies; 10+ messages in thread
From: Shakeel Butt @ 2024-12-11 22:05 UTC (permalink / raw)
  To: Joshua Hahn
  Cc: hannes, mhocko, roman.gushchin, muchun.song, akpm, sj, cgroups,
	linux-mm, linux-kernel, kernel-team

On Wed, Dec 11, 2024 at 12:39:49PM -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>

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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [v3 PATCH 2/3] memcg/hugetlb: Introduce mem_cgroup_charge_hugetlb
  2024-12-11 20:39 ` [v3 PATCH 2/3] memcg/hugetlb: Introduce mem_cgroup_charge_hugetlb Joshua Hahn
@ 2024-12-11 22:06   ` Shakeel Butt
  2024-12-14  1:29   ` Nhat Pham
  1 sibling, 0 replies; 10+ messages in thread
From: Shakeel Butt @ 2024-12-11 22:06 UTC (permalink / raw)
  To: Joshua Hahn
  Cc: hannes, mhocko, roman.gushchin, muchun.song, akpm, sj, cgroups,
	linux-mm, linux-kernel, kernel-team

On Wed, Dec 11, 2024 at 12:39:50PM -0800, Joshua Hahn wrote:
> This patch introduces mem_cgroup_charge_hugetlb which combines the logic
> of mem_cgroup_hugetlb_try_charge / mem_cgroup_hugetlb_commit_charge and
> removes the need for mem_cgroup_hugetlb_cancel_charge. It also reduces
> the footprint of memcg in hugetlb code and consolidates all memcg
> related error paths into one.
> 
> Signed-off-by: Joshua Hahn <joshua.hahnjy@gmail.com>

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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [v3 PATCH 1/3] memcg/hugetlb: Introduce memcg_accounts_hugetlb
  2024-12-11 20:39 ` [v3 PATCH 1/3] memcg/hugetlb: Introduce memcg_accounts_hugetlb Joshua Hahn
  2024-12-11 22:05   ` Shakeel Butt
@ 2024-12-14  1:28   ` Nhat Pham
  1 sibling, 0 replies; 10+ messages in thread
From: Nhat Pham @ 2024-12-14  1:28 UTC (permalink / raw)
  To: Joshua Hahn
  Cc: shakeel.butt, hannes, mhocko, roman.gushchin, muchun.song, akpm,
	sj, cgroups, linux-mm, linux-kernel, kernel-team

On Wed, Dec 11, 2024 at 12:40 PM Joshua Hahn <joshua.hahnjy@gmail.com> 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>
>

LGTM FWIW :)
Reviewed-by: Nhat Pham <nphamcs@gmail.com>


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [v3 PATCH 2/3] memcg/hugetlb: Introduce mem_cgroup_charge_hugetlb
  2024-12-11 20:39 ` [v3 PATCH 2/3] memcg/hugetlb: Introduce mem_cgroup_charge_hugetlb Joshua Hahn
  2024-12-11 22:06   ` Shakeel Butt
@ 2024-12-14  1:29   ` Nhat Pham
  1 sibling, 0 replies; 10+ messages in thread
From: Nhat Pham @ 2024-12-14  1:29 UTC (permalink / raw)
  To: Joshua Hahn
  Cc: shakeel.butt, hannes, mhocko, roman.gushchin, muchun.song, akpm,
	sj, cgroups, linux-mm, linux-kernel, kernel-team

On Wed, Dec 11, 2024 at 12:40 PM Joshua Hahn <joshua.hahnjy@gmail.com> wrote:
>
> This patch introduces mem_cgroup_charge_hugetlb which combines the logic
> of mem_cgroup_hugetlb_try_charge / mem_cgroup_hugetlb_commit_charge and
> removes the need for mem_cgroup_hugetlb_cancel_charge. It also reduces
> the footprint of memcg in hugetlb code and consolidates all memcg
> related error paths into one.
>
> Signed-off-by: Joshua Hahn <joshua.hahnjy@gmail.com>

Thanks for simplifying my convoluted code ;)

Reviewed-by: Nhat Pham <nphamcs@gmail.com>


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 3/3] memcg/hugetlb: Remove memcg hugetlb try-commit-cancel protocol
  2024-12-11 20:39 ` [PATCH 3/3] memcg/hugetlb: Remove memcg hugetlb try-commit-cancel protocol Joshua Hahn
@ 2024-12-14  1:30   ` Nhat Pham
  2024-12-16  3:03     ` Joshua Hahn
  0 siblings, 1 reply; 10+ messages in thread
From: Nhat Pham @ 2024-12-14  1:30 UTC (permalink / raw)
  To: Joshua Hahn
  Cc: shakeel.butt, hannes, mhocko, roman.gushchin, muchun.song, akpm,
	sj, cgroups, linux-mm, linux-kernel, kernel-team

On Wed, Dec 11, 2024 at 12:40 PM Joshua Hahn <joshua.hahnjy@gmail.com> wrote:

I'm assuming this is V3?

>
> This patch fully removes the mem_cgroup_{try, commit, cancel}_charge
> functions, as well as their hugetlb variants.
>
> Signed-off-by: Joshua Hahn <joshua.hahnjy@gmail.com>
> Acked-by: Shakeel Butt <shakeel.butt@linux.dev>

Reviewed-by: Nhat Pham <nphamcs@gmail.com>


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 3/3] memcg/hugetlb: Remove memcg hugetlb try-commit-cancel protocol
  2024-12-14  1:30   ` Nhat Pham
@ 2024-12-16  3:03     ` Joshua Hahn
  0 siblings, 0 replies; 10+ messages in thread
From: Joshua Hahn @ 2024-12-16  3:03 UTC (permalink / raw)
  To: Nhat Pham
  Cc: Joshua Hahn, shakeel.butt, hannes, mhocko, roman.gushchin,
	muchun.song, akpm, sj, cgroups, linux-mm, linux-kernel,
	kernel-team

On Fri, 13 Dec 2024 17:30:34 -0800 Nhat Pham <nphamcs@gmail.com> wrote:

> On Wed, Dec 11, 2024 at 12:40 PM Joshua Hahn <joshua.hahnjy@gmail.com> wrote:
> 
> I'm assuming this is V3?

Hi Nhat,
Sorry, the subject should contain v3. Thank you for the catch : -)

Joshua

> > This patch fully removes the mem_cgroup_{try, commit, cancel}_charge
> > functions, as well as their hugetlb variants.
> >
> > Signed-off-by: Joshua Hahn <joshua.hahnjy@gmail.com>
> > Acked-by: Shakeel Butt <shakeel.butt@linux.dev>
> 
> Reviewed-by: Nhat Pham <nphamcs@gmail.com>


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2024-12-16  3:03 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-12-11 20:39 [v3 PATCH 0/3] memcg/hugetlb: Rework memcg hugetlb charging Joshua Hahn
2024-12-11 20:39 ` [v3 PATCH 1/3] memcg/hugetlb: Introduce memcg_accounts_hugetlb Joshua Hahn
2024-12-11 22:05   ` Shakeel Butt
2024-12-14  1:28   ` Nhat Pham
2024-12-11 20:39 ` [v3 PATCH 2/3] memcg/hugetlb: Introduce mem_cgroup_charge_hugetlb Joshua Hahn
2024-12-11 22:06   ` Shakeel Butt
2024-12-14  1:29   ` Nhat Pham
2024-12-11 20:39 ` [PATCH 3/3] memcg/hugetlb: Remove memcg hugetlb try-commit-cancel protocol Joshua Hahn
2024-12-14  1:30   ` Nhat Pham
2024-12-16  3:03     ` Joshua Hahn

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox