Re: [PATCH 1/8] cgroup: kill cgroup_subsys->__DEPRECATED_clear_css_refs
From: Michal Hocko <hidden>
Date: 2012-10-31 14:37:58
Also in:
lkml
On Tue 30-10-12 21:22:38, Tejun Heo wrote:
2ef37d3fe4 ("memcg: Simplify mem_cgroup_force_empty_list error
handling") removed the last user of __DEPRECATED_clear_css_refs. This
patch removes __DEPRECATED_clear_css_refs and mechanisms to support
it.
* Conditionals dependent on __DEPRECATED_clear_css_refs removed.
* ->pre_destroy() now can only fail if a new task is attached or child
cgroup is created while ->pre_destroy()s are being called. As the
condition is checked again after re-acquiring cgroup_mutex
afterwards, we don't need to take any immediate action on
->pre_destroy() failures. This reduces cgroup_call_pre_destroy() to
a simple loop surrounding ->pre_destory(). Remove
cgroup_call_pre_destroy() and open-code the loop into
cgroup_rmdir().
* cgroup_clear_css_refs() can no longer fail. All that needs to be
done are deactivating refcnts, setting CSS_REMOVED and putting the
base reference on each css. Remove cgroup_clear_css_refs() and the
failure path, and open-code the loops into cgroup_rmdir().
Note that cgroup_rmdir() will see more cleanup soon.
Signed-off-by: Tejun Heo <redacted>Looks good to me and the diffstat is really encouraging Reviewed-by: Michal Hocko <redacted> with a minor note bellow
--- include/linux/cgroup.h | 12 ---- kernel/cgroup.c | 159 ++++++++++++------------------------------------- 2 files changed, 38 insertions(+), 133 deletions(-)
[...]
quoted hunk ↗ jump to hunk
diff --git a/kernel/cgroup.c b/kernel/cgroup.c index 7981850..033bf4b 100644 --- a/kernel/cgroup.c +++ b/kernel/cgroup.c
[...]
quoted hunk ↗ jump to hunk
@@ -4168,11 +4075,9 @@ again: * Call pre_destroy handlers of subsys. Notify subsystems * that rmdir() request comes. */ - ret = cgroup_call_pre_destroy(cgrp); - if (ret) { - clear_bit(CGRP_WAIT_ON_RMDIR, &cgrp->flags); - return ret; - } + for_each_subsys(cgrp->root, ss) + if (ss->pre_destroy) + WARN_ON_ONCE(ss->pre_destroy(cgrp));
Hmm, I am not sure I like this WARN_ON_ONCE. First it can happen for more than one controller and second we can just clear CGRP_WAIT_ON_RMDIR and return with EBUSY. The only possible failure at the moment is when a new task or a child group appear. I know it is not a big deal because it will disappear later in the series but it would be more readable IMO. Thanks! -- Michal Hocko SUSE Labs