Re: [PATCH 1/2] memcg: dirty pages accounting and limiting infrastructure

[Minchan Kim]

On Fri, Feb 26, 2010 at 2:01 PM, KAMEZAWA Hiroyuki
<kamezawa.hiroyu@xxxxxxxxxxxxxx> wrote:
> Hi,
>
> On Fri, 26 Feb 2010 13:50:04 +0900
> Minchan Kim <minchan.kim@xxxxxxxxx> wrote:
>
>> > Hm ? I don't read the whole thread but can_attach() is called under
>> > cgroup_mutex(). So, it doesn't need to use RCU.
>>
>> Vivek mentioned memcg is protected by RCU if I understand his intention right.
>> So I commented that without enough knowledge of memcg.
>> After your comment, I dive into the code.
>>
>> Just out of curiosity.
>>
>> Really, memcg is protected by RCU?
> yes. All cgroup subsystem is protected by RCU.
>
>> I think most of RCU around memcg is for protecting task_struct and
>> cgroup_subsys_state.
>> The memcg is protected by cgroup_mutex as you mentioned.
>> Am I missing something?
>
> There are several levels of protections.
>
> cgroup subsystem's ->destroy() call back is finally called by
>
> As this.
>
> Â768 Â Â Â Â Â Â Â Â synchronize_rcu();
> Â769
> Â770 Â Â Â Â Â Â Â Â mutex_lock(&cgroup_mutex);
> Â771 Â Â Â Â Â Â Â Â /*
> Â772 Â Â Â Â Â Â Â Â Â* Release the subsystem state objects.
> Â773 Â Â Â Â Â Â Â Â Â*/
> Â774 Â Â Â Â Â Â Â Â for_each_subsys(cgrp->root, ss)
> Â775 Â Â Â Â Â Â Â Â Â Â Â Â ss->destroy(ss, cgrp);
> Â776
> Â777 Â Â Â Â Â Â Â Â cgrp->root->number_of_cgroups--;
> Â778 Â Â Â Â Â Â Â Â mutex_unlock(&cgroup_mutex);
>
> Before here,
> Â Â Â Â- there are no tasks under this cgroup (cgroup's refcnt is 0)
> Â Â Â Â Â&& cgroup is marked as REMOVED.
>
> Then, this access
> Â Â Â Ârcu_read_lock();
> Â Â Â Âmem = mem_cgroup_from_task(task);
> Â Â Â Âif (css_tryget(mem->css)) Â <===============checks cgroup refcnt

If it it, do we always need css_tryget after mem_cgroup_from_task
without cgroup_mutex to make sure css is vaild?

But I found several cases that don't call css_tryget

1. mm_match_cgroup
It's used by page_referenced_xxx. so we I think we don't grab
cgroup_mutex at that time.

2. mem_cgroup_oom_called
I think in here we don't grab cgroup_mutex, too.

I guess some design would cover that problems.
Could you tell me if you don't mind?
Sorry for bothering you.

Thanks, Kame.



-- 
Kind regards,
Minchan Kim
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/