lkml.org 
[lkml]   [2015]   [Sep]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    Subject[PATCH cgroup/for-4.3-fixes 2/2] Revert "sched, cgroup: replace signal_struct->group_rwsem with a global percpu_rwsem"
    From 0c986253b939cc14c69d4adbe2b4121bdf4aa220 Mon Sep 17 00:00:00 2001
    From: Tejun Heo <tj@kernel.org>
    Date: Wed, 16 Sep 2015 11:51:12 -0400

    This reverts commit d59cfc09c32a2ae31f1c3bc2983a0cd79afb3f14.

    d59cfc09c32a ("sched, cgroup: replace signal_struct->group_rwsem with
    a global percpu_rwsem") and b5ba75b5fc0e ("cgroup: simplify
    threadgroup locking") changed how cgroup synchronizes against task
    fork and exits so that it uses global percpu_rwsem instead of
    per-process rwsem; unfortunately, the write [un]lock paths of
    percpu_rwsem always involve synchronize_rcu_expedited() which turned
    out to be too expensive.

    Improvements for percpu_rwsem are scheduled to be merged in the coming
    v4.4-rc1 merge window which alleviates this issue. For now, revert
    the two commits to restore per-process rwsem. They will be re-applied
    for the v4.4-rc1 merge window.

    Signed-off-by: Tejun Heo <tj@kernel.org>
    Link: http://lkml.kernel.org/g/55F8097A.7000206@de.ibm.com
    Reported-by: Christian Borntraeger <borntraeger@de.ibm.com>
    Cc: Oleg Nesterov <oleg@redhat.com>
    Cc: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
    Cc: Peter Zijlstra <peterz@infradead.org>
    Cc: Paolo Bonzini <pbonzini@redhat.com>
    Cc: stable@vger.kernel.org # v4.2+
    ---
    include/linux/cgroup-defs.h | 27 ++--------------
    include/linux/init_task.h | 8 +++++
    include/linux/sched.h | 12 +++++++
    kernel/cgroup.c | 77 +++++++++++++++++++++++++++++++++------------
    kernel/fork.c | 4 +++
    5 files changed, 83 insertions(+), 45 deletions(-)

    diff --git a/include/linux/cgroup-defs.h b/include/linux/cgroup-defs.h
    index 4d8fcf2..8492721 100644
    --- a/include/linux/cgroup-defs.h
    +++ b/include/linux/cgroup-defs.h
    @@ -473,31 +473,8 @@ struct cgroup_subsys {
    unsigned int depends_on;
    };

    -extern struct percpu_rw_semaphore cgroup_threadgroup_rwsem;
    -
    -/**
    - * cgroup_threadgroup_change_begin - threadgroup exclusion for cgroups
    - * @tsk: target task
    - *
    - * Called from threadgroup_change_begin() and allows cgroup operations to
    - * synchronize against threadgroup changes using a percpu_rw_semaphore.
    - */
    -static inline void cgroup_threadgroup_change_begin(struct task_struct *tsk)
    -{
    - percpu_down_read(&cgroup_threadgroup_rwsem);
    -}
    -
    -/**
    - * cgroup_threadgroup_change_end - threadgroup exclusion for cgroups
    - * @tsk: target task
    - *
    - * Called from threadgroup_change_end(). Counterpart of
    - * cgroup_threadcgroup_change_begin().
    - */
    -static inline void cgroup_threadgroup_change_end(struct task_struct *tsk)
    -{
    - percpu_up_read(&cgroup_threadgroup_rwsem);
    -}
    +void cgroup_threadgroup_change_begin(struct task_struct *tsk);
    +void cgroup_threadgroup_change_end(struct task_struct *tsk);

    #else /* CONFIG_CGROUPS */

    diff --git a/include/linux/init_task.h b/include/linux/init_task.h
    index d0b380e..e38681f 100644
    --- a/include/linux/init_task.h
    +++ b/include/linux/init_task.h
    @@ -25,6 +25,13 @@
    extern struct files_struct init_files;
    extern struct fs_struct init_fs;

    +#ifdef CONFIG_CGROUPS
    +#define INIT_GROUP_RWSEM(sig) \
    + .group_rwsem = __RWSEM_INITIALIZER(sig.group_rwsem),
    +#else
    +#define INIT_GROUP_RWSEM(sig)
    +#endif
    +
    #ifdef CONFIG_CPUSETS
    #define INIT_CPUSET_SEQ(tsk) \
    .mems_allowed_seq = SEQCNT_ZERO(tsk.mems_allowed_seq),
    @@ -57,6 +64,7 @@ extern struct fs_struct init_fs;
    INIT_PREV_CPUTIME(sig) \
    .cred_guard_mutex = \
    __MUTEX_INITIALIZER(sig.cred_guard_mutex), \
    + INIT_GROUP_RWSEM(sig) \
    }

    extern struct nsproxy init_nsproxy;
    diff --git a/include/linux/sched.h b/include/linux/sched.h
    index a4ab9da..b7b9501 100644
    --- a/include/linux/sched.h
    +++ b/include/linux/sched.h
    @@ -762,6 +762,18 @@ struct signal_struct {
    unsigned audit_tty_log_passwd;
    struct tty_audit_buf *tty_audit_buf;
    #endif
    +#ifdef CONFIG_CGROUPS
    + /*
    + * group_rwsem prevents new tasks from entering the threadgroup and
    + * member tasks from exiting,a more specifically, setting of
    + * PF_EXITING. fork and exit paths are protected with this rwsem
    + * using threadgroup_change_begin/end(). Users which require
    + * threadgroup to remain stable should use threadgroup_[un]lock()
    + * which also takes care of exec path. Currently, cgroup is the
    + * only user.
    + */
    + struct rw_semaphore group_rwsem;
    +#endif

    oom_flags_t oom_flags;
    short oom_score_adj; /* OOM kill score adjustment */
    diff --git a/kernel/cgroup.c b/kernel/cgroup.c
    index 115091e..2c9eae6 100644
    --- a/kernel/cgroup.c
    +++ b/kernel/cgroup.c
    @@ -46,7 +46,6 @@
    #include <linux/slab.h>
    #include <linux/spinlock.h>
    #include <linux/rwsem.h>
    -#include <linux/percpu-rwsem.h>
    #include <linux/string.h>
    #include <linux/sort.h>
    #include <linux/kmod.h>
    @@ -104,8 +103,6 @@ static DEFINE_SPINLOCK(cgroup_idr_lock);
    */
    static DEFINE_SPINLOCK(release_agent_path_lock);

    -struct percpu_rw_semaphore cgroup_threadgroup_rwsem;
    -
    #define cgroup_assert_mutex_or_rcu_locked() \
    RCU_LOCKDEP_WARN(!rcu_read_lock_held() && \
    !lockdep_is_held(&cgroup_mutex), \
    @@ -874,6 +871,48 @@ static struct css_set *find_css_set(struct css_set *old_cset,
    return cset;
    }

    +void cgroup_threadgroup_change_begin(struct task_struct *tsk)
    +{
    + down_read(&tsk->signal->group_rwsem);
    +}
    +
    +void cgroup_threadgroup_change_end(struct task_struct *tsk)
    +{
    + up_read(&tsk->signal->group_rwsem);
    +}
    +
    +/**
    + * threadgroup_lock - lock threadgroup
    + * @tsk: member task of the threadgroup to lock
    + *
    + * Lock the threadgroup @tsk belongs to. No new task is allowed to enter
    + * and member tasks aren't allowed to exit (as indicated by PF_EXITING) or
    + * change ->group_leader/pid. This is useful for cases where the threadgroup
    + * needs to stay stable across blockable operations.
    + *
    + * fork and exit explicitly call threadgroup_change_{begin|end}() for
    + * synchronization. While held, no new task will be added to threadgroup
    + * and no existing live task will have its PF_EXITING set.
    + *
    + * de_thread() does threadgroup_change_{begin|end}() when a non-leader
    + * sub-thread becomes a new leader.
    + */
    +static void threadgroup_lock(struct task_struct *tsk)
    +{
    + down_write(&tsk->signal->group_rwsem);
    +}
    +
    +/**
    + * threadgroup_unlock - unlock threadgroup
    + * @tsk: member task of the threadgroup to unlock
    + *
    + * Reverse threadgroup_lock().
    + */
    +static inline void threadgroup_unlock(struct task_struct *tsk)
    +{
    + up_write(&tsk->signal->group_rwsem);
    +}
    +
    static struct cgroup_root *cgroup_root_from_kf(struct kernfs_root *kf_root)
    {
    struct cgroup *root_cgrp = kf_root->kn->priv;
    @@ -2074,9 +2113,9 @@ static void cgroup_task_migrate(struct cgroup *old_cgrp,
    lockdep_assert_held(&css_set_rwsem);

    /*
    - * We are synchronized through cgroup_threadgroup_rwsem against
    - * PF_EXITING setting such that we can't race against cgroup_exit()
    - * changing the css_set to init_css_set and dropping the old one.
    + * We are synchronized through threadgroup_lock() against PF_EXITING
    + * setting such that we can't race against cgroup_exit() changing the
    + * css_set to init_css_set and dropping the old one.
    */
    WARN_ON_ONCE(tsk->flags & PF_EXITING);
    old_cset = task_css_set(tsk);
    @@ -2133,11 +2172,10 @@ static void cgroup_migrate_finish(struct list_head *preloaded_csets)
    * @src_cset and add it to @preloaded_csets, which should later be cleaned
    * up by cgroup_migrate_finish().
    *
    - * This function may be called without holding cgroup_threadgroup_rwsem
    - * even if the target is a process. Threads may be created and destroyed
    - * but as long as cgroup_mutex is not dropped, no new css_set can be put
    - * into play and the preloaded css_sets are guaranteed to cover all
    - * migrations.
    + * This function may be called without holding threadgroup_lock even if the
    + * target is a process. Threads may be created and destroyed but as long
    + * as cgroup_mutex is not dropped, no new css_set can be put into play and
    + * the preloaded css_sets are guaranteed to cover all migrations.
    */
    static void cgroup_migrate_add_src(struct css_set *src_cset,
    struct cgroup *dst_cgrp,
    @@ -2240,7 +2278,7 @@ static int cgroup_migrate_prepare_dst(struct cgroup *dst_cgrp,
    * @threadgroup: whether @leader points to the whole process or a single task
    *
    * Migrate a process or task denoted by @leader to @cgrp. If migrating a
    - * process, the caller must be holding cgroup_threadgroup_rwsem. The
    + * process, the caller must be holding threadgroup_lock of @leader. The
    * caller is also responsible for invoking cgroup_migrate_add_src() and
    * cgroup_migrate_prepare_dst() on the targets before invoking this
    * function and following up with cgroup_migrate_finish().
    @@ -2368,7 +2406,7 @@ static int cgroup_migrate(struct cgroup *cgrp, struct task_struct *leader,
    * @leader: the task or the leader of the threadgroup to be attached
    * @threadgroup: attach the whole threadgroup?
    *
    - * Call holding cgroup_mutex and cgroup_threadgroup_rwsem.
    + * Call holding cgroup_mutex and threadgroup_lock of @leader.
    */
    static int cgroup_attach_task(struct cgroup *dst_cgrp,
    struct task_struct *leader, bool threadgroup)
    @@ -2490,7 +2528,7 @@ static ssize_t __cgroup_procs_write(struct kernfs_open_file *of, char *buf,
    get_task_struct(tsk);
    rcu_read_unlock();

    - percpu_down_write(&cgroup_threadgroup_rwsem);
    + threadgroup_lock(tsk);
    if (threadgroup) {
    if (!thread_group_leader(tsk)) {
    /*
    @@ -2500,7 +2538,7 @@ static ssize_t __cgroup_procs_write(struct kernfs_open_file *of, char *buf,
    * try again; this is
    * "double-double-toil-and-trouble-check locking".
    */
    - percpu_up_write(&cgroup_threadgroup_rwsem);
    + threadgroup_unlock(tsk);
    put_task_struct(tsk);
    goto retry_find_task;
    }
    @@ -2510,7 +2548,7 @@ static ssize_t __cgroup_procs_write(struct kernfs_open_file *of, char *buf,
    if (!ret)
    ret = cgroup_attach_task(cgrp, tsk, threadgroup);

    - percpu_up_write(&cgroup_threadgroup_rwsem);
    + threadgroup_unlock(tsk);

    put_task_struct(tsk);
    out_unlock_cgroup:
    @@ -2713,17 +2751,17 @@ static int cgroup_update_dfl_csses(struct cgroup *cgrp)
    goto out_finish;
    last_task = task;

    - percpu_down_write(&cgroup_threadgroup_rwsem);
    + threadgroup_lock(task);
    /* raced against de_thread() from another thread? */
    if (!thread_group_leader(task)) {
    - percpu_up_write(&cgroup_threadgroup_rwsem);
    + threadgroup_unlock(task);
    put_task_struct(task);
    continue;
    }

    ret = cgroup_migrate(src_cset->dfl_cgrp, task, true);

    - percpu_up_write(&cgroup_threadgroup_rwsem);
    + threadgroup_unlock(task);
    put_task_struct(task);

    if (WARN(ret, "cgroup: failed to update controllers for the default hierarchy (%d), further operations may crash or hang\n", ret))
    @@ -5045,7 +5083,6 @@ int __init cgroup_init(void)
    unsigned long key;
    int ssid, err;

    - BUG_ON(percpu_init_rwsem(&cgroup_threadgroup_rwsem));
    BUG_ON(cgroup_init_cftypes(NULL, cgroup_dfl_base_files));
    BUG_ON(cgroup_init_cftypes(NULL, cgroup_legacy_base_files));

    diff --git a/kernel/fork.c b/kernel/fork.c
    index 7d5f0f1..2845623 100644
    --- a/kernel/fork.c
    +++ b/kernel/fork.c
    @@ -1149,6 +1149,10 @@ static int copy_signal(unsigned long clone_flags, struct task_struct *tsk)
    tty_audit_fork(sig);
    sched_autogroup_fork(sig);

    +#ifdef CONFIG_CGROUPS
    + init_rwsem(&sig->group_rwsem);
    +#endif
    +
    sig->oom_score_adj = current->signal->oom_score_adj;
    sig->oom_score_adj_min = current->signal->oom_score_adj_min;

    --
    2.4.3


    \
     
     \ /
      Last update: 2015-09-16 18:01    [W:5.085 / U:0.240 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site