From: Bingfang Guo 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 --- 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