mm/list_lru: code clean up for reparenting

No feature change, just change of code structure and fix comment.

The list lrus are not empty until memcg_reparent_list_lru_node() calls are
all done, so the comments in memcg_offline_kmem were slightly inaccurate.

Link: https://lkml.kernel.org/r/20241104175257.60853-4-ryncsn@gmail.com
Signed-off-by: Kairui Song <kasong@tencent.com>
Reviewed-by: Muchun Song <muchun.song@linux.dev>
Acked-by: Shakeel Butt <shakeel.butt@linux.dev>
Cc: Chengming Zhou <zhouchengming@bytedance.com>
Cc: Johannes Weiner <hannes@cmpxchg.org>
Cc: Matthew Wilcox (Oracle) <willy@infradead.org>
Cc: Michal Hocko <mhocko@suse.com>
Cc: Qi Zheng <zhengqi.arch@bytedance.com>
Cc: Roman Gushchin <roman.gushchin@linux.dev>
Cc: Waiman Long <longman@redhat.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>

diff --git a/mm/list_lru.c b/mm/list_lru.c
index a798e7624f6903d1b384c4a30156547a0e59d3d4..b54f092d4d65542ecaf6f8e33819db3fad08ef3f 100644 (file)
--- a/mm/list_lru.c
+++ b/mm/list_lru.c
@@ -421,35 +421,16 @@ out:
        spin_unlock_irq(&nlru->lock);
 }
 
-static void memcg_reparent_list_lru(struct list_lru *lru,
-                                   int src_idx, struct mem_cgroup *dst_memcg)
-{
-       int i;
-
-       for_each_node(i)
-               memcg_reparent_list_lru_node(lru, i, src_idx, dst_memcg);
-
-       memcg_list_lru_free(lru, src_idx);
-}
-
 void memcg_reparent_list_lrus(struct mem_cgroup *memcg, struct mem_cgroup *parent)
 {
        struct cgroup_subsys_state *css;
        struct list_lru *lru;
-       int src_idx = memcg->kmemcg_id;
+       int src_idx = memcg->kmemcg_id, i;
 
        /*
         * Change kmemcg_id of this cgroup and all its descendants to the
         * parent's id, and then move all entries from this cgroup's list_lrus
         * to ones of the parent.
-        *
-        * After we have finished, all list_lrus corresponding to this cgroup
-        * are guaranteed to remain empty. So we can safely free this cgroup's
-        * list lrus in memcg_list_lru_free().
-        *
-        * Changing ->kmemcg_id to the parent can prevent memcg_list_lru_alloc()
-        * from allocating list lrus for this cgroup after memcg_list_lru_free()
-        * call.
         */
        rcu_read_lock();
        css_for_each_descendant_pre(css, &memcg->css) {
@@ -460,9 +441,23 @@ void memcg_reparent_list_lrus(struct mem_cgroup *memcg, struct mem_cgroup *paren
        }
        rcu_read_unlock();
 
+       /*
+        * With kmemcg_id set to parent, holding the lock of each list_lru_node
+        * below can prevent list_lru_{add,del,isolate} from touching the lru,
+        * safe to reparent.
+        */
        mutex_lock(&list_lrus_mutex);
-       list_for_each_entry(lru, &memcg_list_lrus, list)
-               memcg_reparent_list_lru(lru, src_idx, parent);
+       list_for_each_entry(lru, &memcg_list_lrus, list) {
+               for_each_node(i)
+                       memcg_reparent_list_lru_node(lru, i, src_idx, parent);
+
+               /*
+                * Here all list_lrus corresponding to the cgroup are guaranteed
+                * to remain empty, we can safely free this lru, any further
+                * memcg_list_lru_alloc() call will simply bail out.
+                */
+               memcg_list_lru_free(lru, src_idx);
+       }
        mutex_unlock(&list_lrus_mutex);
 }
 

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 6486e96528434fc7134dee79d4cc3965451a266b..8b9061b58a728ac92c782bc8d8957c343bcd0c7a 100644 (file)
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -3111,13 +3111,6 @@ static void memcg_offline_kmem(struct mem_cgroup *memcg)
                parent = root_mem_cgroup;
 
        memcg_reparent_objcgs(memcg, parent);
-
-       /*
-        * After we have finished memcg_reparent_objcgs(), all list_lrus
-        * corresponding to this cgroup are guaranteed to remain empty.
-        * The ordering is imposed by list_lru_node->lock taken by
-        * memcg_reparent_list_lrus().
-        */
        memcg_reparent_list_lrus(memcg, parent);
 }