cgroup: remove cgroup_tree_mutex

cgroup_tree_mutex was introduced to work around the circular
dependency between cgroup_mutex and kernfs active protection - some
kernfs file and directory operations needed cgroup_mutex putting
cgroup_mutex under active protection but cgroup also needs to be able
to access cgroup hierarchies and cftypes to determine which
kernfs_nodes need to be removed.  cgroup_tree_mutex nested above both
cgroup_mutex and kernfs active protection and used to protect the
hierarchy and cftypes.  While this worked, it added a lot of double
lockings and was generally cumbersome.

kernfs provides a mechanism to opt out of active protection and cgroup
was already using it for removal and subtree_control.  There's no
reason to mix both methods of avoiding circular locking dependency and
the preceding cgroup_kn_lock_live() changes applied it to all relevant
cgroup kernfs operations making it unnecessary to nest cgroup_mutex
under kernfs active protection.  The previous patch reversed the
original lock ordering and put cgroup_mutex above kernfs active
protection.

After these changes, all cgroup_tree_mutex usages are now accompanied
by cgroup_mutex making the former completely redundant.  This patch
removes cgroup_tree_mutex and all its usages.

Signed-off-by: Tejun Heo <tj@kernel.org>
Acked-by: Li Zefan <lizefan@huawei.com>
This commit is contained in:
Tejun Heo 2014-05-13 12:19:23 -04:00
parent 01f6474ce0
commit 8353da1f91

View file

