Benjamin Blum wrote:
> On Mon, Aug 3, 2009 at 6:09 PM, Li Zefan<lizf@xxxxxxxxxxxxxx> wrote:
>> Benjamin Blum wrote:
>>> On Sun, Aug 2, 2009 at 8:00 PM, Li Zefan<lizf@xxxxxxxxxxxxxx> wrote:
>>>> Ben Blum wrote:
>>>>> + }
>>>>> + write_unlock(&css_set_lock);
>>>>> +
>>>>> + /*
>>>>> + * We just gained a reference on oldcg by taking it from the task. As
>>>> This comment is incorrect, the ref we just got has been dropped by
>>>> the above put_css_set(oldcg).
>>> No, the idea is that even though we had a reference that we already
>>> dropped, we in effect "traded" the newcg to the task for its oldcg,
>>> giving it our reference on newcg and gaining its reference on oldcg. I
>>> believe the cgroup_mutex guarantees that it'll still be there when we
>>> do the trade - perhaps a BUG_ON(tsk->cgroups != oldcg) is wanted
>>> inside the second task_lock section there? At the very least, a
>>> clearer comment.
>>>
>> Maybe my English sucks..
>>
>> By "gained a reference", doesn't it mean get_css_set()? But this
>> put_css_set() is not against the get() just called.
>
> not in the conventional way, no. the comment there is bad enough that
> this is unclear: before trading pointers, the task had a reference on
> its tsk->cgroups pointer (same as our oldcg pointer), which is what we
> are overwriting with newcg. the task will think that the reference it
> has is still on tsk->cgroups, but since the pointer has changed, its
> reference also changes to a reference on newcg - one that this
> function took care of getting for the task. additionally, now that the
> task's reference is no longer for oldcg, we have to take care of the
> refcount that still thinks it's being used.
>
Ok.
>> And in fact the ref can be 0 before this put(), because task_exit
>> can drop the last ref, but put_css_set() will check this case,
>> so it's Ok.
>
> the check for PF_EXITING precludes that case.
>
No. Note task exiting is not protected by cgroup_lock, so this can
happen:
| cgroup_attach_task()
| oldcg = tsk->cgroups;
| (tasks->flags & TASK_EXISING == 0)
| rcu_assign_pointer(tsk->cgroups, newcg);
cgroup_exit() |
oldcg = tsk->cgroups; |
put_css_set_taskexit(oldcg); |
(now ref of olcg is 0) |
| put_css_set(oldcg);
_______________________________________________
Containers mailing list
Containers@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linux-foundation.org/mailman/listinfo/containers
