Skip to content

Commit

Permalink
sched: Fix PROVE_RCU vs cpu_cgroup
Browse files Browse the repository at this point in the history 
PROVE_RCU has a few issues with the cpu_cgroup because the scheduler
typically holds rq->lock around the css rcu derefs but the generic
cgroup code doesn't (and can't) know about that lock.

Provide means to add extra checks to the css dereference and use that
in the scheduler to annotate its users.

The addition of rq->lock to these checks is correct because the
cgroup_subsys::attach() method takes the rq->lock for each task it
moves, therefore by holding that lock, we ensure the task is pinned to
the current cgroup and the RCU derefence is valid.

That leaves one genuine race in __sched_setscheduler() where we used
task_group() without holding any of the required locks and thus raced
with the cgroup code. Solve this by moving the check under the
appropriate lock.

Signed-off-by: Peter Zijlstra <[email protected]>
Cc: "Paul E. McKenney" <[email protected]>
LKML-Reference: <new-submission>
Signed-off-by: Ingo Molnar <[email protected]>
  • Loading branch information
Peter Zijlstra authored and Ingo Molnar committed Jun 8, 2010
1 parent 3975d16 commit dc61b1d
Show file tree
Hide file tree
Showing 2 changed files with 73 additions and 62 deletions.
20 changes: 14 additions & 6 deletions include/linux/cgroup.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -525,13 +525,21 @@ static inline struct cgroup_subsys_state *cgroup_subsys_state(
return cgrp->subsys[subsys_id];
}

static inline struct cgroup_subsys_state *task_subsys_state(
struct task_struct *task, int subsys_id)
/*
* function to get the cgroup_subsys_state which allows for extra
* rcu_dereference_check() conditions, such as locks used during the
* cgroup_subsys::attach() methods.
*/
#define task_subsys_state_check(task, subsys_id, __c) \
rcu_dereference_check(task->cgroups->subsys[subsys_id], \
rcu_read_lock_held() || \
lockdep_is_held(&task->alloc_lock) || \
cgroup_lock_is_held() || (__c))

static inline struct cgroup_subsys_state *
task_subsys_state(struct task_struct *task, int subsys_id)
{
return rcu_dereference_check(task->cgroups->subsys[subsys_id],
rcu_read_lock_held() ||
lockdep_is_held(&task->alloc_lock) ||
cgroup_lock_is_held());
return task_subsys_state_check(task, subsys_id, false);
}

static inline struct cgroup* task_cgroup(struct task_struct *task,
Expand Down
115 changes: 59 additions & 56 deletions kernel/sched.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -306,52 +306,6 @@ static int init_task_group_load = INIT_TASK_GROUP_LOAD;
*/
struct task_group init_task_group;

/* return group to which a task belongs */
static inline struct task_group *task_group(struct task_struct *p)
{
struct task_group *tg;

#ifdef CONFIG_CGROUP_SCHED
tg = container_of(task_subsys_state(p, cpu_cgroup_subsys_id),
struct task_group, css);
#else
tg = &init_task_group;
#endif
return tg;
}

/* Change a task's cfs_rq and parent entity if it moves across CPUs/groups */
static inline void set_task_rq(struct task_struct *p, unsigned int cpu)
{
/*
* Strictly speaking this rcu_read_lock() is not needed since the
* task_group is tied to the cgroup, which in turn can never go away
* as long as there are tasks attached to it.
*
* However since task_group() uses task_subsys_state() which is an
* rcu_dereference() user, this quiets CONFIG_PROVE_RCU.
*/
rcu_read_lock();
#ifdef CONFIG_FAIR_GROUP_SCHED
p->se.cfs_rq = task_group(p)->cfs_rq[cpu];
p->se.parent = task_group(p)->se[cpu];
#endif

#ifdef CONFIG_RT_GROUP_SCHED
p->rt.rt_rq = task_group(p)->rt_rq[cpu];
p->rt.parent = task_group(p)->rt_se[cpu];
#endif
rcu_read_unlock();
}

#else

static inline void set_task_rq(struct task_struct *p, unsigned int cpu) { }
static inline struct task_group *task_group(struct task_struct *p)
{
return NULL;
}

#endif /* CONFIG_CGROUP_SCHED */

/* CFS-related fields in a runqueue */
Expand Down Expand Up @@ -644,6 +598,49 @@ static inline int cpu_of(struct rq *rq)
#define cpu_curr(cpu) (cpu_rq(cpu)->curr)
#define raw_rq() (&__raw_get_cpu_var(runqueues))

#ifdef CONFIG_CGROUP_SCHED

/*
* Return the group to which this tasks belongs.
*
* We use task_subsys_state_check() and extend the RCU verification
* with lockdep_is_held(&task_rq(p)->lock) because cpu_cgroup_attach()
* holds that lock for each task it moves into the cgroup. Therefore
* by holding that lock, we pin the task to the current cgroup.
*/
static inline struct task_group *task_group(struct task_struct *p)
{
struct cgroup_subsys_state *css;

css = task_subsys_state_check(p, cpu_cgroup_subsys_id,
lockdep_is_held(&task_rq(p)->lock));
return container_of(css, struct task_group, css);
}

/* Change a task's cfs_rq and parent entity if it moves across CPUs/groups */
static inline void set_task_rq(struct task_struct *p, unsigned int cpu)
{
#ifdef CONFIG_FAIR_GROUP_SCHED
p->se.cfs_rq = task_group(p)->cfs_rq[cpu];
p->se.parent = task_group(p)->se[cpu];
#endif

#ifdef CONFIG_RT_GROUP_SCHED
p->rt.rt_rq = task_group(p)->rt_rq[cpu];
p->rt.parent = task_group(p)->rt_se[cpu];
#endif
}

#else /* CONFIG_CGROUP_SCHED */

static inline void set_task_rq(struct task_struct *p, unsigned int cpu) { }
static inline struct task_group *task_group(struct task_struct *p)
{
return NULL;
}

#endif /* CONFIG_CGROUP_SCHED */

inline void update_rq_clock(struct rq *rq)
{
if (!rq->skip_clock_update)
Expand Down Expand Up @@ -4465,16 +4462,6 @@ static int __sched_setscheduler(struct task_struct *p, int policy,
}

if (user) {
#ifdef CONFIG_RT_GROUP_SCHED
/*
* Do not allow realtime tasks into groups that have no runtime
* assigned.
*/
if (rt_bandwidth_enabled() && rt_policy(policy) &&
task_group(p)->rt_bandwidth.rt_runtime == 0)
return -EPERM;
#endif

retval = security_task_setscheduler(p, policy, param);
if (retval)
return retval;
Expand All @@ -4490,6 +4477,22 @@ static int __sched_setscheduler(struct task_struct *p, int policy,
* runqueue lock must be held.
*/
rq = __task_rq_lock(p);

#ifdef CONFIG_RT_GROUP_SCHED
if (user) {
/*
* Do not allow realtime tasks into groups that have no runtime
* assigned.
*/
if (rt_bandwidth_enabled() && rt_policy(policy) &&
task_group(p)->rt_bandwidth.rt_runtime == 0) {
__task_rq_unlock(rq);
raw_spin_unlock_irqrestore(&p->pi_lock, flags);
return -EPERM;
}
}
#endif

/* recheck policy now with rq lock held */
if (unlikely(oldpolicy != -1 && oldpolicy != p->policy)) {
policy = oldpolicy = -1;
Expand Down

0 comments on commit dc61b1d

Please sign in to comment.