Skip to content

Commit

Permalink
cgroup: pass around cgroup_subsys_state instead of cgroup in subsyste…
Browse files Browse the repository at this point in the history 
…m methods

cgroup is currently in the process of transitioning to using struct
cgroup_subsys_state * as the primary handle instead of struct cgroup *
in subsystem implementations for the following reasons.

* With unified hierarchy, subsystems will be dynamically bound and
  unbound from cgroups and thus css's (cgroup_subsys_state) may be
  created and destroyed dynamically over the lifetime of a cgroup,
  which is different from the current state where all css's are
  allocated and destroyed together with the associated cgroup.  This
  in turn means that cgroup_css() should be synchronized and may
  return NULL, making it more cumbersome to use.

* Differing levels of per-subsystem granularity in the unified
  hierarchy means that the task and descendant iterators should behave
  differently depending on the specific subsystem the iteration is
  being performed for.

* In majority of the cases, subsystems only care about its part in the
  cgroup hierarchy - ie. the hierarchy of css's.  Subsystem methods
  often obtain the matching css pointer from the cgroup and don't
  bother with the cgroup pointer itself.  Passing around css fits
  much better.

This patch converts all cgroup_subsys methods to take @css instead of
@CGROUP.  The conversions are mostly straight-forward.  A few
noteworthy changes are

* ->css_alloc() now takes css of the parent cgroup rather than the
  pointer to the new cgroup as the css for the new cgroup doesn't
  exist yet.  Knowing the parent css is enough for all the existing
  subsystems.

* In kernel/cgroup.c::offline_css(), unnecessary open coded css
  dereference is replaced with local variable access.

This patch shouldn't cause any behavior differences.