@ -70,15 +70,6 @@
#define CGROUP_FILE_NAME_MAX (MAX_CGROUP_TYPE_NAMELEN + \ #define CGROUP_FILE_NAME_MAX (MAX_CGROUP_TYPE_NAMELEN + \
MAX_CFTYPE_NAME + 2) MAX_CFTYPE_NAME + 2)
/*
* cgroup_tree_mutex nests above cgroup_mutex and protects cftypes, file
* creation/removal and hierarchy changing operations including cgroup
* creation, removal, css association and controller rebinding. This outer
* lock is needed mainly to resolve the circular dependency between kernfs
* active ref and cgroup_mutex. cgroup_tree_mutex nests above both.
*/
static DEFINE_MUTEX(cgroup_tree_mutex);
/* /*
* cgroup_mutex is the master lock. Any modification to cgroup or its * cgroup_mutex is the master lock. Any modification to cgroup or its
* hierarchy must be performed while holding it. * hierarchy must be performed while holding it.
@ -111,11 +102,10 @@ static DEFINE_SPINLOCK(cgroup_idr_lock);
*/ */
static DEFINE_SPINLOCK(release_agent_path_lock); static DEFINE_SPINLOCK(release_agent_path_lock);
#define cgroup_assert_mutexes_or_rcu_locked() \ #define cgroup_assert_mutex_or_rcu_locked() \
rcu_lockdep_assert(rcu_read_lock_held() || \ rcu_lockdep_assert(rcu_read_lock_held() || \
lockdep_is_held(&cgroup_tree_mutex) || \
lockdep_is_held(&cgroup_mutex), \ lockdep_is_held(&cgroup_mutex), \
"cgroup_[tree_]mutex or RCU read lock required"); "cgroup_mutex or RCU read lock required");
/* /*
* cgroup destruction makes heavy use of work items and there can be a lot * cgroup destruction makes heavy use of work items and there can be a lot
@ -243,7 +233,6 @@ static struct cgroup_subsys_state *cgroup_css(struct cgroup *cgrp,
{ {
if (ss) if (ss)
return rcu_dereference_check(cgrp->subsys[ss->id], return rcu_dereference_check(cgrp->subsys[ss->id],
lockdep_is_held(&cgroup_tree_mutex) ||
lockdep_is_held(&cgroup_mutex)); lockdep_is_held(&cgroup_mutex));
else else
return &cgrp->dummy_css; return &cgrp->dummy_css;
@ -347,7 +336,6 @@ static int notify_on_release(const struct cgroup *cgrp)
for ((ssid) = 0; (ssid) < CGROUP_SUBSYS_COUNT; (ssid)++) \ for ((ssid) = 0; (ssid) < CGROUP_SUBSYS_COUNT; (ssid)++) \
if (!((css) = rcu_dereference_check( \ if (!((css) = rcu_dereference_check( \
(cgrp)->subsys[(ssid)], \ (cgrp)->subsys[(ssid)], \
lockdep_is_held(&cgroup_tree_mutex) || \
lockdep_is_held(&cgroup_mutex)))) { } \ lockdep_is_held(&cgroup_mutex)))) { } \
else else
@ -381,7 +369,7 @@ static int notify_on_release(const struct cgroup *cgrp)
/* iterate over child cgrps, lock should be held throughout iteration */ /* iterate over child cgrps, lock should be held throughout iteration */
#define cgroup_for_each_live_child(child, cgrp) \ #define cgroup_for_each_live_child(child, cgrp) \
list_for_each_entry((child), &(cgrp)->children, sibling) \ list_for_each_entry((child), &(cgrp)->children, sibling) \
if (({ lockdep_assert_held(&cgroup_tree_mutex); \ if (({ lockdep_assert_held(&cgroup_mutex); \
cgroup_is_dead(child); })) \ cgroup_is_dead(child); })) \
; \ ; \
else else
@ -869,7 +857,6 @@ static void cgroup_destroy_root(struct cgroup_root *root)
struct cgroup *cgrp = &root->cgrp; struct cgroup *cgrp = &root->cgrp;
struct cgrp_cset_link *link, *tmp_link; struct cgrp_cset_link *link, *tmp_link;
mutex_lock(&cgroup_tree_mutex);
mutex_lock(&cgroup_mutex); mutex_lock(&cgroup_mutex);
BUG_ON(atomic_read(&root->nr_cgrps)); BUG_ON(atomic_read(&root->nr_cgrps));
@ -899,7 +886,6 @@ static void cgroup_destroy_root(struct cgroup_root *root)
cgroup_exit_root_id(root); cgroup_exit_root_id(root);
mutex_unlock(&cgroup_mutex); mutex_unlock(&cgroup_mutex);
mutex_unlock(&cgroup_tree_mutex);
kernfs_destroy_root(root->kf_root); kernfs_destroy_root(root->kf_root);
cgroup_free_root(root); cgroup_free_root(root);
@ -1096,7 +1082,6 @@ static void cgroup_kn_unlock(struct kernfs_node *kn)
cgrp = kn->parent->priv; cgrp = kn->parent->priv;
mutex_unlock(&cgroup_mutex); mutex_unlock(&cgroup_mutex);
mutex_unlock(&cgroup_tree_mutex);
kernfs_unbreak_active_protection(kn); kernfs_unbreak_active_protection(kn);
cgroup_put(cgrp); cgroup_put(cgrp);
@ -1135,7 +1120,6 @@ static struct cgroup *cgroup_kn_lock_live(struct kernfs_node *kn)
cgroup_get(cgrp); cgroup_get(cgrp);
kernfs_break_active_protection(kn); kernfs_break_active_protection(kn);
mutex_lock(&cgroup_tree_mutex);
mutex_lock(&cgroup_mutex); mutex_lock(&cgroup_mutex);
if (!cgroup_is_dead(cgrp)) if (!cgroup_is_dead(cgrp))
@ -1149,7 +1133,6 @@ static void cgroup_rm_file(struct cgroup *cgrp, const struct cftype *cft)
{ {
char name[CGROUP_FILE_NAME_MAX]; char name[CGROUP_FILE_NAME_MAX];
lockdep_assert_held(&cgroup_tree_mutex);
lockdep_assert_held(&cgroup_mutex); lockdep_assert_held(&cgroup_mutex);
kernfs_remove_by_name(cgrp->kn, cgroup_file_name(cgrp, cft, name)); kernfs_remove_by_name(cgrp->kn, cgroup_file_name(cgrp, cft, name));
} }
@ -1179,7 +1162,6 @@ static int rebind_subsystems(struct cgroup_root *dst_root, unsigned int ss_mask)
struct cgroup_subsys *ss; struct cgroup_subsys *ss;
int ssid, i, ret; int ssid, i, ret;
lockdep_assert_held(&cgroup_tree_mutex);
lockdep_assert_held(&cgroup_mutex); lockdep_assert_held(&cgroup_mutex);
for_each_subsys(ss, ssid) { for_each_subsys(ss, ssid) {
@ -1457,7 +1439,6 @@ static int cgroup_remount(struct kernfs_root *kf_root, int *flags, char *data)
return -EINVAL; return -EINVAL;
} }
mutex_lock(&cgroup_tree_mutex);
mutex_lock(&cgroup_mutex); mutex_lock(&cgroup_mutex);
/* See what subsystems are wanted */ /* See what subsystems are wanted */
@ -1503,7 +1484,6 @@ static int cgroup_remount(struct kernfs_root *kf_root, int *flags, char *data)
kfree(opts.release_agent); kfree(opts.release_agent);
kfree(opts.name); kfree(opts.name);
mutex_unlock(&cgroup_mutex); mutex_unlock(&cgroup_mutex);
mutex_unlock(&cgroup_tree_mutex);
return ret; return ret;
} }
@ -1606,7 +1586,6 @@ static int cgroup_setup_root(struct cgroup_root *root, unsigned int ss_mask)
struct css_set *cset; struct css_set *cset;
int i, ret; int i, ret;
lockdep_assert_held(&cgroup_tree_mutex);
lockdep_assert_held(&cgroup_mutex); lockdep_assert_held(&cgroup_mutex);
ret = cgroup_idr_alloc(&root->cgroup_idr, root_cgrp, 1, 2, GFP_NOWAIT); ret = cgroup_idr_alloc(&root->cgroup_idr, root_cgrp, 1, 2, GFP_NOWAIT);
@ -1696,7 +1675,6 @@ static struct dentry *cgroup_mount(struct file_system_type *fs_type,
if (!use_task_css_set_links) if (!use_task_css_set_links)
cgroup_enable_task_cg_lists(); cgroup_enable_task_cg_lists();
mutex_lock(&cgroup_tree_mutex);
mutex_lock(&cgroup_mutex); mutex_lock(&cgroup_mutex);
/* First find the desired set of subsystems */ /* First find the desired set of subsystems */
@ -1761,9 +1739,7 @@ retry:
*/ */
if (!atomic_inc_not_zero(&root->cgrp.refcnt)) { if (!atomic_inc_not_zero(&root->cgrp.refcnt)) {
mutex_unlock(&cgroup_mutex); mutex_unlock(&cgroup_mutex);
mutex_unlock(&cgroup_tree_mutex);
msleep(10); msleep(10);
mutex_lock(&cgroup_tree_mutex);
mutex_lock(&cgroup_mutex); mutex_lock(&cgroup_mutex);
goto retry; goto retry;
} }
@ -1796,7 +1772,6 @@ retry:
out_unlock: out_unlock:
mutex_unlock(&cgroup_mutex); mutex_unlock(&cgroup_mutex);
mutex_unlock(&cgroup_tree_mutex);
kfree(opts.release_agent); kfree(opts.release_agent);
kfree(opts.name); kfree(opts.name);
@ -2507,7 +2482,6 @@ static int cgroup_update_dfl_csses(struct cgroup *cgrp)
struct css_set *src_cset; struct css_set *src_cset;
int ret; int ret;
lockdep_assert_held(&cgroup_tree_mutex);
lockdep_assert_held(&cgroup_mutex); lockdep_assert_held(&cgroup_mutex);
/* look up all csses currently attached to @cgrp's subtree */ /* look up all csses currently attached to @cgrp's subtree */
@ -2866,20 +2840,18 @@ static int cgroup_rename(struct kernfs_node *kn, struct kernfs_node *new_parent,
return -EPERM; return -EPERM;
/* /*
* We're gonna grab cgroup_tree_mutex which nests outside kernfs * We're gonna grab cgroup_mutex which nests outside kernfs
* active_ref. kernfs_rename() doesn't require active_ref * active_ref. kernfs_rename() doesn't require active_ref
* protection. Break them before grabbing cgroup_tree_mutex. * protection. Break them before grabbing cgroup_mutex.
*/ */
kernfs_break_active_protection(new_parent); kernfs_break_active_protection(new_parent);
kernfs_break_active_protection(kn); kernfs_break_active_protection(kn);
mutex_lock(&cgroup_tree_mutex);
mutex_lock(&cgroup_mutex); mutex_lock(&cgroup_mutex);
ret = kernfs_rename(kn, new_parent, new_name_str); ret = kernfs_rename(kn, new_parent, new_name_str);
mutex_unlock(&cgroup_mutex); mutex_unlock(&cgroup_mutex);
mutex_unlock(&cgroup_tree_mutex);
kernfs_unbreak_active_protection(kn); kernfs_unbreak_active_protection(kn);
kernfs_unbreak_active_protection(new_parent); kernfs_unbreak_active_protection(new_parent);
@ -2944,7 +2916,6 @@ static int cgroup_addrm_files(struct cgroup *cgrp, struct cftype cfts[],
struct cftype *cft; struct cftype *cft;
int ret; int ret;
lockdep_assert_held(&cgroup_tree_mutex);
lockdep_assert_held(&cgroup_mutex); lockdep_assert_held(&cgroup_mutex);
for (cft = cfts; cft->name[0] != '\0'; cft++) { for (cft = cfts; cft->name[0] != '\0'; cft++) {
@ -2980,7 +2951,6 @@ static int cgroup_apply_cftypes(struct cftype *cfts, bool is_add)
struct cgroup_subsys_state *css; struct cgroup_subsys_state *css;
int ret = 0; int ret = 0;
lockdep_assert_held(&cgroup_tree_mutex);
lockdep_assert_held(&cgroup_mutex); lockdep_assert_held(&cgroup_mutex);
/* add/rm files for all cgroups created before */ /* add/rm files for all cgroups created before */
@ -3049,7 +3019,6 @@ static int cgroup_init_cftypes(struct cgroup_subsys *ss, struct cftype *cfts)
static int cgroup_rm_cftypes_locked(struct cftype *cfts) static int cgroup_rm_cftypes_locked(struct cftype *cfts)
{ {
lockdep_assert_held(&cgroup_tree_mutex);
lockdep_assert_held(&cgroup_mutex); lockdep_assert_held(&cgroup_mutex);
if (!cfts || !cfts[0].ss) if (!cfts || !cfts[0].ss)
@ -3076,11 +3045,9 @@ int cgroup_rm_cftypes(struct cftype *cfts)
{ {
int ret; int ret;
mutex_lock(&cgroup_tree_mutex);
mutex_lock(&cgroup_mutex); mutex_lock(&cgroup_mutex);
ret = cgroup_rm_cftypes_locked(cfts); ret = cgroup_rm_cftypes_locked(cfts);
mutex_unlock(&cgroup_mutex); mutex_unlock(&cgroup_mutex);
mutex_unlock(&cgroup_tree_mutex);
return ret; return ret;
} }
@ -3109,7 +3076,6 @@ int cgroup_add_cftypes(struct cgroup_subsys *ss, struct cftype *cfts)
if (ret) if (ret)
return ret; return ret;
mutex_lock(&cgroup_tree_mutex);
mutex_lock(&cgroup_mutex); mutex_lock(&cgroup_mutex);
list_add_tail(&cfts->node, &ss->cfts); list_add_tail(&cfts->node, &ss->cfts);
@ -3118,7 +3084,6 @@ int cgroup_add_cftypes(struct cgroup_subsys *ss, struct cftype *cfts)
cgroup_rm_cftypes_locked(cfts); cgroup_rm_cftypes_locked(cfts);
mutex_unlock(&cgroup_mutex); mutex_unlock(&cgroup_mutex);
mutex_unlock(&cgroup_tree_mutex);
return ret; return ret;
} }
@ -3158,7 +3123,7 @@ css_next_child(struct cgroup_subsys_state *pos_css,
struct cgroup *cgrp = parent_css->cgroup; struct cgroup *cgrp = parent_css->cgroup;
struct cgroup *next; struct cgroup *next;
cgroup_assert_mutexes_or_rcu_locked(); cgroup_assert_mutex_or_rcu_locked();
/* /*
* @pos could already have been removed. Once a cgroup is removed, * @pos could already have been removed. Once a cgroup is removed,
@ -3224,7 +3189,7 @@ css_next_descendant_pre(struct cgroup_subsys_state *pos,
{ {
struct cgroup_subsys_state *next; struct cgroup_subsys_state *next;
cgroup_assert_mutexes_or_rcu_locked(); cgroup_assert_mutex_or_rcu_locked();
/* if first iteration, visit @root */ /* if first iteration, visit @root */
if (!pos) if (!pos)
@ -3264,7 +3229,7 @@ css_rightmost_descendant(struct cgroup_subsys_state *pos)
{ {
struct cgroup_subsys_state *last, *tmp; struct cgroup_subsys_state *last, *tmp;
cgroup_assert_mutexes_or_rcu_locked(); cgroup_assert_mutex_or_rcu_locked();
do { do {
last = pos; last = pos;
@ -3311,7 +3276,7 @@ css_next_descendant_post(struct cgroup_subsys_state *pos,
{ {
struct cgroup_subsys_state *next; struct cgroup_subsys_state *next;
cgroup_assert_mutexes_or_rcu_locked(); cgroup_assert_mutex_or_rcu_locked();
/* if first iteration, visit leftmost descendant which may be @root */ /* if first iteration, visit leftmost descendant which may be @root */
if (!pos) if (!pos)
@ -4178,7 +4143,6 @@ static int online_css(struct cgroup_subsys_state *css)
struct cgroup_subsys *ss = css->ss; struct cgroup_subsys *ss = css->ss;
int ret = 0; int ret = 0;
lockdep_assert_held(&cgroup_tree_mutex);
lockdep_assert_held(&cgroup_mutex); lockdep_assert_held(&cgroup_mutex);
if (ss->css_online) if (ss->css_online)
@ -4196,7 +4160,6 @@ static void offline_css(struct cgroup_subsys_state *css)
{ {
struct cgroup_subsys *ss = css->ss; struct cgroup_subsys *ss = css->ss;
lockdep_assert_held(&cgroup_tree_mutex);
lockdep_assert_held(&cgroup_mutex); lockdep_assert_held(&cgroup_mutex);
if (!(css->flags & CSS_ONLINE)) if (!(css->flags & CSS_ONLINE))
@ -4399,7 +4362,6 @@ static void css_killed_work_fn(struct work_struct *work)
container_of(work, struct cgroup_subsys_state, destroy_work); container_of(work, struct cgroup_subsys_state, destroy_work);
struct cgroup *cgrp = css->cgroup; struct cgroup *cgrp = css->cgroup;
mutex_lock(&cgroup_tree_mutex);
mutex_lock(&cgroup_mutex); mutex_lock(&cgroup_mutex);
/* /*
@ -4417,7 +4379,6 @@ static void css_killed_work_fn(struct work_struct *work)
cgroup_destroy_css_killed(cgrp); cgroup_destroy_css_killed(cgrp);
mutex_unlock(&cgroup_mutex); mutex_unlock(&cgroup_mutex);
mutex_unlock(&cgroup_tree_mutex);
/* /*
* Put the css refs from kill_css(). Each css holds an extra * Put the css refs from kill_css(). Each css holds an extra
@ -4450,7 +4411,6 @@ static void css_killed_ref_fn(struct percpu_ref *ref)
*/ */
static void kill_css(struct cgroup_subsys_state *css) static void kill_css(struct cgroup_subsys_state *css)
{ {
lockdep_assert_held(&cgroup_tree_mutex);
lockdep_assert_held(&cgroup_mutex); lockdep_assert_held(&cgroup_mutex);
/* /*
@ -4510,7 +4470,6 @@ static int cgroup_destroy_locked(struct cgroup *cgrp)
bool empty; bool empty;
int ssid; int ssid;
lockdep_assert_held(&cgroup_tree_mutex);
lockdep_assert_held(&cgroup_mutex); lockdep_assert_held(&cgroup_mutex);
/* /*
@ -4593,7 +4552,6 @@ static void cgroup_destroy_css_killed(struct cgroup *cgrp)
{ {
struct cgroup *parent = cgrp->parent; struct cgroup *parent = cgrp->parent;
lockdep_assert_held(&cgroup_tree_mutex);
lockdep_assert_held(&cgroup_mutex); lockdep_assert_held(&cgroup_mutex);
/* delete this cgroup from parent->children */ /* delete this cgroup from parent->children */
@ -4647,7 +4605,6 @@ static void __init cgroup_init_subsys(struct cgroup_subsys *ss, bool early)
printk(KERN_INFO "Initializing cgroup subsys %s\n", ss->name); printk(KERN_INFO "Initializing cgroup subsys %s\n", ss->name);
mutex_lock(&cgroup_tree_mutex);
mutex_lock(&cgroup_mutex); mutex_lock(&cgroup_mutex);
idr_init(&ss->css_idr); idr_init(&ss->css_idr);
@ -4685,7 +4642,6 @@ static void __init cgroup_init_subsys(struct cgroup_subsys *ss, bool early)
cgrp_dfl_root.subsys_mask |= 1 << ss->id; cgrp_dfl_root.subsys_mask |= 1 << ss->id;
mutex_unlock(&cgroup_mutex); mutex_unlock(&cgroup_mutex);
mutex_unlock(&cgroup_tree_mutex);
} }
/** /**
@ -4735,7 +4691,6 @@ int __init cgroup_init(void)
BUG_ON(cgroup_init_cftypes(NULL, cgroup_base_files)); BUG_ON(cgroup_init_cftypes(NULL, cgroup_base_files));
mutex_lock(&cgroup_tree_mutex);
mutex_lock(&cgroup_mutex); mutex_lock(&cgroup_mutex);
/* Add init_css_set to the hash table */ /* Add init_css_set to the hash table */
@ -4745,7 +4700,6 @@ int __init cgroup_init(void)
BUG_ON(cgroup_setup_root(&cgrp_dfl_root, 0)); BUG_ON(cgroup_setup_root(&cgrp_dfl_root, 0));
mutex_unlock(&cgroup_mutex); mutex_unlock(&cgroup_mutex);
mutex_unlock(&cgroup_tree_mutex);
for_each_subsys(ss, ssid) { for_each_subsys(ss, ssid) {
if (ss->early_init) { if (ss->early_init) {