[PATCH RFC 3/5] memcg: remove retry logic in mem_cgroup_private_id_get_online

[Bingfang Guo via B4 Relay]

From: Bingfang Guo <bingfangguo@xxxxxxxxxxx>

With the ID released only in css_released() (patch 1) and every caller
holding a stable reference obtained from get_mem_cgroup_from_objcg()
(patch 2), the id refcount is guaranteed to be non-zero whenever the ID
is taken, so the retry loop that walked up the parent chain can never
trigger.

Remove the retry logic, rename the function to
mem_cgroup_private_id_get() and turn the fallible return value into a
VM_WARN_ON() that documents the invariant.

Signed-off-by: Bingfang Guo <bingfangguo@xxxxxxxxxxx>
---
 mm/memcontrol-v1.c |  2 +-
 mm/memcontrol-v1.h |  3 +--
 mm/memcontrol.c    | 20 +++++---------------
 3 files changed, 7 insertions(+), 18 deletions(-)

diff --git a/mm/memcontrol-v1.c b/mm/memcontrol-v1.c
index a913d32ad1e17..e5161e061bd11 100644
--- a/mm/memcontrol-v1.c
+++ b/mm/memcontrol-v1.c
@@ -648,7 +648,7 @@ void __memcg1_swapout(struct folio *folio, struct swap_cluster_info *ci)
 	mod_memcg_state(memcg, MEMCG_SWAP, nr_entries);
 
 	/* we have a reference to it, so we should get exact memcg itself */
-	mem_cgroup_private_id_get_online(memcg, nr_entries);
+	mem_cgroup_private_id_get(memcg, nr_entries);
 
 	__swap_cgroup_set(ci, swp_cluster_offset(folio->swap), nr_entries,
 			  mem_cgroup_private_id(memcg));
diff --git a/mm/memcontrol-v1.h b/mm/memcontrol-v1.h
index 0f703f239c80f..9c74400aa7ddb 100644
--- a/mm/memcontrol-v1.h
+++ b/mm/memcontrol-v1.h
@@ -21,8 +21,7 @@ void drain_all_stock(struct mem_cgroup *root_memcg);
 
 int memory_stat_show(struct seq_file *m, void *v);
 
-struct mem_cgroup *mem_cgroup_private_id_get_online(struct mem_cgroup *memcg,
-						    unsigned int n);
+void mem_cgroup_private_id_get(struct mem_cgroup *memcg, unsigned int n);
 
 /* Cgroup v1-specific declarations */
 #ifdef CONFIG_MEMCG_V1
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index a210fe2501219..12545ca48194d 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -3985,23 +3985,13 @@ static inline void mem_cgroup_private_id_put(struct mem_cgroup *memcg, unsigned
 	__mem_cgroup_private_id_put(memcg->id_objcg, memcg->id, n);
 }
 
-struct mem_cgroup *mem_cgroup_private_id_get_online(struct mem_cgroup *memcg, unsigned int n)
+void mem_cgroup_private_id_get(struct mem_cgroup *memcg, unsigned int n)
 {
+	bool success;
 	struct obj_cgroup *objcg = memcg->id_objcg;
 
-	while (!refcount_add_not_zero(n, &objcg->id_ref)) {
-		/*
-		 * The root cgroup cannot be destroyed, so it's refcount must
-		 * always be >= 1.
-		 */
-		if (WARN_ON_ONCE(mem_cgroup_is_root(memcg))) {
-			VM_BUG_ON(1);
-			break;
-		}
-		memcg = parent_mem_cgroup(memcg);
-		objcg = memcg->id_objcg;
-	}
-	return memcg;
+	success = refcount_add_not_zero(n, &objcg->id_ref);
+	VM_WARN_ON(!success);
 }
 
 /**
@@ -5673,7 +5663,7 @@ int __mem_cgroup_try_charge_swap(struct folio *folio)
 	mod_memcg_state(memcg, MEMCG_SWAP, nr_pages);
 
 	/* we have a reference to it, so we should get exact memcg itself */
-	mem_cgroup_private_id_get_online(memcg, nr_pages);
+	mem_cgroup_private_id_get(memcg, nr_pages);
 
 	ci = swap_cluster_get_and_lock(folio);
 	__swap_cgroup_set(ci, swp_cluster_offset(folio->swap), nr_pages,

-- 
2.43.7