Re: [PATCH] memcg: don't do cleanup manually if mem_cgroup_css_online() fails

Michal Hocko
2013-04-02T14:28:25Z

On Tue 02-04-13 18:20:56, Glauber Costa wrote:
> On 04/02/2013 06:16 PM, Michal Hocko wrote:
> > mem_cgroup_css_online
> > memcg_init_kmem
> > mem_cgroup_get # refcnt = 2
> > memcg_update_all_caches
> > memcg_update_cache_size # fails with ENOMEM
>
> Here is the thing: this one in kmem only happens for kmem enabled
> memcgs. For those, we tend to do a get once, and put only when the last
> kmem reference is gone.
>
> For non-kmem memcgs, refcnt will be 1 here, and will be balanced out by
> the mem_cgroup_put() in css_free.

So we need this, right?
---
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index f608546..2ef875d 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -5306,6 +5306,8 @@ static int memcg_propagate_kmem(struct mem_cgroup *memcg)
ret = memcg_update_cache_sizes(memcg);
mutex_unlock(&set_limit_mutex);
out:
+ if (ret)
+ mem_cgroup_put(memcg);
return ret;
}
#endif /* CONFIG_MEMCG_KMEM */
@@ -6417,16 +6419,6 @@ mem_cgroup_css_online(struct cgroup *cont)

error = memcg_init_kmem(memcg, &mem_cgroup_subsys);
mutex_unlock(&memcg_create_mutex);
- if (error) {
- /*
- * We call put now because our (and parent's) refcnts
- * are already in place. mem_cgroup_put() will internally
- * call __mem_cgroup_free, so return directly
- */
- mem_cgroup_put(memcg);
- if (parent->use_hierarchy)
- mem_cgroup_put(parent);
- }
return error;
}

--
Michal Hocko
SUSE Labs