lkml.org 
[lkml]   [2010]   [Dec]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    Subject[PATCH v6 2/3] cgroups: add can_attach callback for checking all threads in a group
    Add cgroup wrapper for safely calling can_attach on all threads in a threadgroup

    From: Ben Blum <bblum@andrew.cmu.edu>

    This patch adds a function cgroup_can_attach_per_thread which handles iterating
    over each thread in a threadgroup safely with respect to the invariants that
    will be used in cgroup_attach_proc. Also, subsystems whose can_attach calls
    require per-thread validation are modified to use the per_thread wrapper to
    avoid duplicating cgroup-internal code.

    This is a pre-patch for cgroup-procs-writable.patch.

    Signed-off-by: Ben Blum <bblum@andrew.cmu.edu>
    ---
    block/blk-cgroup.c | 31 ++++++++++++++++++++++++++-----
    include/linux/cgroup.h | 14 ++++++++++++++
    kernel/cgroup.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
    kernel/cgroup_freezer.c | 33 ++++++++++++++-------------------
    kernel/cpuset.c | 30 ++++++++++--------------------
    kernel/ns_cgroup.c | 25 +++++++++----------------
    kernel/sched.c | 24 ++++++------------------
    7 files changed, 124 insertions(+), 78 deletions(-)

    diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
    index b1febd0..865e208 100644
    --- a/block/blk-cgroup.c
    +++ b/block/blk-cgroup.c
    @@ -1475,9 +1475,7 @@ done:
    * 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 blkiocg_can_attach(struct cgroup_subsys *subsys,
    - struct cgroup *cgroup, struct task_struct *tsk,
    - bool threadgroup)
    +static int blkiocg_can_attach_task(struct cgroup *cgrp, struct task_struct *tsk)
    {
    struct io_context *ioc;
    int ret = 0;
    @@ -1492,10 +1490,17 @@ static int blkiocg_can_attach(struct cgroup_subsys *subsys,
    return ret;
    }

    -static void blkiocg_attach(struct cgroup_subsys *subsys, struct cgroup *cgroup,
    - struct cgroup *prev, struct task_struct *tsk,
    +static int blkiocg_can_attach(struct cgroup_subsys *subsys,
    + struct cgroup *cgroup, struct task_struct *tsk,
    bool threadgroup)
    {
    + return cgroup_can_attach_per_thread(cgroup, tsk,
    + blkiocg_can_attach_task,
    + threadgroup, false);
    +}
    +
    +static void blkiocg_attach_task(struct task_struct *tsk)
    +{
    struct io_context *ioc;

    task_lock(tsk);
    @@ -1505,6 +1510,22 @@ static void blkiocg_attach(struct cgroup_subsys *subsys, struct cgroup *cgroup,
    task_unlock(tsk);
    }

    +static void blkiocg_attach(struct cgroup_subsys *subsys, struct cgroup *cgroup,
    + struct cgroup *prev, struct task_struct *tsk,
    + bool threadgroup)
    +{
    + blkiocg_attach_task(tsk);
    + if (threadgroup) {
    + struct task_struct *c;
    +
    + /* tasklist_lock will be held. */
    + BUG_ON(!thread_group_leader(tsk));
    + list_for_each_entry_rcu(c, &tsk->thread_group, thread_group) {
    + blkiocg_attach_task(c);
    + }
    + }
    +}
    +
    void blkio_policy_register(struct blkio_policy_type *blkiop)
    {
    spin_lock(&blkio_list_lock);
    diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
    index ce104e3..96898e6 100644
    --- a/include/linux/cgroup.h
    +++ b/include/linux/cgroup.h
    @@ -590,6 +590,20 @@ static inline int cgroup_attach_task_current_cg(struct task_struct *tsk)
    }

    /*
    + * For use in subsystems whose can_attach functions need to run an operation
    + * on every task in the threadgroup. Calls the given callback once if the
    + * 'threadgroup' flag is false, or once per thread in the group if true.
    + * The callback should return 0/-ERR; this will return 0/-ERR.
    + * The callback will run within an rcu_read section, so must not sleep.
    + * 'need_rcu' should specify whether the callback needs to run in an rcu_read
    + * section even in the single-task case.
    + */
    +int cgroup_can_attach_per_thread(struct cgroup *cgrp, struct task_struct *task,
    + int (*cb)(struct cgroup *cgrp,
    + struct task_struct *task),
    + bool threadgroup, bool need_rcu);
    +
    +/*
    * CSS ID is ID for cgroup_subsys_state structs under subsys. This only works
    * if cgroup_subsys.use_id == true. It can be used for looking up and scanning.
    * CSS ID is assigned at cgroup allocation (create) automatically
    diff --git a/kernel/cgroup.c b/kernel/cgroup.c
    index 66a416b..f86dd9c 100644
    --- a/kernel/cgroup.c
    +++ b/kernel/cgroup.c
    @@ -1726,6 +1726,51 @@ int cgroup_path(const struct cgroup *cgrp, char *buf, int buflen)
    }
    EXPORT_SYMBOL_GPL(cgroup_path);

    +int cgroup_can_attach_per_thread(struct cgroup *cgrp, struct task_struct *task,
    + int (*cb)(struct cgroup *cgrp,
    + struct task_struct *task),
    + bool threadgroup, bool need_rcu)
    +{
    + int ret;
    +
    + /* Run callback on the leader first, taking rcu_read_lock if needed. */
    + if (need_rcu)
    + rcu_read_lock();
    +
    + ret = cb(cgrp, task);
    +
    + if (need_rcu)
    + rcu_read_unlock();
    +
    + if (ret < 0)
    + return ret;
    +
    + /* Run on each task in the threadgroup. */
    + if (threadgroup) {
    + struct task_struct *c;
    +
    + rcu_read_lock();
    + /*
    + * It is necessary for the given task to still be the leader
    + * to safely traverse thread_group. See cgroup_attach_proc.
    + */
    + if (!thread_group_leader(task)) {
    + rcu_read_unlock();
    + return -EAGAIN;
    + }
    + list_for_each_entry_rcu(c, &task->thread_group, thread_group) {
    + ret = cb(cgrp, c);
    + if (ret < 0) {
    + rcu_read_unlock();
    + return ret;
    + }
    + }
    + rcu_read_unlock();
    + }
    + return 0;
    +}
    +EXPORT_SYMBOL_GPL(cgroup_can_attach_per_thread);
    +
    /**
    * cgroup_attach_task - attach task 'tsk' to cgroup 'cgrp'
    * @cgrp: the cgroup the task is attaching to
    diff --git a/kernel/cgroup_freezer.c b/kernel/cgroup_freezer.c
    index e7bebb7..1f5ac8f 100644
    --- a/kernel/cgroup_freezer.c
    +++ b/kernel/cgroup_freezer.c
    @@ -153,6 +153,13 @@ static void freezer_destroy(struct cgroup_subsys *ss,
    kfree(cgroup_freezer(cgroup));
    }

    +static int freezer_can_attach_cb(struct cgroup *cgrp, struct task_struct *task)
    +{
    + if (__cgroup_freezing_or_frozen(task))
    + return -EBUSY;
    + return 0;
    +}
    +
    /*
    * The call to cgroup_lock() in the freezer.state write method prevents
    * a write to that file racing against an attach, and hence the
    @@ -163,6 +170,7 @@ static int freezer_can_attach(struct cgroup_subsys *ss,
    struct task_struct *task, bool threadgroup)
    {
    struct freezer *freezer;
    + int ret;

    /*
    * Anything frozen can't move or be moved to/from.
    @@ -172,25 +180,12 @@ static int freezer_can_attach(struct cgroup_subsys *ss,
    if (freezer->state != CGROUP_THAWED)
    return -EBUSY;

    - rcu_read_lock();
    - if (__cgroup_freezing_or_frozen(task)) {
    - rcu_read_unlock();
    - return -EBUSY;
    - }
    - rcu_read_unlock();
    -
    - if (threadgroup) {
    - struct task_struct *c;
    -
    - rcu_read_lock();
    - list_for_each_entry_rcu(c, &task->thread_group, thread_group) {
    - if (__cgroup_freezing_or_frozen(c)) {
    - rcu_read_unlock();
    - return -EBUSY;
    - }
    - }
    - rcu_read_unlock();
    - }
    + /* Need to take rcu_read_lock even around the call on the leader. */
    + ret = cgroup_can_attach_per_thread(new_cgroup, task,
    + freezer_can_attach_cb, threadgroup,
    + true);
    + if (ret < 0)
    + return ret;

    return 0;
    }
    diff --git a/kernel/cpuset.c b/kernel/cpuset.c
    index 4349935..8fbe1e3 100644
    --- a/kernel/cpuset.c
    +++ b/kernel/cpuset.c
    @@ -1375,11 +1375,15 @@ static int fmeter_getrate(struct fmeter *fmp)
    /* Protected by cgroup_lock */
    static cpumask_var_t cpus_attach;

    +static int cpuset_can_attach_cb(struct cgroup *cgrp, struct task_struct *task)
    +{
    + return security_task_setscheduler(task);
    +}
    +
    /* Called by cgroups to determine if a cpuset is usable; cgroup_mutex held */
    static int cpuset_can_attach(struct cgroup_subsys *ss, struct cgroup *cont,
    struct task_struct *tsk, bool threadgroup)
    {
    - int ret;
    struct cpuset *cs = cgroup_cs(cont);

    if (cpumask_empty(cs->cpus_allowed) || nodes_empty(cs->mems_allowed))
    @@ -1396,23 +1400,8 @@ static int cpuset_can_attach(struct cgroup_subsys *ss, struct cgroup *cont,
    if (tsk->flags & PF_THREAD_BOUND)
    return -EINVAL;

    - ret = security_task_setscheduler(tsk);
    - if (ret)
    - return ret;
    - if (threadgroup) {
    - struct task_struct *c;
    -
    - rcu_read_lock();
    - list_for_each_entry_rcu(c, &tsk->thread_group, thread_group) {
    - ret = security_task_setscheduler(c);
    - if (ret) {
    - rcu_read_unlock();
    - return ret;
    - }
    - }
    - rcu_read_unlock();
    - }
    - return 0;
    + return cgroup_can_attach_per_thread(cont, tsk, cpuset_can_attach_cb,
    + threadgroup, false);
    }

    static void cpuset_attach_task(struct task_struct *tsk, nodemask_t *to,
    @@ -1455,11 +1444,12 @@ static void cpuset_attach(struct cgroup_subsys *ss, struct cgroup *cont,
    cpuset_attach_task(tsk, to, cs);
    if (threadgroup) {
    struct task_struct *c;
    - rcu_read_lock();
    +
    + /* tasklist_lock will be held. */
    + BUG_ON(!thread_group_leader(tsk));
    list_for_each_entry_rcu(c, &tsk->thread_group, thread_group) {
    cpuset_attach_task(c, to, cs);
    }
    - rcu_read_unlock();
    }

    /* change mm; only needs to be done once even if threadgroup */
    diff --git a/kernel/ns_cgroup.c b/kernel/ns_cgroup.c
    index 2c98ad9..66ba860 100644
    --- a/kernel/ns_cgroup.c
    +++ b/kernel/ns_cgroup.c
    @@ -42,6 +42,13 @@ int ns_cgroup_clone(struct task_struct *task, struct pid *pid)
    * (hence either you are in the same cgroup as task, or in an
    * ancestor cgroup thereof)
    */
    +static int ns_can_attach_cb(struct cgroup *new_cgroup, struct task_struct *task)
    +{
    + if (!cgroup_is_descendant(new_cgroup, task))
    + return -EPERM;
    + return 0;
    +}
    +
    static int ns_can_attach(struct cgroup_subsys *ss, struct cgroup *new_cgroup,
    struct task_struct *task, bool threadgroup)
    {
    @@ -53,22 +60,8 @@ static int ns_can_attach(struct cgroup_subsys *ss, struct cgroup *new_cgroup,
    return -EPERM;
    }

    - if (!cgroup_is_descendant(new_cgroup, task))
    - return -EPERM;
    -
    - if (threadgroup) {
    - struct task_struct *c;
    - rcu_read_lock();
    - list_for_each_entry_rcu(c, &task->thread_group, thread_group) {
    - if (!cgroup_is_descendant(new_cgroup, c)) {
    - rcu_read_unlock();
    - return -EPERM;
    - }
    - }
    - rcu_read_unlock();
    - }
    -
    - return 0;
    + return cgroup_can_attach_per_thread(new_cgroup, task, ns_can_attach_cb,
    + threadgroup, false);
    }

    /*
    diff --git a/kernel/sched.c b/kernel/sched.c
    index 218ef20..8e89bf9 100644
    --- a/kernel/sched.c
    +++ b/kernel/sched.c
    @@ -8659,22 +8659,9 @@ static int
    cpu_cgroup_can_attach(struct cgroup_subsys *ss, struct cgroup *cgrp,
    struct task_struct *tsk, bool threadgroup)
    {
    - int retval = cpu_cgroup_can_attach_task(cgrp, tsk);
    - if (retval)
    - return retval;
    - if (threadgroup) {
    - struct task_struct *c;
    - rcu_read_lock();
    - list_for_each_entry_rcu(c, &tsk->thread_group, thread_group) {
    - retval = cpu_cgroup_can_attach_task(cgrp, c);
    - if (retval) {
    - rcu_read_unlock();
    - return retval;
    - }
    - }
    - rcu_read_unlock();
    - }
    - return 0;
    + return cgroup_can_attach_per_thread(cgrp, tsk,
    + cpu_cgroup_can_attach_task,
    + threadgroup, false);
    }

    static void
    @@ -8685,11 +8672,12 @@ cpu_cgroup_attach(struct cgroup_subsys *ss, struct cgroup *cgrp,
    sched_move_task(tsk);
    if (threadgroup) {
    struct task_struct *c;
    - rcu_read_lock();
    +
    + /* tasklist_lock will be held. */
    + BUG_ON(!thread_group_leader(tsk));
    list_for_each_entry_rcu(c, &tsk->thread_group, thread_group) {
    sched_move_task(c);
    }
    - rcu_read_unlock();
    }
    }


    \
     
     \ /
      Last update: 2010-12-24 09:53    [W:4.142 / U:0.172 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site