On Tue, Aug 3, 2010 at 9:38 PM, Ben Blum <bblum@xxxxxxxxxxxxxx> wrote:
>
> rcu_read_lock();
> for_each_subsys(...) {
> can_attach(...);
> }
> rcu_read_unlock();
Sorry, I was misreading this, and didn't notice that it was already
inside an "if (threadgroup) {}" test.
>
> Which forces all can_attaches to not sleep. So by dropping
> rcu_read_lock(), we allow the possibility of the exec race I described
> in my last email, and therefore we have to check each time we re-acquire
> rcu_read to iterate thread_group.
Agreed.
>
> Yeah, it is not pretty. I call it "double-double-toil-and-trouble-check
> locking". But it is safe.
As a cleanup, I'd be inclined to have a wrapper in cgroup.c, something like
cgroup_can_attach_threadgroup(struct cgroup_subsys *ss, struct cgroup
*cg, struct task_struct *leader, int (*cb)(struct task_struct *t,
struct cgroup *cg))
which handles the RCU section, checking threadgroup_leader(), and
looping through each thread. The the subsystem just has to define a
callback which will be called for each thread.
Paul
_______________________________________________
Containers mailing list
Containers@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linux-foundation.org/mailman/listinfo/containers
