Skip to content

Commit

Permalink
cgroup: no need to check css refs for release notification
Browse files Browse the repository at this point in the history
We no longer fail rmdir() when there're still css refs, so we don't
need to check css refs in check_for_release().

This also voids a bug. cgroup_has_css_refs() accesses subsys[i]
without cgroup_mutex, so it can race with cgroup_unload_subsys().

cgroup_has_css_refs()
...
  if (ss == NULL || ss->root != cgrp->root)

if ss pointers to net_cls_subsys, and cls_cgroup module is unloaded
right after the former check but before the latter, the memory that
net_cls_subsys resides has become invalid.

Signed-off-by: Li Zefan <lizefan@huawei.com>
Signed-off-by: Tejun Heo <tj@kernel.org>
  • Loading branch information
Li Zefan authored and Tejun Heo committed Mar 4, 2013
1 parent f440d98 commit f50daa7
Showing 1 changed file with 8 additions and 59 deletions.
67 changes: 8 additions & 59 deletions kernel/cgroup.c
Original file line number Diff line number Diff line change
Expand Up @@ -4341,47 +4341,6 @@ static int cgroup_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode)
return cgroup_create(c_parent, dentry, mode | S_IFDIR);
}

/*
* Check the reference count on each subsystem. Since we already
* established that there are no tasks in the cgroup, if the css refcount
* is also 1, then there should be no outstanding references, so the
* subsystem is safe to destroy. We scan across all subsystems rather than
* using the per-hierarchy linked list of mounted subsystems since we can
* be called via check_for_release() with no synchronization other than
* RCU, and the subsystem linked list isn't RCU-safe.
*/
static int cgroup_has_css_refs(struct cgroup *cgrp)
{
int i;

/*
* We won't need to lock the subsys array, because the subsystems
* we're concerned about aren't going anywhere since our cgroup root
* has a reference on them.
*/
for (i = 0; i < CGROUP_SUBSYS_COUNT; i++) {
struct cgroup_subsys *ss = subsys[i];
struct cgroup_subsys_state *css;

/* Skip subsystems not present or not in this hierarchy */
if (ss == NULL || ss->root != cgrp->root)
continue;

css = cgrp->subsys[ss->subsys_id];
/*
* When called from check_for_release() it's possible
* that by this point the cgroup has been removed
* and the css deleted. But a false-positive doesn't
* matter, since it can only happen if the cgroup
* has been deleted and hence no longer needs the
* release agent to be called anyway.
*/
if (css && css_refcnt(css) > 1)
return 1;
}
return 0;
}

static int cgroup_destroy_locked(struct cgroup *cgrp)
__releases(&cgroup_mutex) __acquires(&cgroup_mutex)
{
Expand Down Expand Up @@ -5108,12 +5067,15 @@ static void check_for_release(struct cgroup *cgrp)
{
/* All of these checks rely on RCU to keep the cgroup
* structure alive */
if (cgroup_is_releasable(cgrp) && !atomic_read(&cgrp->count)
&& list_empty(&cgrp->children) && !cgroup_has_css_refs(cgrp)) {
/* Control Group is currently removeable. If it's not
if (cgroup_is_releasable(cgrp) &&
!atomic_read(&cgrp->count) && list_empty(&cgrp->children)) {
/*
* Control Group is currently removeable. If it's not
* already queued for a userspace notification, queue
* it now */
* it now
*/
int need_schedule_work = 0;

raw_spin_lock(&release_list_lock);
if (!cgroup_is_removed(cgrp) &&
list_empty(&cgrp->release_list)) {
Expand Down Expand Up @@ -5146,24 +5108,11 @@ EXPORT_SYMBOL_GPL(__css_tryget);
/* Caller must verify that the css is not for root cgroup */
void __css_put(struct cgroup_subsys_state *css)
{
struct cgroup *cgrp = css->cgroup;
int v;

rcu_read_lock();
v = css_unbias_refcnt(atomic_dec_return(&css->refcnt));

switch (v) {
case 1:
if (notify_on_release(cgrp)) {
set_bit(CGRP_RELEASABLE, &cgrp->flags);
check_for_release(cgrp);
}
break;
case 0:
if (v == 0)
schedule_work(&css->dput_work);
break;
}
rcu_read_unlock();
}
EXPORT_SYMBOL_GPL(__css_put);

Expand Down

0 comments on commit f50daa7

Please sign in to comment.