[rh7,v5,4/9] mm/mem_cgroup_iter: Always assign iter->last_visited under rcu

Submitted by Konstantin Khorenko on Feb. 26, 2021, 2:26 p.m.

Details

Message ID 20210226142605.7789-5-khorenko@virtuozzo.com
State New
Series "mm/mem_cgroup_iter: Reduce the number of iterator restarts upon cgroup removals"
Headers show

Commit Message

Konstantin Khorenko Feb. 26, 2021, 2:26 p.m.
It's quite strange to have rcu section in mem_cgroup_iter(),
but do not use rcu_dereference/rcu_assign for pointers being defended.

We plan to access/assign '->last_visited' during iterator invalidation,
so we'll need the protection there anyway.

https://jira.sw.ru/browse/PSBM-123655

Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com>
Reviewed-by: Kirill Tkhai <ktkhai@virtuozzo.com>
---
 mm/memcontrol.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

Patch hide | download patch | download mbox

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 8040f09425bf..7921f04b3670 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -194,8 +194,18 @@  struct mem_cgroup_reclaim_iter {
 	/*
 	 * last scanned hierarchy member. Valid only if last_dead_count
 	 * matches memcg->dead_count of the hierarchy root group.
+	 *
+	 * Memory pointed by 'last_visited' is freed not earlier than
+	 * one rcu period after we accessed it:
+	 *   cgroup_offline_fn()
+	 *    offline_css()
+	 *    list_del_rcu()
+	 *    dput()
+	 *    ...
+	 *     cgroup_diput()
+	 *      call_rcu(&cgrp->rcu_head, cgroup_free_rcu)
 	 */
-	struct mem_cgroup *last_visited;
+	struct mem_cgroup __rcu *last_visited;
 	unsigned long last_dead_count;
 
 	/* scan generation, increased every round-trip */
@@ -1592,7 +1602,7 @@  mem_cgroup_iter_load(struct mem_cgroup_reclaim_iter *iter,
 	*sequence = atomic_read(&root->dead_count);
 	if (iter->last_dead_count == *sequence) {
 		smp_rmb();
-		position = iter->last_visited;
+		position = rcu_dereference(iter->last_visited);
 
 		/*
 		 * We cannot take a reference to root because we might race
@@ -1620,7 +1630,7 @@  static void mem_cgroup_iter_update(struct mem_cgroup_reclaim_iter *iter,
 	 * don't lose destruction events in between.  We could have
 	 * raced with the destruction of @new_position after all.
 	 */
-	iter->last_visited = new_position;
+	rcu_assign_pointer(iter->last_visited, new_position);
 	smp_wmb();
 	iter->last_dead_count = sequence;
 
@@ -1681,7 +1691,7 @@  struct mem_cgroup *mem_cgroup_iter(struct mem_cgroup *root,
 			mz = mem_cgroup_zoneinfo(root, nid, zid);
 			iter = &mz->reclaim_iter[reclaim->priority];
 			if (prev && reclaim->generation != iter->generation) {
-				iter->last_visited = NULL;
+				rcu_assign_pointer(iter->last_visited, NULL);
 				goto out_unlock;
 			}