Re: [PATCH v4 2/2] cgroups: make procs file writable

[Paul Menage <menage@google.com>]

On Tue, Aug 3, 2010 at 9:38 PM, Ben Blum <bblum@andrew.cmu.edu> 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