| Message ID | 20240724190214.1108049-3-kinseyho@google.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Improve mem_cgroup_iter() | expand |
On Wed, Jul 24, 2024 at 07:02:12PM +0000, Kinsey Ho wrote: > The generation number in struct mem_cgroup_reclaim_iter should be > incremented on every round-trip. Currently, it is possible for a > concurrent reclaimer to jump in at the end of the hierarchy, causing a > traversal restart (resetting the iteration position) without > incrementing the generation number. > > Move the traversal restart such that the generation number is > incremented before the restart. > > Signed-off-by: Kinsey Ho <kinseyho@google.com> The consequence of resetting the position without bumping the generation would be that another ongoing mem_cgroup_iter() could walk the tree twice, which is undesirable. It would be good to spell that out in the changelog. Otherwise, looks good to me. Acked-by: Johannes Weiner <hannes@cmpxchg.org>
diff --git a/mm/memcontrol.c b/mm/memcontrol.c index 062bfeee799c..f672bc47c6b5 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -1003,7 +1003,7 @@ struct mem_cgroup *mem_cgroup_iter(struct mem_cgroup *root, root = root_mem_cgroup; rcu_read_lock(); - +restart: if (reclaim) { struct mem_cgroup_per_node *mz; @@ -1030,14 +1030,6 @@ struct mem_cgroup *mem_cgroup_iter(struct mem_cgroup *root, for (;;) { css = css_next_descendant_pre(css, &root->css); if (!css) { - /* - * Reclaimers share the hierarchy walk, and a - * new one might jump in right at the end of - * the hierarchy - make sure they see at least - * one group and restart from the beginning. - */ - if (!prev) - continue; break; } @@ -1060,8 +1052,18 @@ struct mem_cgroup *mem_cgroup_iter(struct mem_cgroup *root, */ (void)cmpxchg(&iter->position, pos, memcg); - if (!memcg) + if (!memcg) { iter->generation++; + + /* + * Reclaimers share the hierarchy walk, and a + * new one might jump in right at the end of + * the hierarchy - make sure they see at least + * one group and restart from the beginning. + */ + if (!prev) + goto restart; + } } out_unlock:
The generation number in struct mem_cgroup_reclaim_iter should be incremented on every round-trip. Currently, it is possible for a concurrent reclaimer to jump in at the end of the hierarchy, causing a traversal restart (resetting the iteration position) without incrementing the generation number. Move the traversal restart such that the generation number is incremented before the restart. Signed-off-by: Kinsey Ho <kinseyho@google.com> --- mm/memcontrol.c | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-)