| From: Kinsey Ho <kinseyho@google.com> |
| Subject: mm: increment gen # before restarting traversal |
| Date: Thu, 5 Sep 2024 00:30:52 +0000 |
| |
| 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. |
| |
| Link: https://lkml.kernel.org/r/20240905003058.1859929-4-kinseyho@google.com |
| Signed-off-by: Kinsey Ho <kinseyho@google.com> |
| Reviewed-by: T.J. Mercier <tjmercier@google.com> |
| Cc: Johannes Weiner <hannes@cmpxchg.org> |
| Cc: Michal Hocko <mhocko@kernel.org> |
| Cc: Michal Koutný <mkoutny@suse.com> |
| Cc: Muchun Song <muchun.song@linux.dev> |
| Cc: Roman Gushchin <roman.gushchin@linux.dev> |
| Cc: Shakeel Butt <shakeel.butt@linux.dev> |
| Cc: Tejun Heo <tj@kernel.org> |
| Cc: Yosry Ahmed <yosryahmed@google.com> |
| Cc: Zefan Li <lizefan.x@bytedance.com> |
| Cc: Hugh Dickins <hughd@google.com> |
| Signed-off-by: Andrew Morton <akpm@linux-foundation.org> |
| --- |
| |
| mm/memcontrol.c | 22 ++++++++++++---------- |
| 1 file changed, 12 insertions(+), 10 deletions(-) |
| |
| --- a/mm/memcontrol.c~mm-increment-gen-before-restarting-traversal |
| +++ a/mm/memcontrol.c |
| @@ -997,7 +997,7 @@ struct mem_cgroup *mem_cgroup_iter(struc |
| 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(struc |
| 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(struc |
| */ |
| (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: |
| _ |