Commit b0a1f4ec authored by Michal Hocko's avatar Michal Hocko Committed by Greg Kroah-Hartman

memcg: fix endless loop caused by mem_cgroup_iter

commit ecc736fc upstream.

Hugh has reported an endless loop when the hardlimit reclaim sees the
same group all the time.  This might happen when the reclaim races with
the memcg removal.

shrink_zone
                                                [rmdir root]
  mem_cgroup_iter(root, NULL, reclaim)
    // prev = NULL
    rcu_read_lock()
    mem_cgroup_iter_load
      last_visited = iter->last_visited   // gets root || NULL
      css_tryget(last_visited)            // failed
      last_visited = NULL                 [1]
    memcg = root = __mem_cgroup_iter_next(root, NULL)
    mem_cgroup_iter_update
      iter->last_visited = root;
    reclaim->generation = iter->generation

 mem_cgroup_iter(root, root, reclaim)
   // prev = root
   rcu_read_lock
    mem_cgroup_iter_load
      last_visited = iter->last_visited   // gets root
      css_tryget(last_visited)            // failed
    [1]

The issue seemed to be introduced by commit 5f578161 ("memcg: relax
memcg iter caching") which has replaced unconditional css_get/css_put by
css_tryget/css_put for the cached iterator.

This patch fixes the issue by skipping css_tryget on the root of the
tree walk in mem_cgroup_iter_load and symmetrically doesn't release it
in mem_cgroup_iter_update.
Signed-off-by: default avatarMichal Hocko <mhocko@suse.cz>
Reported-by: default avatarHugh Dickins <hughd@google.com>
Tested-by: default avatarHugh Dickins <hughd@google.com>
Cc: Johannes Weiner <hannes@cmpxchg.org>
Cc: Greg Thelen <gthelen@google.com>
Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent 414f6b9f
......@@ -1120,7 +1120,15 @@ mem_cgroup_iter_load(struct mem_cgroup_reclaim_iter *iter,
if (iter->last_dead_count == *sequence) {
smp_rmb();
position = iter->last_visited;
if (position && !css_tryget(&position->css))
/*
* We cannot take a reference to root because we might race
* with root removal and returning NULL would end up in
* an endless loop on the iterator user level when root
* would be returned all the time.
*/
if (position && position != root &&
!css_tryget(&position->css))
position = NULL;
}
return position;
......@@ -1129,9 +1137,11 @@ mem_cgroup_iter_load(struct mem_cgroup_reclaim_iter *iter,
static void mem_cgroup_iter_update(struct mem_cgroup_reclaim_iter *iter,
struct mem_cgroup *last_visited,
struct mem_cgroup *new_position,
struct mem_cgroup *root,
int sequence)
{
if (last_visited)
/* root reference counting symmetric to mem_cgroup_iter_load */
if (last_visited && last_visited != root)
css_put(&last_visited->css);
/*
* We store the sequence count from the time @last_visited was
......@@ -1206,7 +1216,8 @@ struct mem_cgroup *mem_cgroup_iter(struct mem_cgroup *root,
memcg = __mem_cgroup_iter_next(root, last_visited);
if (reclaim) {
mem_cgroup_iter_update(iter, last_visited, memcg, seq);
mem_cgroup_iter_update(iter, last_visited, memcg, root,
seq);
if (!memcg)
iter->generation++;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment