2012/10/19 Tejun Heo <tj@xxxxxxxxxx>:
> Hello, Frederic.
>
> On Fri, Oct 19, 2012 at 03:44:20PM -0400, Frederic Weisbecker wrote:
>> > For -stable, I think it's better to revert. If you want to remove
>> > task_lock, let's do it for 3.8.
>>
>> I don't think that a wrong comment justifies a patch to stable.
>
> I'm not really sure whether it's safe or not. It seems all usages are
> protected by write locking css_set_lock but maybe I'm missing
> something and as the commit is born out of confusion, I'm very
> inclined to revert it by default. Are you sure this one is safe?
Thinking about it further, one scenario is worrying me but it
eventually looks safe but by accident.
CPU 0
CPU 1
cgroup_task_migrate {
task_lock(p)
rcu_assign_pointer(tsk->cgroups, newcg);
task_unlock(tsk);
write_lock(&css_set_lock);
if (!list_empty(&tsk->cg_list))
list_move(&tsk->cg_list, &newcg->tasks);
write_unlock(&css_set_lock);
write_lock(&css_set_lock);
put_css_set(oldcg);
list_add(&child->cg_list, &child->cgroups->tasks); (1)
On (1), child->cgroups should have the value of newcg and not oldcg
due to the memory ordering implied by the locking of css_set_lock. Now
I can't guarantee that because I'm no memory ordering expert. And even
if it's safe, it's so very non obvious that I now agree with you:
let's revert the patch and restart with a better base by gathering
all the cgroup fork code in the current cgroup_post_fork place.
Thanks.
_______________________________________________
Containers mailing list
Containers@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/containers