v2: Unnecessary explicit cgrp->subsys[] deref in css_online() replaced
    with local variable @css as suggested by Li Zefan.

    Rebased on top of new for-3.12 which includes for-3.11-fixes so
    that ->css_free() invocation added by da0a12c ("cgroup: fix a
    leak when percpu_ref_init() fails") is converted too.  Suggested
    by Li Zefan.

Signed-off-by: Tejun Heo <tj@kernel.org>
Acked-by: Li Zefan <lizefan@huawei.com>
Acked-by: Michal Hocko <mhocko@suse.cz>
Acked-by: Vivek Goyal <vgoyal@redhat.com>
Acked-by: Aristeu Rozanski <aris@redhat.com>
Acked-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Johannes Weiner <hannes@cmpxchg.org>
Cc: Balbir Singh <bsingharora@gmail.com>
Cc: Matt Helsley <matthltc@us.ibm.com>
Cc: Jens Axboe <axboe@kernel.dk>
Cc: Steven Rostedt <rostedt@goodmis.org>
  • Loading branch information
@htejun
htejun committed Aug 9, 2013
1 parent 6387698 commit eb95419
Show file tree
Hide file tree
Showing 13 changed files with 197 additions and 171 deletions.
25 changes: 13 additions & 12 deletions block/blk-cgroup.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -765,18 +765,18 @@ struct cftype blkcg_files[] = {

/**
* blkcg_css_offline - cgroup css_offline callback
* @cgroup: cgroup of interest
* @css: css of interest
*
* This function is called when @cgroup is about to go away and responsible
* for shooting down all blkgs associated with @cgroup. blkgs should be
* This function is called when @css is about to go away and responsible
* for shooting down all blkgs associated with @css. blkgs should be
* removed while holding both q and blkcg locks. As blkcg lock is nested
* inside q lock, this function performs reverse double lock dancing.
*
* This is the blkcg counterpart of ioc_release_fn().
*/
static void blkcg_css_offline(struct cgroup *cgroup)
static void blkcg_css_offline(struct cgroup_subsys_state *css)
{
struct blkcg *blkcg = cgroup_to_blkcg(cgroup);
struct blkcg *blkcg = css_to_blkcg(css);

spin_lock_irq(&blkcg->lock);

Expand All @@ -798,21 +798,21 @@ static void blkcg_css_offline(struct cgroup *cgroup)
spin_unlock_irq(&blkcg->lock);
}

static void blkcg_css_free(struct cgroup *cgroup)
static void blkcg_css_free(struct cgroup_subsys_state *css)
{
struct blkcg *blkcg = cgroup_to_blkcg(cgroup);
struct blkcg *blkcg = css_to_blkcg(css);

if (blkcg != &blkcg_root)
kfree(blkcg);
}

static struct cgroup_subsys_state *blkcg_css_alloc(struct cgroup *cgroup)
static struct cgroup_subsys_state *
blkcg_css_alloc(struct cgroup_subsys_state *parent_css)
{
static atomic64_t id_seq = ATOMIC64_INIT(0);
struct blkcg *blkcg;
struct cgroup *parent = cgroup->parent;

if (!parent) {
if (!parent_css) {
blkcg = &blkcg_root;
goto done;
}
Expand Down Expand Up @@ -883,14 +883,15 @@ void blkcg_exit_queue(struct request_queue *q)
* of the main cic data structures. For now we allow a task to change
* its cgroup only if it's the only owner of its ioc.
*/
static int blkcg_can_attach(struct cgroup *cgrp, struct cgroup_taskset *tset)
static int blkcg_can_attach(struct cgroup_subsys_state *css,
struct cgroup_taskset *tset)
{
struct task_struct *task;
struct io_context *ioc;
int ret = 0;

/* task_lock() is needed to avoid races with exit_io_context() */
cgroup_taskset_for_each(task, cgrp, tset) {
cgroup_taskset_for_each(task, css->cgroup, tset) {
task_lock(task);
ioc = task->io_context;
if (ioc && atomic_read(&ioc->nr_tasks) > 1)
Expand Down
24 changes: 14 additions & 10 deletions include/linux/cgroup.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -579,18 +579,22 @@ int cgroup_taskset_size(struct cgroup_taskset *tset);
*/

struct cgroup_subsys {
struct cgroup_subsys_state *(*css_alloc)(struct cgroup *cgrp);
int (*css_online)(struct cgroup *cgrp);
void (*css_offline)(struct cgroup *cgrp);
void (*css_free)(struct cgroup *cgrp);

int (*can_attach)(struct cgroup *cgrp, struct cgroup_taskset *tset);
void (*cancel_attach)(struct cgroup *cgrp, struct cgroup_taskset *tset);
void (*attach)(struct cgroup *cgrp, struct cgroup_taskset *tset);
struct cgroup_subsys_state *(*css_alloc)(struct cgroup_subsys_state *parent_css);
int (*css_online)(struct cgroup_subsys_state *css);
void (*css_offline)(struct cgroup_subsys_state *css);
void (*css_free)(struct cgroup_subsys_state *css);

int (*can_attach)(struct cgroup_subsys_state *css,
struct cgroup_taskset *tset);
void (*cancel_attach)(struct cgroup_subsys_state *css,
struct cgroup_taskset *tset);
void (*attach)(struct cgroup_subsys_state *css,
struct cgroup_taskset *tset);
void (*fork)(struct task_struct *task);
void (*exit)(struct cgroup *cgrp, struct cgroup *old_cgrp,
void (*exit)(struct cgroup_subsys_state *css,
struct cgroup_subsys_state *old_css,
struct task_struct *task);
void (*bind)(struct cgroup *root);
void (*bind)(struct cgroup_subsys_state *root_css);

int subsys_id;
int disabled;
Expand Down
57 changes: 34 additions & 23 deletions kernel/cgroup.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -853,8 +853,11 @@ static void cgroup_free_fn(struct work_struct *work)
/*
* Release the subsystem state objects.
*/
for_each_root_subsys(cgrp->root, ss)
ss->css_free(cgrp);
for_each_root_subsys(cgrp->root, ss) {
struct cgroup_subsys_state *css = cgrp->subsys[ss->subsys_id];

ss->css_free(css);
}

cgrp->root->number_of_cgroups--;
mutex_unlock(&cgroup_mutex);
Expand Down Expand Up @@ -1056,7 +1059,7 @@ static int rebind_subsystems(struct cgroupfs_root *root,
list_move(&ss->sibling, &root->subsys_list);
ss->root = root;
if (ss->bind)
ss->bind(cgrp);
ss->bind(cgrp->subsys[i]);

/* refcount was already taken, and we're keeping it */
root->subsys_mask |= bit;
Expand All @@ -1066,7 +1069,7 @@ static int rebind_subsystems(struct cgroupfs_root *root,
BUG_ON(cgrp->subsys[i]->cgroup != cgrp);

if (ss->bind)
ss->bind(cgroup_dummy_top);
ss->bind(cgroup_dummy_top->subsys[i]);
cgroup_dummy_top->subsys[i]->cgroup = cgroup_dummy_top;
cgrp->subsys[i] = NULL;
cgroup_subsys[i]->root = &cgroup_dummy_root;
Expand Down Expand Up @@ -2049,8 +2052,10 @@ static int cgroup_attach_task(struct cgroup *cgrp, struct task_struct *tsk,
* step 1: check that we can legitimately attach to the cgroup.
*/
for_each_root_subsys(root, ss) {
struct cgroup_subsys_state *css = cgrp->subsys[ss->subsys_id];

if (ss->can_attach) {
retval = ss->can_attach(cgrp, &tset);
retval = ss->can_attach(css, &tset);
if (retval) {
failed_ss = ss;
goto out_cancel_attach;
Expand Down Expand Up @@ -2089,8 +2094,10 @@ static int cgroup_attach_task(struct cgroup *cgrp, struct task_struct *tsk,
* step 4: do subsystem attach callbacks.
*/
for_each_root_subsys(root, ss) {
struct cgroup_subsys_state *css = cgrp->subsys[ss->subsys_id];

if (ss->attach)
ss->attach(cgrp, &tset);
ss->attach(css, &tset);
}

/*
Expand All @@ -2109,10 +2116,12 @@ static int cgroup_attach_task(struct cgroup *cgrp, struct task_struct *tsk,
out_cancel_attach:
if (retval) {
for_each_root_subsys(root, ss) {
struct cgroup_subsys_state *css = cgrp->subsys[ss->subsys_id];

if (ss == failed_ss)
break;
if (ss->cancel_attach)
ss->cancel_attach(cgrp, &tset);
ss->cancel_attach(css, &tset);
}
}
out_free_group_list:
Expand Down Expand Up @@ -4206,14 +4215,15 @@ static void init_cgroup_css(struct cgroup_subsys_state *css,
/* invoke ->css_online() on a new CSS and mark it online if successful */
static int online_css(struct cgroup_subsys *ss, struct cgroup *cgrp)
{
struct cgroup_subsys_state *css = cgrp->subsys[ss->subsys_id];
int ret = 0;

lockdep_assert_held(&cgroup_mutex);

if (ss->css_online)
ret = ss->css_online(cgrp);
ret = ss->css_online(css);
if (!ret)
cgrp->subsys[ss->subsys_id]->flags |= CSS_ONLINE;
css->flags |= CSS_ONLINE;
return ret;
}

Expand All @@ -4228,9 +4238,9 @@ static void offline_css(struct cgroup_subsys *ss, struct cgroup *cgrp)
return;

if (ss->css_offline)
ss->css_offline(cgrp);
ss->css_offline(css);

cgrp->subsys[ss->subsys_id]->flags &= ~CSS_ONLINE;
css->flags &= ~CSS_ONLINE;
}

/*
Expand Down Expand Up @@ -4305,15 +4315,15 @@ static long cgroup_create(struct cgroup *parent, struct dentry *dentry,
for_each_root_subsys(root, ss) {
struct cgroup_subsys_state *css;

css = ss->css_alloc(cgrp);
css = ss->css_alloc(parent->subsys[ss->subsys_id]);
if (IS_ERR(css)) {
err = PTR_ERR(css);
goto err_free_all;
}

err = percpu_ref_init(&css->refcnt, css_release);
if (err) {
ss->css_free(cgrp);
ss->css_free(css);
goto err_free_all;
}

Expand Down Expand Up @@ -4386,7 +4396,7 @@ static long cgroup_create(struct cgroup *parent, struct dentry *dentry,

if (css) {
percpu_ref_cancel_init(&css->refcnt);
ss->css_free(cgrp);
ss->css_free(css);
}
}
mutex_unlock(&cgroup_mutex);
Expand Down Expand Up @@ -4641,7 +4651,7 @@ static void __init cgroup_init_subsys(struct cgroup_subsys *ss)
/* Create the top cgroup state for this subsystem */
list_add(&ss->sibling, &cgroup_dummy_root.subsys_list);
ss->root = &cgroup_dummy_root;
css = ss->css_alloc(cgroup_dummy_top);
css = ss->css_alloc(cgroup_dummy_top->subsys[ss->subsys_id]);
/* We don't handle early failures gracefully */
BUG_ON(IS_ERR(css));
init_cgroup_css(css, ss, cgroup_dummy_top);
Expand Down Expand Up @@ -4720,7 +4730,7 @@ int __init_or_module cgroup_load_subsys(struct cgroup_subsys *ss)
* struct, so this can happen first (i.e. before the dummy root
* attachment).
*/
css = ss->css_alloc(cgroup_dummy_top);
css = ss->css_alloc(cgroup_dummy_top->subsys[ss->subsys_id]);
if (IS_ERR(css)) {
/* failure case - need to deassign the cgroup_subsys[] slot. */
cgroup_subsys[ss->subsys_id] = NULL;
Expand Down Expand Up @@ -4836,7 +4846,7 @@ void cgroup_unload_subsys(struct cgroup_subsys *ss)
* the cgrp->subsys pointer to find their state. note that this
* also takes care of freeing the css_id.
*/
ss->css_free(cgroup_dummy_top);
ss->css_free(cgroup_dummy_top->subsys[ss->subsys_id]);
cgroup_dummy_top->subsys[ss->subsys_id] = NULL;

mutex_unlock(&cgroup_mutex);
Expand Down Expand Up @@ -5192,10 +5202,10 @@ void cgroup_exit(struct task_struct *tsk, int run_callbacks)
*/
for_each_builtin_subsys(ss, i) {
if (ss->exit) {
struct cgroup *old_cgrp = cset->subsys[i]->cgroup;
struct cgroup *cgrp = task_cgroup(tsk, i);
struct cgroup_subsys_state *old_css = cset->subsys[i];
struct cgroup_subsys_state *css = task_css(tsk, i);

ss->exit(cgrp, old_cgrp, tsk);
ss->exit(css, old_css, tsk);
}
}
}
Expand Down Expand Up @@ -5529,7 +5539,8 @@ struct cgroup_subsys_state *cgroup_css_from_dir(struct file *f, int id)
}

#ifdef CONFIG_CGROUP_DEBUG
static struct cgroup_subsys_state *debug_css_alloc(struct cgroup *cgrp)
static struct cgroup_subsys_state *
debug_css_alloc(struct cgroup_subsys_state *parent_css)
{
struct cgroup_subsys_state *css = kzalloc(sizeof(*css), GFP_KERNEL);

Expand All @@ -5539,9 +5550,9 @@ static struct cgroup_subsys_state *debug_css_alloc(struct cgroup *cgrp)
return css;
}

static void debug_css_free(struct cgroup *cgrp)
static void debug_css_free(struct cgroup_subsys_state *css)
{
kfree(cgrp->subsys[debug_subsys_id]);
kfree(css);
}

static u64 debug_taskcount_read(struct cgroup *cgrp, struct cftype *cft)
Expand Down
40 changes: 21 additions & 19 deletions kernel/cgroup_freezer.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,8 @@ static const char *freezer_state_strs(unsigned int state)

struct cgroup_subsys freezer_subsys;

static struct cgroup_subsys_state *freezer_css_alloc(struct cgroup *cgroup)
static struct cgroup_subsys_state *
freezer_css_alloc(struct cgroup_subsys_state *parent_css)
{
struct freezer *freezer;

Expand All @@ -104,16 +105,16 @@ static struct cgroup_subsys_state *freezer_css_alloc(struct cgroup *cgroup)
}

/**
* freezer_css_online - commit creation of a freezer cgroup
* @cgroup: cgroup being created
* freezer_css_online - commit creation of a freezer css
* @css: css being created
*
* We're committing to creation of @cgroup. Mark it online and inherit
* We're committing to creation of @css. Mark it online and inherit
* parent's freezing state while holding both parent's and our
* freezer->lock.
*/
static int freezer_css_online(struct cgroup *cgroup)
static int freezer_css_online(struct cgroup_subsys_state *css)
{
struct freezer *freezer = cgroup_freezer(cgroup);
struct freezer *freezer = css_freezer(css);
struct freezer *parent = parent_freezer(freezer);

/*
Expand All @@ -140,15 +141,15 @@ static int freezer_css_online(struct cgroup *cgroup)
}

/**
* freezer_css_offline - initiate destruction of @cgroup
* @cgroup: cgroup being destroyed
* freezer_css_offline - initiate destruction of a freezer css
* @css: css being destroyed
*
* @cgroup is going away. Mark it dead and decrement system_freezing_count
* if it was holding one.
* @css is going away. Mark it dead and decrement system_freezing_count if
* it was holding one.
*/
static void freezer_css_offline(struct cgroup *cgroup)
static void freezer_css_offline(struct cgroup_subsys_state *css)
{
struct freezer *freezer = cgroup_freezer(cgroup);
struct freezer *freezer = css_freezer(css);

spin_lock_irq(&freezer->lock);

Expand All @@ -160,9 +161,9 @@ static void freezer_css_offline(struct cgroup *cgroup)
spin_unlock_irq(&freezer->lock);
}

static void freezer_css_free(struct cgroup *cgroup)
static void freezer_css_free(struct cgroup_subsys_state *css)
{
kfree(cgroup_freezer(cgroup));
kfree(css_freezer(css));
}

/*
Expand All @@ -174,25 +175,26 @@ static void freezer_css_free(struct cgroup *cgroup)
* @freezer->lock. freezer_attach() makes the new tasks conform to the
* current state and all following state changes can see the new tasks.
*/
static void freezer_attach(struct cgroup *new_cgrp, struct cgroup_taskset *tset)
static void freezer_attach(struct cgroup_subsys_state *new_css,
struct cgroup_taskset *tset)
{
struct freezer *freezer = cgroup_freezer(new_cgrp);
struct freezer *freezer = css_freezer(new_css);
struct task_struct *task;
bool clear_frozen = false;

spin_lock_irq(&freezer->lock);

/*
* Make the new tasks conform to the current state of @new_cgrp.
* Make the new tasks conform to the current state of @new_css.
* For simplicity, when migrating any task to a FROZEN cgroup, we
* revert it to FREEZING and let update_if_frozen() determine the
* correct state later.
*
* Tasks in @tset are on @new_cgrp but may not conform to its
* Tasks in @tset are on @new_css but may not conform to its
* current state before executing the following - !frozen tasks may
* be visible in a FROZEN cgroup and frozen tasks in a THAWED one.
*/
cgroup_taskset_for_each(task, new_cgrp, tset) {
cgroup_taskset_for_each(task, new_css->cgroup, tset) {
if (!(freezer->state & CGROUP_FREEZING)) {
__thaw_task(task);
} else {
Expand Down
Loading

0 comments on commit eb95419

Please sign in to comment.