Hey,
On Fri, Oct 04, 2013 at 03:02:07PM +0200, Oleg Nesterov wrote:
> Not sure I understand, but just in case: yes the lockless
> while_each_thread() is buggy and should be fixed (it should actually
> die eventually). And a lot of current users of while_each_thread()
> are themselves buggy and need the fixes no matter what we do with
> while_each_thread.
>
> Oh. and just in case... I am (slowly) working on this, but didn't
> finish it yet, sorry.
Is there anything I can do to make the hang go away in the meantime?
> But I do not really understand cgroup_attach_task(), and I am not
> sure you observed the same problem. Add Tejun.
Sounds like the same problem to me.
> --- x/kernel/cgroup.c
> +++ x/kernel/cgroup.c
> @@ -2034,7 +2034,7 @@ static int cgroup_attach_task(struct cgr
> * take an rcu_read_lock.
> */
> rcu_read_lock();
> - do {
> + for_each_thread(leader->signal, task) {
> struct task_and_cgroup ent;
>
> /* @tsk either already exited or can't exit until the end */
> @@ -2058,7 +2058,7 @@ static int cgroup_attach_task(struct cgr
>
> if (!threadgroup)
> break;
> - } while_each_thread(leader, tsk);
> + }
> rcu_read_unlock();
> /* remember the number of threads in the array for later. */
> group_size = i;
>
> after we add for_each_thread/etc. But I am not sure we need another
> "threadgroup" check.
...
> > diff --git a/kernel/cgroup.c b/kernel/cgroup.c
> > index 1f53387..cae8416 100644
> > --- a/kernel/cgroup.c
> > +++ b/kernel/cgroup.c
> > @@ -2002,7 +2002,9 @@ static int cgroup_attach_task(struct cgroup *cgrp, struct task_struct *tsk,
> > ent.task = tsk;
> > ent.cgrp = task_cgroup_from_root(tsk, root);
> > /* nothing to do if this task is already in the cgroup */
> > - if (ent.cgrp == cgrp)
> > + if (ent.cgrp == cgrp && !threadgroup)
> > + break;
> > + else if(ent.cgrp == cgrp)
> > continue;
I have no idea know how this would make any difference. Hmm...
Thanks.
--
tejun
--
To unsubscribe from this list: send the line "unsubscribe cgroups" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
