Message ID | 20240827230753.2073580-4-kinseyho@google.com (mailing list archive) |
---|---|
State | New |
Headers | show |
Series | Improve mem_cgroup_iter() | expand |
On Tue, Aug 27, 2024 at 4:11 PM Kinsey Ho <kinseyho@google.com> 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. > > By resetting the position without incrementing the generation, it's > possible for another ongoing mem_cgroup_iter() thread to walk the tree > twice. > > Move the traversal restart such that the generation number is > incremented before the restart. > > Signed-off-by: Kinsey Ho <kinseyho@google.com> Reviewed-by: T.J. Mercier <tjmercier@google.com>
diff --git a/mm/memcontrol.c b/mm/memcontrol.c index 67b1994377b7..51b194a4c375 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -997,7 +997,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; @@ -1024,14 +1024,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; } @@ -1054,8 +1046,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. By resetting the position without incrementing the generation, it's possible for another ongoing mem_cgroup_iter() thread to walk the tree twice. 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(-)