| Message ID | 4F74095B.70105@jp.fujitsu.com |
|---|---|
| State | Rejected, archived |
| Delegated to: | David Miller |
| Headers | show |
On 03/29/2012 09:03 AM, KAMEZAWA Hiroyuki wrote: > > Now, tcp memory control cgroup ignores memcg's use_hierarchy value > and act as use_hierarchy=1 always. After this patch, tcp memcontrol will > work as memcg is designed. > > Note: > I know there is a discussion to remove use_hierarchy but this is BUG, now. > Kame, Are you sure about that? I just tried it myself, and it seems to work: root@inf5072-11:~/glommer-temporary/a/b# cat memory.kmem.tcp.usage_in_bytes 724992 root@inf5072-11:~/glommer-temporary/a/b# cat ../memory.kmem.tcp.usage_in_bytes 0 Did you got this conclusion through testing or code inspection? As a matter of fact, that's why I believe the current behavior is indeed correct: the res_counter is initialized as: parent_cg = tcp_prot.proto_cgroup(parent); if (parent_cg) res_parent = parent_cg->memory_allocated; res_counter_init(&tcp->tcp_memory_allocated, res_parent); now, parent is drawn from parent_mem_cgroup(), that reads as follows: struct mem_cgroup *parent_mem_cgroup(struct mem_cgroup *memcg) { if (!memcg->res.parent) return NULL; return mem_cgroup_from_res_counter(memcg->res.parent, res); } so if we have use_hierarchy = 0, res.parent should be NULL (because that is the way we initialize it) -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
(2012/03/29 18:14), Glauber Costa wrote: > On 03/29/2012 09:03 AM, KAMEZAWA Hiroyuki wrote: >> >> Now, tcp memory control cgroup ignores memcg's use_hierarchy value >> and act as use_hierarchy=1 always. After this patch, tcp memcontrol will >> work as memcg is designed. >> >> Note: >> I know there is a discussion to remove use_hierarchy but this is BUG, now. >> > > Kame, > > Are you sure about that? > > I just tried it myself, and it seems to work: > > root@inf5072-11:~/glommer-temporary/a/b# cat memory.kmem.tcp.usage_in_bytes > 724992 > root@inf5072-11:~/glommer-temporary/a/b# cat > ../memory.kmem.tcp.usage_in_bytes > 0 > > > Did you got this conclusion through testing or code inspection? > > As a matter of fact, that's why I believe the current behavior is indeed > correct: > > the res_counter is initialized as: > > parent_cg = tcp_prot.proto_cgroup(parent); > if (parent_cg) > res_parent = parent_cg->memory_allocated; > > res_counter_init(&tcp->tcp_memory_allocated, res_parent); > > now, parent is drawn from parent_mem_cgroup(), that reads as follows: > > struct mem_cgroup *parent_mem_cgroup(struct mem_cgroup *memcg) > { > if (!memcg->res.parent) > return NULL; > return mem_cgroup_from_res_counter(memcg->res.parent, res); > } > > > so if we have use_hierarchy = 0, res.parent should be NULL (because that > is the way we initialize it) > Ah, sorry. you're right. please forget this patch. To be honest, I wrote this after seeing WARN_ON in patch 2 and 3 and misunderstood the code at writing patch 2 and 3. Thanks, -Kame -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h index f94efd2..e116b7c 100644 --- a/include/linux/memcontrol.h +++ b/include/linux/memcontrol.h @@ -199,6 +199,9 @@ void mem_cgroup_split_huge_fixup(struct page *head); bool mem_cgroup_bad_page_check(struct page *page); void mem_cgroup_print_bad_page(struct page *page); #endif + +bool mem_cgroup_use_hierarchy(struct mem_cgroup *memcg); + #else /* CONFIG_CGROUP_MEM_RES_CTLR */ struct mem_cgroup; diff --git a/mm/memcontrol.c b/mm/memcontrol.c index 7d698df..467881f 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -867,6 +867,11 @@ struct mem_cgroup *try_get_mem_cgroup_from_mm(struct mm_struct *mm) return memcg; } +bool mem_cgroup_use_hierarchy(struct mem_cgroup *memcg) +{ + return memcg->use_hierarchy; +} + /** * mem_cgroup_iter - iterate over memory cgroup hierarchy * @root: hierarchy root diff --git a/net/ipv4/tcp_memcontrol.c b/net/ipv4/tcp_memcontrol.c index e795272..32764a6 100644 --- a/net/ipv4/tcp_memcontrol.c +++ b/net/ipv4/tcp_memcontrol.c @@ -75,7 +75,7 @@ int tcp_init_cgroup(struct cgroup *cgrp, struct cgroup_subsys *ss) tcp->tcp_memory_pressure = 0; parent_cg = tcp_prot.proto_cgroup(parent); - if (parent_cg) + if (parent_cg && mem_cgroup_use_hierarchy(parent)) res_parent = parent_cg->memory_allocated; res_counter_init(&tcp->tcp_memory_allocated, res_parent);
Now, tcp memory control cgroup ignores memcg's use_hierarchy value and act as use_hierarchy=1 always. After this patch, tcp memcontrol will work as memcg is designed. Note: I know there is a discussion to remove use_hierarchy but this is BUG, now. Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> --- include/linux/memcontrol.h | 3 +++ mm/memcontrol.c | 5 +++++ net/ipv4/tcp_memcontrol.c | 2 +- 3 files changed, 9 insertions(+), 1 deletions(-)