OpenVZ Forum - RDF feed
type: channel
items: [PATCH v9 0/9] Request for inclusion: per-cgroup tcp memory pressure controls
items: [PATCH v9 1/9] Basic kernel memory functionality for the Memory Controller
items: [PATCH v9 2/9] foundations of per-cgroup memory pressure controlling.
items: [PATCH v9 3/9] socket: initial cgroup code.
items: [PATCH v9 4/9] tcp memory pressure controls
items: [PATCH v9 5/9] per-netns ipv4 sysctl_tcp_mem
items: [PATCH v9 6/9] tcp buffer limitation: per-cgroup limit
items: [PATCH v9 7/9] Display current tcp memory allocation in kmem cgroup
items: [PATCH v9 8/9] Display current tcp failcnt in kmem cgroup
items: [PATCH v9 9/9] Display maximum tcp memory allocation in kmem cgroup
items: Re: [PATCH v9 0/9] Request for inclusion: per-cgroup tcp memory pressure controls
items: Re: [PATCH v9 1/9] Basic kernel memory functionality for the Memory Controller
items: Re: [PATCH v9 3/9] socket: initial cgroup code.

[PATCH v9 0/9] Request for inclusion: per-cgroup tcp memory pressure controls
type: item
description: Hi, This series fixes all the few comments raised in the last round, and seem to have acquired consensus from the memcg side. Dave, do you think it is acceptable now from the networking PoV? In case positive, would you prefer merging this trough your tree, or acking this so a cgroup maintainer can do it? Thanks Glauber Costa (9): Basic kernel memory functionality for the Memory Controller foundations of per-cgroup memory pressure controlling. socket: initial cgroup code. tcp memory pressure controls per-netns ipv4 sysctl_tcp_mem tcp buffer limitation: per-cgroup limit Display current tcp memory allocation in kmem cgroup Display current tcp failcnt in kmem cgroup Display maximum tcp memory allocation in kmem cgroup Documentation/cgroups/memory.txt | 46 ++++++- include/linux/memcontrol.h | 23 ++++ include/net/netns/ipv4.h | 1 + include/net/sock.h | 244 +++++++++++++++++++++++++++++++++- include/net/tcp.h | 4 +- include/net/tcp_memcontrol.h | 19 +++ init/Kconfig | 11 ++ mm/memcontrol.c | 191 +++++++++++++++++++++++++- net/core/sock.c | 112 ++++++++++++---- net/ipv4/Makefile | 1 + net/ipv4/af_inet.c | 2 + net/ipv4/proc.c | 6 +- net/ipv4/sysctl_net_ipv4.c | 65 ++++++++- net/ipv4/tcp.c | 11 +-- net/ipv4/tcp_input.c | 12 +- net/ipv4/tcp_ipv4.c | 14 ++- net/ipv4/tcp_memcontrol.c | 272 ++++++++++++++++++++++++++++++++++++++ net/ipv4/tcp_output.c | 2 +- net/ipv4/tcp_timer.c | 2 +- net/ipv6/af_inet6.c | 2 + net/ipv6/tcp_ipv6.c | 8 +- 21 files changed, 973 insertions(+), 75 deletions(-) create mode 100644 include/net/tcp_memcontrol.h create mode 100644 net/ipv4/tcp_memcontrol.c -- 1.7.6.4
creator: Glauber Costa
date: 2011-12-12T07:47:00-00:00

[PATCH v9 1/9] Basic kernel memory functionality for the Memory Controller
type: item
description: This patch lays down the foundation for the kernel memory component of the Memory Controller. As of today, I am only laying down the following files: * memory.independent_kmem_limit * memory.kmem.limit_in_bytes (currently ignored) * memory.kmem.usage_in_bytes (always zero) Signed-off-by: Glauber Costa <glommer@parallels.com> CC: Kirill A. Shutemov <kirill@shutemov.name> CC: Paul Menage <paul@paulmenage.org> CC: Greg Thelen <gthelen@google.com> CC: Johannes Weiner <jweiner@redhat.com> CC: Michal Hocko <mhocko@suse.cz> --- Documentation/cgroups/memory.txt | 40 ++++++++++++++- init/Kconfig | 11 ++++ mm/memcontrol.c | 105 ++++++++++++++++++++++++++++++++++++-- 3 files changed, 149 insertions(+), 7 deletions(-) diff --git a/Documentation/cgroups/memory.txt b/Documentation/cgroups/memory.txt index cc0ebc5..f245324 100644 --- a/Documentation/cgroups/memory.txt +++ b/Documentation/cgroups/memory.txt @@ -44,8 +44,9 @@ Features: - oom-killer disable knob and oom-notifier - Root cgroup has no limit controls. - Kernel memory and Hugepages are not under control yet. We just manage - pages on LRU. To add more controls, we have to take care of performance. + Hugepages is not under control yet. We just manage pages on LRU. To add more + controls, we have to take care of performance. Kernel memory support is work + in progress, and the current version provides basically functionality. Brief summary of control files. @@ -56,8 +57,11 @@ Brief summary of control files. (See 5.5 for details) memory.memsw.usage_in_bytes # show current res_counter usage for memory+Swap (See 5.5 for details) + memory.kmem.usage_in_bytes # show current res_counter usage for kmem only. + (See 2.7 for details) memory.limit_in_bytes # set/show limit of memory usage memory.memsw.limit_in_bytes # set/show limit of memory+Swap usage + memory.kmem.limit_in_bytes # if allowed, set/show limit of kernel memory memory.failcnt # show the number of memory usage hits limits memory.memsw.failcnt # show the number of memor…
creator: Glauber Costa
date: 2011-12-12T07:47:01-00:00

[PATCH v9 2/9] foundations of per-cgroup memory pressure controlling.
type: item
description: This patch replaces all uses of struct sock fields' memory_pressure, memory_allocated, sockets_allocated, and sysctl_mem to acessor macros. Those macros can either receive a socket argument, or a mem_cgroup argument, depending on the context they live in. Since we're only doing a macro wrapping here, no performance impact at all is expected in the case where we don't have cgroups disabled. Signed-off-by: Glauber Costa <glommer@parallels.com> Reviewed-by: Hiroyouki Kamezawa <kamezawa.hiroyu@jp.fujitsu.com> CC: David S. Miller <davem@davemloft.net> CC: Eric W. Biederman <ebiederm@xmission.com> CC: Eric Dumazet <eric.dumazet@gmail.com> --- include/net/sock.h | 96 ++++++++++++++++++++++++++++++++++++++++++++++++- include/net/tcp.h | 3 +- net/core/sock.c | 57 +++++++++++++++++------------ net/ipv4/proc.c | 6 ++-- net/ipv4/tcp_input.c | 12 +++--- net/ipv4/tcp_ipv4.c | 4 +- net/ipv4/tcp_output.c | 2 +- net/ipv4/tcp_timer.c | 2 +- net/ipv6/tcp_ipv6.c | 2 +- 9 files changed, 145 insertions(+), 39 deletions(-) diff --git a/include/net/sock.h b/include/net/sock.h index abb6e0f..5f43fd9 100644 --- a/include/net/sock.h +++ b/include/net/sock.h @@ -53,6 +53,7 @@ #include <linux/security.h> #include <linux/slab.h> #include <linux/uaccess.h> +#include <linux/memcontrol.h> #include <linux/filter.h> #include <linux/rculist_nulls.h> @@ -863,6 +864,99 @@ static inline void sk_refcnt_debug_release(const struct sock *sk) #define sk_refcnt_debug_release(sk) do { } while (0) #endif /* SOCK_REFCNT_DEBUG */ +static inline bool sk_has_memory_pressure(const struct sock *sk) +{ + return sk->sk_prot->memory_pressure != NULL; +} + +static inline bool sk_under_memory_pressure(const struct sock *sk) +{ + if (!sk->sk_prot->memory_pressure) + return false; + return !!*sk->sk_prot->memory_pressure; +} + +static inline void sk_leave_memory_pressure(struct sock *sk) +{ + int *memory_pressure = sk->sk_prot->memory_pressure; + + if (memory_pressure && *memory_pressure) + *memory_pressure = 0; +} + +static…
creator: Glauber Costa
date: 2011-12-12T07:47:02-00:00

[PATCH v9 3/9] socket: initial cgroup code.
type: item
description: The goal of this work is to move the memory pressure tcp controls to a cgroup, instead of just relying on global conditions. To avoid excessive overhead in the network fast paths, the code that accounts allocated memory to a cgroup is hidden inside a static_branch(). This branch is patched out until the first non-root cgroup is created. So when nobody is using cgroups, even if it is mounted, no significant performance penalty should be seen. This patch handles the generic part of the code, and has nothing tcp-specific. Signed-off-by: Glauber Costa <glommer@parallels.com> Reviewed-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujtsu.com> CC: Kirill A. Shutemov <kirill@shutemov.name> CC: David S. Miller <davem@davemloft.net> CC: Eric W. Biederman <ebiederm@xmission.com> CC: Eric Dumazet <eric.dumazet@gmail.com> --- Documentation/cgroups/memory.txt | 4 +- include/linux/memcontrol.h | 22 ++++++ include/net/sock.h | 156 ++++++++++++++++++++++++++++++++++++-- mm/memcontrol.c | 46 +++++++++++- net/core/sock.c | 24 ++++-- 5 files changed, 235 insertions(+), 17 deletions(-) diff --git a/Documentation/cgroups/memory.txt b/Documentation/cgroups/memory.txt index f245324..23a8dc5 100644 --- a/Documentation/cgroups/memory.txt +++ b/Documentation/cgroups/memory.txt @@ -289,7 +289,9 @@ to trigger slab reclaim when those limits are reached. 2.7.1 Current Kernel Memory resources accounted -None +* sockets memory pressure: some sockets protocols have memory pressure +thresholds. The Memory Controller allows them to be controlled individually +per cgroup, instead of globally. 3. User Interface diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h index b87068a..f15021b 100644 --- a/include/linux/memcontrol.h +++ b/include/linux/memcontrol.h @@ -85,6 +85,8 @@ extern struct mem_cgroup *try_get_mem_cgroup_from_page(struct page *page); extern struct mem_cgroup *mem_cgroup_from_task(struct task_struct *p); extern struct mem_cgroup *try_get_mem_cgroup_from_mm(struct mm_struct *mm)…
creator: Glauber Costa
date: 2011-12-12T07:47:03-00:00

[PATCH v9 4/9] tcp memory pressure controls
type: item
description: This patch introduces memory pressure controls for the tcp protocol. It uses the generic socket memory pressure code introduced in earlier patches, and fills in the necessary data in cg_proto struct. Signed-off-by: Glauber Costa <glommer@parallels.com> Reviewed-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujtisu.com> CC: Eric W. Biederman <ebiederm@xmission.com> --- Documentation/cgroups/memory.txt | 2 + include/linux/memcontrol.h | 1 + include/net/sock.h | 2 + include/net/tcp_memcontrol.h | 17 +++++++++ mm/memcontrol.c | 40 ++++++++++++++++++++- net/core/sock.c | 43 ++++++++++++++++++++-- net/ipv4/Makefile | 1 + net/ipv4/tcp_ipv4.c | 9 ++++- net/ipv4/tcp_memcontrol.c | 74 ++++++++++++++++++++++++++++++++++++++ net/ipv6/tcp_ipv6.c | 5 +++ 10 files changed, 189 insertions(+), 5 deletions(-) create mode 100644 include/net/tcp_memcontrol.h create mode 100644 net/ipv4/tcp_memcontrol.c diff --git a/Documentation/cgroups/memory.txt b/Documentation/cgroups/memory.txt index 23a8dc5..687dea5 100644 --- a/Documentation/cgroups/memory.txt +++ b/Documentation/cgroups/memory.txt @@ -293,6 +293,8 @@ to trigger slab reclaim when those limits are reached. thresholds. The Memory Controller allows them to be controlled individually per cgroup, instead of globally. +* tcp memory pressure: sockets memory pressure for the tcp protocol. + 3. User Interface 0. Configuration diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h index f15021b..1513994 100644 --- a/include/linux/memcontrol.h +++ b/include/linux/memcontrol.h @@ -86,6 +86,7 @@ extern struct mem_cgroup *mem_cgroup_from_task(struct task_struct *p); extern struct mem_cgroup *try_get_mem_cgroup_from_mm(struct mm_struct *mm); extern struct mem_cgroup *parent_mem_cgroup(struct mem_cgroup *memcg); +extern struct mem_cgroup *mem_cgroup_from_cont(struct cgroup *cont); static inline int mm_match_cgroup(const struct mm_struct *mm, const struct mem_cgroup *cgroup) diff --git a/include/net/sock.h b/include/net/sock.h index 6cb…
creator: Glauber Costa
date: 2011-12-12T07:47:04-00:00

[PATCH v9 5/9] per-netns ipv4 sysctl_tcp_mem
type: item
description: This patch allows each namespace to independently set up its levels for tcp memory pressure thresholds. This patch alone does not buy much: we need to make this values per group of process somehow. This is achieved in the patches that follows in this patchset. Signed-off-by: Glauber Costa <glommer@parallels.com> Reviewed-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> CC: David S. Miller <davem@davemloft.net> CC: Eric W. Biederman <ebiederm@xmission.com> --- include/net/netns/ipv4.h | 1 + include/net/tcp.h | 1 - net/ipv4/af_inet.c | 2 + net/ipv4/sysctl_net_ipv4.c | 51 +++++++++++++++++++++++++++++++++++++------ net/ipv4/tcp.c | 11 +------- net/ipv4/tcp_ipv4.c | 1 - net/ipv4/tcp_memcontrol.c | 9 +++++-- net/ipv6/af_inet6.c | 2 + net/ipv6/tcp_ipv6.c | 1 - 9 files changed, 57 insertions(+), 22 deletions(-) diff --git a/include/net/netns/ipv4.h b/include/net/netns/ipv4.h index d786b4f..bbd023a 100644 --- a/include/net/netns/ipv4.h +++ b/include/net/netns/ipv4.h @@ -55,6 +55,7 @@ struct netns_ipv4 { int current_rt_cache_rebuild_count; unsigned int sysctl_ping_group_range[2]; + long sysctl_tcp_mem[3]; atomic_t rt_genid; atomic_t dev_addr_genid; diff --git a/include/net/tcp.h b/include/net/tcp.h index f080e0b..61c7e76 100644 --- a/include/net/tcp.h +++ b/include/net/tcp.h @@ -230,7 +230,6 @@ extern int sysctl_tcp_fack; extern int sysctl_tcp_reordering; extern int sysctl_tcp_ecn; extern int sysctl_tcp_dsack; -extern long sysctl_tcp_mem[3]; extern int sysctl_tcp_wmem[3]; extern int sysctl_tcp_rmem[3]; extern int sysctl_tcp_app_win; diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c index 1b5096a..a8bbcff 100644 --- a/net/ipv4/af_inet.c +++ b/net/ipv4/af_inet.c @@ -1671,6 +1671,8 @@ static int __init inet_init(void) ip_static_sysctl_init(); #endif + tcp_prot.sysctl_mem = init_net.ipv4.sysctl_tcp_mem; + /* * Add all the base protocols. */ diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c index 69fd720..bbd67ab 100644 --- a/net/ipv4/sysctl_net_ipv4.c…
creator: Glauber Costa
date: 2011-12-12T07:47:05-00:00

[PATCH v9 6/9] tcp buffer limitation: per-cgroup limit
type: item
description: This patch uses the "tcp.limit_in_bytes" field of the kmem_cgroup to effectively control the amount of kernel memory pinned by a cgroup. This value is ignored in the root cgroup, and in all others, caps the value specified by the admin in the net namespaces' view of tcp_sysctl_mem. If namespaces are being used, the admin is allowed to set a value bigger than cgroup's maximum, the same way it is allowed to set pretty much unlimited values in a real box. Signed-off-by: Glauber Costa <glommer@parallels.com> Reviewed-by: Hiroyouki Kamezawa <kamezawa.hiroyu@jp.fujitsu.com> CC: David S. Miller <davem@davemloft.net> CC: Eric W. Biederman <ebiederm@xmission.com> --- Documentation/cgroups/memory.txt | 1 + include/net/tcp_memcontrol.h | 2 + net/ipv4/sysctl_net_ipv4.c | 14 ++++ net/ipv4/tcp_memcontrol.c | 137 +++++++++++++++++++++++++++++++++++++- 4 files changed, 152 insertions(+), 2 deletions(-) diff --git a/Documentation/cgroups/memory.txt b/Documentation/cgroups/memory.txt index 687dea5..1c9779a 100644 --- a/Documentation/cgroups/memory.txt +++ b/Documentation/cgroups/memory.txt @@ -78,6 +78,7 @@ Brief summary of control files. memory.independent_kmem_limit # select whether or not kernel memory limits are independent of user limits + memory.kmem.tcp.limit_in_bytes # set/show hard limit for tcp buf memory 1. History diff --git a/include/net/tcp_memcontrol.h b/include/net/tcp_memcontrol.h index 5f5e158..3512082 100644 --- a/include/net/tcp_memcontrol.h +++ b/include/net/tcp_memcontrol.h @@ -14,4 +14,6 @@ struct tcp_memcontrol { struct cg_proto *tcp_proto_cgroup(struct mem_cgroup *memcg); int tcp_init_cgroup(struct cgroup *cgrp, struct cgroup_subsys *ss); void tcp_destroy_cgroup(struct cgroup *cgrp, struct cgroup_subsys *ss); +unsigned long long tcp_max_memory(const struct mem_cgroup *memcg); +void tcp_prot_mem(struct mem_cgroup *memcg, long val, int idx); #endif /* _TCP_MEMCG_H */ diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c index bbd67ab..fe9bf91 100…
creator: Glauber Costa
date: 2011-12-12T07:47:06-00:00

[PATCH v9 7/9] Display current tcp memory allocation in kmem cgroup
type: item
description: This patch introduces kmem.tcp.usage_in_bytes file, living in the kmem_cgroup filesystem. It is a simple read-only file that displays the amount of kernel memory currently consumed by the cgroup. Signed-off-by: Glauber Costa <glommer@parallels.com> Reviewed-by: Hiroyouki Kamezawa <kamezawa.hiroyu@jp.fujitsu.com> CC: David S. Miller <davem@davemloft.net> CC: Eric W. Biederman <ebiederm@xmission.com> --- Documentation/cgroups/memory.txt | 1 + net/ipv4/tcp_memcontrol.c | 21 +++++++++++++++++++++ 2 files changed, 22 insertions(+), 0 deletions(-) diff --git a/Documentation/cgroups/memory.txt b/Documentation/cgroups/memory.txt index 1c9779a..6922b6c 100644 --- a/Documentation/cgroups/memory.txt +++ b/Documentation/cgroups/memory.txt @@ -79,6 +79,7 @@ Brief summary of control files. memory.independent_kmem_limit # select whether or not kernel memory limits are independent of user limits memory.kmem.tcp.limit_in_bytes # set/show hard limit for tcp buf memory + memory.kmem.tcp.usage_in_bytes # show current tcp buf memory allocation 1. History diff --git a/net/ipv4/tcp_memcontrol.c b/net/ipv4/tcp_memcontrol.c index e353390..9481f23 100644 --- a/net/ipv4/tcp_memcontrol.c +++ b/net/ipv4/tcp_memcontrol.c @@ -17,6 +17,11 @@ static struct cftype tcp_files[] = { .read_u64 = tcp_cgroup_read, .private = RES_LIMIT, }, + { + .name = "kmem.tcp.usage_in_bytes", + .read_u64 = tcp_cgroup_read, + .private = RES_USAGE, + }, }; static inline struct tcp_memcontrol *tcp_from_cgproto(struct cg_proto *cg_proto) @@ -167,6 +172,19 @@ static u64 tcp_read_stat(struct mem_cgroup *memcg, int type, u64 default_val) return res_counter_read_u64(&tcp->tcp_memory_allocated, type); } +static u64 tcp_read_usage(struct mem_cgroup *memcg) +{ + struct tcp_memcontrol *tcp; + struct cg_proto *cg_proto; + + cg_proto = tcp_prot.proto_cgroup(memcg); + if (!cg_proto) + return atomic_long_read(&tcp_memory_allocated) << PAGE_SHIFT; + + tcp = tcp_from_cgproto(cg_proto); + return res_counter_read_u64(&tcp->tcp_memory_allo…
creator: Glauber Costa
date: 2011-12-12T07:47:07-00:00

[PATCH v9 8/9] Display current tcp failcnt in kmem cgroup
type: item
description: This patch introduces kmem.tcp.failcnt file, living in the kmem_cgroup filesystem. Following the pattern in the other memcg resources, this files keeps a counter of how many times allocation failed due to limits being hit in this cgroup. The root cgroup will always show a failcnt of 0. Signed-off-by: Glauber Costa <glommer@parallels.com> Reviewed-by: Hiroyouki Kamezawa <kamezawa.hiroyu@jp.fujitsu.com> CC: David S. Miller <davem@davemloft.net> CC: Eric W. Biederman <ebiederm@xmission.com> --- net/ipv4/tcp_memcontrol.c | 31 +++++++++++++++++++++++++++++++ 1 files changed, 31 insertions(+), 0 deletions(-) diff --git a/net/ipv4/tcp_memcontrol.c b/net/ipv4/tcp_memcontrol.c index 9481f23..d438fba 100644 --- a/net/ipv4/tcp_memcontrol.c +++ b/net/ipv4/tcp_memcontrol.c @@ -9,6 +9,7 @@ static u64 tcp_cgroup_read(struct cgroup *cont, struct cftype *cft); static int tcp_cgroup_write(struct cgroup *cont, struct cftype *cft, const char *buffer); +static int tcp_cgroup_reset(struct cgroup *cont, unsigned int event); static struct cftype tcp_files[] = { { @@ -22,6 +23,12 @@ static struct cftype tcp_files[] = { .read_u64 = tcp_cgroup_read, .private = RES_USAGE, }, + { + .name = "kmem.tcp.failcnt", + .private = RES_FAILCNT, + .trigger = tcp_cgroup_reset, + .read_u64 = tcp_cgroup_read, + }, }; static inline struct tcp_memcontrol *tcp_from_cgproto(struct cg_proto *cg_proto) @@ -197,12 +204,36 @@ static u64 tcp_cgroup_read(struct cgroup *cont, struct cftype *cft) case RES_USAGE: val = tcp_read_usage(memcg); break; + case RES_FAILCNT: + val = tcp_read_stat(memcg, RES_FAILCNT, 0); + break; default: BUG(); } return val; } +static int tcp_cgroup_reset(struct cgroup *cont, unsigned int event) +{ + struct mem_cgroup *memcg; + struct tcp_memcontrol *tcp; + struct cg_proto *cg_proto; + + memcg = mem_cgroup_from_cont(cont); + cg_proto = tcp_prot.proto_cgroup(memcg); + if (!cg_proto) + return 0; + tcp = tcp_from_cgproto(cg_proto); + + switch (event) { + case RES_FAILCNT: + res_counter_reset_failc…
creator: Glauber Costa
date: 2011-12-12T07:47:08-00:00

[PATCH v9 9/9] Display maximum tcp memory allocation in kmem cgroup
type: item
description: This patch introduces kmem.tcp.max_usage_in_bytes file, living in the kmem_cgroup filesystem. The root cgroup will display a value equal to RESOURCE_MAX. This is to avoid introducing any locking schemes in the network paths when cgroups are not being actively used. All others, will see the maximum memory ever used by this cgroup. Signed-off-by: Glauber Costa <glommer@parallels.com> Reviewed-by: Hiroyouki Kamezawa <kamezawa.hiroyu@jp.fujitsu.com> CC: David S. Miller <davem@davemloft.net> CC: Eric W. Biederman <ebiederm@xmission.com> --- net/ipv4/tcp_memcontrol.c | 12 +++++++++++- 1 files changed, 11 insertions(+), 1 deletions(-) diff --git a/net/ipv4/tcp_memcontrol.c b/net/ipv4/tcp_memcontrol.c index d438fba..171d7b6 100644 --- a/net/ipv4/tcp_memcontrol.c +++ b/net/ipv4/tcp_memcontrol.c @@ -29,6 +29,12 @@ static struct cftype tcp_files[] = { .trigger = tcp_cgroup_reset, .read_u64 = tcp_cgroup_read, }, + { + .name = "kmem.tcp.max_usage_in_bytes", + .private = RES_MAX_USAGE, + .trigger = tcp_cgroup_reset, + .read_u64 = tcp_cgroup_read, + }, }; static inline struct tcp_memcontrol *tcp_from_cgproto(struct cg_proto *cg_proto) @@ -205,7 +211,8 @@ static u64 tcp_cgroup_read(struct cgroup *cont, struct cftype *cft) val = tcp_read_usage(memcg); break; case RES_FAILCNT: - val = tcp_read_stat(memcg, RES_FAILCNT, 0); + case RES_MAX_USAGE: + val = tcp_read_stat(memcg, cft->private, 0); break; default: BUG(); @@ -226,6 +233,9 @@ static int tcp_cgroup_reset(struct cgroup *cont, unsigned int event) tcp = tcp_from_cgproto(cg_proto); switch (event) { + case RES_MAX_USAGE: + res_counter_reset_max(&tcp->tcp_memory_allocated); + break; case RES_FAILCNT: res_counter_reset_failcnt(&tcp->tcp_memory_allocated); break; -- 1.7.6.4
creator: Glauber Costa
date: 2011-12-12T07:47:09-00:00

Re: [PATCH v9 0/9] Request for inclusion: per-cgroup tcp memory pressure controls
type: item
description: From: Glauber Costa <glommer@parallels.com> Date: Mon, 12 Dec 2011 11:47:00 +0400 > This series fixes all the few comments raised in the last round, > and seem to have acquired consensus from the memcg side. > > Dave, do you think it is acceptable now from the networking PoV? > In case positive, would you prefer merging this trough your tree, > or acking this so a cgroup maintainer can do it? All applied to net-next, thanks.
creator: davem
date: 2011-12-13T00:07:34-00:00

Re: [PATCH v9 0/9] Request for inclusion: per-cgroup tcp memory pressure controls
type: item
description: Hi, On 12/13/2011 01:07 AM, David Miller wrote: > From: Glauber Costa <glommer@parallels.com> > Date: Mon, 12 Dec 2011 11:47:00 +0400 > >> This series fixes all the few comments raised in the last round, >> and seem to have acquired consensus from the memcg side. >> >> Dave, do you think it is acceptable now from the networking PoV? >> In case positive, would you prefer merging this trough your tree, >> or acking this so a cgroup maintainer can do it? > > All applied to net-next, thanks. now there are plenty of compiler-warnings when CONFIG_CGROUPS is not set: In file included from include/linux/tcp.h:211:0, from include/linux/ipv6.h:221, from include/net/ip_vs.h:23, from kernel/sysctl_binary.c:6: include/net/sock.h:67:57: warning: ‘struct cgroup_subsys’ declared inside parameter list [enabled by default] include/net/sock.h:67:57: warning: its scope is only this definition or declaration, which is probably not what you want [enabled by default] include/net/sock.h:67:57: warning: ‘struct cgroup’ declared inside parameter list [enabled by default] include/net/sock.h:68:61: warning: ‘struct cgroup_subsys’ declared inside parameter list [enabled by default] include/net/sock.h:68:61: warning: ‘struct cgroup’ declared inside parameter list [enabled by default] Because struct cgroup is only declared if CONFIG_CGROUPS is enabled. (cfr. linux/cgroup.h) Christoph -- Christoph Paasch PhD Student IP Networking Lab --- http://inl.info.ucl.ac.be MultiPath TCP in the Linux Kernel --- http://mptcp.info.ucl.ac.be Université Catholique de Louvain --
creator: Christoph Paasch
date: 2011-12-13T13:49:11-00:00

Re: [PATCH v9 0/9] Request for inclusion: per-cgroup tcp memory pressure controls
type: item
description: Le mardi 13 décembre 2011 à 14:49 +0100, Christoph Paasch a écrit : > now there are plenty of compiler-warnings when CONFIG_CGROUPS is not set: > > In file included from include/linux/tcp.h:211:0, > from include/linux/ipv6.h:221, > from include/net/ip_vs.h:23, > from kernel/sysctl_binary.c:6: > include/net/sock.h:67:57: warning: ‘struct cgroup_subsys’ declared > inside parameter list [enabled by default] > include/net/sock.h:67:57: warning: its scope is only this definition or > declaration, which is probably not what you want [enabled by default] > include/net/sock.h:67:57: warning: ‘struct cgroup’ declared inside > parameter list [enabled by default] > include/net/sock.h:68:61: warning: ‘struct cgroup_subsys’ declared > inside parameter list [enabled by default] > include/net/sock.h:68:61: warning: ‘struct cgroup’ declared inside > parameter list [enabled by default] > > > Because struct cgroup is only declared if CONFIG_CGROUPS is enabled. > (cfr. linux/cgroup.h) > Yes, we probably need forward reference like this : Thanks ! [PATCH net-next] net: fix build error if CONFIG_CGROUPS=n Reported-by: Christoph Paasch <christoph.paasch@uclouvain.be> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com> --- include/net/sock.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/include/net/sock.h b/include/net/sock.h index 18ecc99..6fe0dae 100644 --- a/include/net/sock.h +++ b/include/net/sock.h @@ -64,6 +64,8 @@ #include <net/dst.h> #include <net/checksum.h> +struct cgroup; +struct cgroup_subsys; int mem_cgroup_sockets_init(struct cgroup *cgrp, struct cgroup_subsys *ss); void mem_cgroup_sockets_destroy(struct cgroup *cgrp, struct cgroup_subsys *ss); /*
creator: Eric Dumazet
date: 2011-12-13T13:59:08-00:00

Re: [PATCH v9 0/9] Request for inclusion: per-cgroup tcp memory pressure controls
type: item
description: From: Eric Dumazet <eric.dumazet@gmail.com> Date: Tue, 13 Dec 2011 14:59:08 +0100 > [PATCH net-next] net: fix build error if CONFIG_CGROUPS=n > > Reported-by: Christoph Paasch <christoph.paasch@uclouvain.be> > Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com> Applied, thanks.
creator: davem
date: 2011-12-13T18:45:44-00:00

Re: [PATCH v9 0/9] Request for inclusion: per-cgroup tcp memory pressure controls
type: item
description: On 12/13/2011 05:59 PM, Eric Dumazet wrote: > Le mardi 13 décembre 2011 à 14:49 +0100, Christoph Paasch a écrit : > >> now there are plenty of compiler-warnings when CONFIG_CGROUPS is not set: >> >> In file included from include/linux/tcp.h:211:0, >> from include/linux/ipv6.h:221, >> from include/net/ip_vs.h:23, >> from kernel/sysctl_binary.c:6: >> include/net/sock.h:67:57: warning: ‘struct cgroup_subsys’ declared >> inside parameter list [enabled by default] >> include/net/sock.h:67:57: warning: its scope is only this definition or >> declaration, which is probably not what you want [enabled by default] >> include/net/sock.h:67:57: warning: ‘struct cgroup’ declared inside >> parameter list [enabled by default] >> include/net/sock.h:68:61: warning: ‘struct cgroup_subsys’ declared >> inside parameter list [enabled by default] >> include/net/sock.h:68:61: warning: ‘struct cgroup’ declared inside >> parameter list [enabled by default] >> >> >> Because struct cgroup is only declared if CONFIG_CGROUPS is enabled. >> (cfr. linux/cgroup.h) >> > > Yes, we probably need forward reference like this : > > Thanks ! > > [PATCH net-next] net: fix build error if CONFIG_CGROUPS=n > > Reported-by: Christoph Paasch<christoph.paasch@uclouvain.be> > Signed-off-by: Eric Dumazet<eric.dumazet@gmail.com> I am deeply sorry about that. I was pretty sure I tested this case. But now that I looked into it, it occurs to me that I may have tested it only with the Memory Cgroup disabled, not with the master flag off. Thanks for spotting this
creator: Glauber Costa
date: 2011-12-13T20:11:00-00:00

Re: [PATCH v9 1/9] Basic kernel memory functionality for the Memory Controller
type: item
description: [Now with the current patch version, I hope] On Mon 12-12-11 11:47:01, Glauber Costa wrote: > This patch lays down the foundation for the kernel memory component > of the Memory Controller. > > As of today, I am only laying down the following files: > > * memory.independent_kmem_limit Maybe has been already discussed but the name is rather awkward and it would deserve more clarification. It is independent in the way that it doesn't add up to the standard (user) allocations or it enables/disables accounting? > * memory.kmem.limit_in_bytes (currently ignored) What happens if we reach the limit? Are all kernel allocations considered or only selected caches? How do I find out which are those? AFAIU you have implemented it for network buffers at this stage but I guess that dentries will follow... > * memory.kmem.usage_in_bytes (always zero) > > Signed-off-by: Glauber Costa <glommer@parallels.com> > CC: Kirill A. Shutemov <kirill@shutemov.name> > CC: Paul Menage <paul@paulmenage.org> > CC: Greg Thelen <gthelen@google.com> > CC: Johannes Weiner <jweiner@redhat.com> > CC: Michal Hocko <mhocko@suse.cz> > --- > Documentation/cgroups/memory.txt | 40 ++++++++++++++- > init/Kconfig | 11 ++++ > mm/memcontrol.c | 105 ++++++++++++++++++++++++++++++++++++-- > 3 files changed, 149 insertions(+), 7 deletions(-) > > diff --git a/Documentation/cgroups/memory.txt b/Documentation/cgroups/memory.txt > index cc0ebc5..f245324 100644 > --- a/Documentation/cgroups/memory.txt > +++ b/Documentation/cgroups/memory.txt > @@ -44,8 +44,9 @@ Features: > - oom-killer disable knob and oom-notifier > - Root cgroup has no limit controls. > > - Kernel memory and Hugepages are not under control yet. We just manage > - pages on LRU. To add more controls, we have to take care of performance. > + Hugepages is not under control yet. We just manage pages on LRU. To add more Hugepages are not Anyway this sounds outdated as we track both THP and hugetlb, right? > + controls, we have to take care of performance. K…
creator: Michal Hocko
date: 2011-12-14T17:04:47-00:00

Re: [PATCH v9 0/9] Request for inclusion: per-cgroup tcp memory pressure controls
type: item
description: On Mon, 12 Dec 2011 11:47:00 +0400 Glauber Costa <glommer@parallels.com> wrote: > Hi, > > This series fixes all the few comments raised in the last round, > and seem to have acquired consensus from the memcg side. > > Dave, do you think it is acceptable now from the networking PoV? > In case positive, would you prefer merging this trough your tree, > or acking this so a cgroup maintainer can do it? > I met this bug at _1st_ run. Please enable _all_ debug options!. >From this log, your mem_cgroup_sockets_init() is totally buggy because tcp init routine will call percpu_alloc() under read_lock. please fix. I'll turn off the config for today's linux-next. Regards, -Kame == Dec 15 14:47:15 bluextal kernel: [ 228.386054] BUG: sleeping function called from invalid context at k ernel/mutex.c:271 Dec 15 14:47:15 bluextal kernel: [ 228.386286] in_atomic(): 1, irqs_disabled(): 0, pid: 2692, name: cg create Dec 15 14:47:15 bluextal kernel: [ 228.386438] 4 locks held by cgcreate/2692: Dec 15 14:47:15 bluextal kernel: [ 228.386580] #0: (&sb->s_type->i_mutex_key#15/1){+.+.+.}, at: [<ff ffffff8118717e>] kern_path_create+0x8e/0x150 Dec 15 14:47:15 bluextal kernel: [ 228.387238] #1: (cgroup_mutex){+.+.+.}, at: [<ffffffff810c2907>] cgroup_mkdir+0x77/0x3f0 Dec 15 14:47:15 bluextal kernel: [ 228.387755] #2: (&sb->s_type->i_mutex_key#15/2){+.+.+.}, at: [<ff ffffff810be168>] cgroup_create_file+0xc8/0xf0 Dec 15 14:47:15 bluextal kernel: [ 228.388401] #3: (proto_list_lock){++++.+}, at: [<ffffffff814e7fe4 >] mem_cgroup_sockets_init+0x24/0xf0 Dec 15 14:47:15 bluextal kernel: [ 228.388922] Pid: 2692, comm: cgcreate Not tainted 3.2.0-rc5-next-20 111214+ #34 Dec 15 14:47:15 bluextal kernel: [ 228.389154] Call Trace: Dec 15 14:47:15 bluextal kernel: [ 228.389296] [<ffffffff81080f07>] __might_sleep+0x107/0x140 Dec 15 14:47:15 bluextal kernel: [ 228.389446] [<ffffffff815ce15f>] mutex_lock_nested+0x2f/0x50 Dec 15 14:47:15 bluextal kernel: [ 228.389597] [<ffffffff811354fd>] pcpu_alloc+0x4d/0xa20 Dec…
creator: KAMEZAWA Hiroyuki
date: 2011-12-15T05:40:19-00:00

Re: [PATCH v9 0/9] Request for inclusion: per-cgroup tcp memory pressure controls
type: item
description: From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> Date: Thu, 15 Dec 2011 14:40:19 +0900 > I met this bug at _1st_ run. Please enable _all_ debug options!. Plus the CONFIG_NET=n and other build failures. This patch series was seriously rushed, and very poorly handled. Yet I kept getting so much pressure to review, comment upon, and ultimately apply these patches. Never, ever, do this to me ever again. If I don't feel your patches are high priority enough or ready enough for me to review, then TOO BAD. Don't ask people to pressure me or get my attention. Instead, ask others for help and do testing before wasting MY time and crapping up MY tree. I should have noticed a red flag when I have James Bottomly asking me to look at these patches, I should have pushed back. Instead, I relented, and now I'm very seriously regretting it. All the regressions in the net-next tree over the past several days have been due to this patch set, and this patch set alone. This code wasn't ready and needed, at a minimum, several more weeks of work before being put in. Instead, we're going to bandaid patch it up after the fact, rather than just letting these changes mature naturally during the review process.
creator: davem
date: 2011-12-15T05:48:36-00:00

Re: [PATCH v9 0/9] Request for inclusion: per-cgroup tcp memory pressure controls
type: item
description: On 12/15/2011 09:48 AM, David Miller wrote: > From: KAMEZAWA Hiroyuki<kamezawa.hiroyu@jp.fujitsu.com> > Date: Thu, 15 Dec 2011 14:40:19 +0900 > >> I met this bug at _1st_ run. Please enable _all_ debug options!. > > Plus the CONFIG_NET=n and other build failures. > > This patch series was seriously rushed, and very poorly handled. > > Yet I kept getting so much pressure to review, comment upon, and > ultimately apply these patches. Never, ever, do this to me ever > again. > > If I don't feel your patches are high priority enough or ready enough > for me to review, then TOO BAD. Don't ask people to pressure me or > get my attention. Instead, ask others for help and do testing before > wasting MY time and crapping up MY tree. > > I should have noticed a red flag when I have James Bottomly asking me > to look at these patches, I should have pushed back. Instead, I > relented, and now I'm very seriously regretting it. > > All the regressions in the net-next tree over the past several days > have been due to this patch set, and this patch set alone. > > This code wasn't ready and needed, at a minimum, several more weeks of > work before being put in. > > Instead, we're going to bandaid patch it up after the fact, rather > than just letting these changes mature naturally during the review > process. Hi Dave, You are right about all points. I will admit to it, face it, and apologize it. I guess the best I can do right now is fix whatever you guys point me to and not repeat it in the future. Thanks
creator: Glauber Costa
date: 2011-12-15T06:48:26-00:00

Re: [PATCH v9 1/9] Basic kernel memory functionality for the Memory Controller
type: item
description: On 12/14/2011 09:04 PM, Michal Hocko wrote: > [Now with the current patch version, I hope] > > On Mon 12-12-11 11:47:01, Glauber Costa wrote: >> This patch lays down the foundation for the kernel memory component >> of the Memory Controller. >> >> As of today, I am only laying down the following files: >> >> * memory.independent_kmem_limit > > Maybe has been already discussed but the name is rather awkward and it > would deserve more clarification. It is independent in the way that it > doesn't add up to the standard (user) allocations or it enables/disables > accounting? If turned on, it doesn't add up to the user allocations. As for the name, this is marked experimental, so I don't think anyone will be relying on it for a while. We can change it, if you have a better suggestion. >> * memory.kmem.limit_in_bytes (currently ignored) > > What happens if we reach the limit? Are all kernel allocations > considered or only selected caches? How do I find out which are those? > > AFAIU you have implemented it for network buffers at this stage but I > guess that dentries will follow... Further allocations should fail. About other caches, tcp is a bit different because we are concerned with conditions that applies after the allocation already took place. It is not clear to me if we will treat the other caches as a single entity, or separate them. >> * memory.kmem.usage_in_bytes (always zero) >> >> Signed-off-by: Glauber Costa<glommer@parallels.com> >> CC: Kirill A. Shutemov<kirill@shutemov.name> >> CC: Paul Menage<paul@paulmenage.org> >> CC: Greg Thelen<gthelen@google.com> >> CC: Johannes Weiner<jweiner@redhat.com> >> CC: Michal Hocko<mhocko@suse.cz> >> --- >> Documentation/cgroups/memory.txt | 40 ++++++++++++++- >> init/Kconfig | 11 ++++ >> mm/memcontrol.c | 105 ++++++++++++++++++++++++++++++++++++-- >> 3 files changed, 149 insertions(+), 7 deletions(-) >> >> diff --git a/Documentation/cgroups/memory.txt b/Documentation/cgroups/memory.txt >> index cc0ebc5..f245324 100644 >>…
creator: Glauber Costa
date: 2011-12-15T12:29:18-00:00

Re: [PATCH v9 1/9] Basic kernel memory functionality for the Memory Controller
type: item
description: On Sun, Dec 11, 2011 at 11:47 PM, Glauber Costa <glommer@parallels.com> wrote: > +Memory limits as specified by the standard Memory Controller may or may not > +take kernel memory into consideration. This is achieved through the file > +memory.independent_kmem_limit. A Value different than 0 will allow for kernel s/Value/value/ It is probably worth documenting the default value for memory.independent_kmem_limit? I figure it would be zero at root and and inherited from parents. But I think the implementation differs. > @@ -277,6 +281,11 @@ struct mem_cgroup { > */ > unsigned long move_charge_at_immigrate; > /* > + * Should kernel memory limits be stabilished independently > + * from user memory ? > + */ > + int kmem_independent_accounting; I have no serious objection, but a full int seems like overkill for a boolean value. > +static int register_kmem_files(struct cgroup *cont, struct cgroup_subsys *ss) > +{ > + int ret = 0; > + > + ret = cgroup_add_files(cont, ss, kmem_cgroup_files, > + ARRAY_SIZE(kmem_cgroup_files)); > + return ret; If you want to this function could be condensed down to: return cgroup_add_files(...);
creator: Greg Thelen
date: 2011-12-16T06:20:52-00:00

Re: [PATCH v9 1/9] Basic kernel memory functionality for the Memory Controller
type: item
description: On Thu 15-12-11 16:29:18, Glauber Costa wrote: > On 12/14/2011 09:04 PM, Michal Hocko wrote: > >[Now with the current patch version, I hope] > >On Mon 12-12-11 11:47:01, Glauber Costa wrote: [...] > >>@@ -3848,10 +3862,17 @@ static inline u64 mem_cgroup_usage(struct mem_cgroup *memcg, bool swap) > >> u64 val; > >> > >> if (!mem_cgroup_is_root(memcg)) { > >>+ val = 0; > >>+#ifdef CONFIG_CGROUP_MEM_RES_CTLR_KMEM > >>+ if (!memcg->kmem_independent_accounting) > >>+ val = res_counter_read_u64(&memcg->kmem, RES_USAGE); > >>+#endif > >> if (!swap) > >>- return res_counter_read_u64(&memcg->res, RES_USAGE); > >>+ val += res_counter_read_u64(&memcg->res, RES_USAGE); > >> else > >>- return res_counter_read_u64(&memcg->memsw, RES_USAGE); > >>+ val += res_counter_read_u64(&memcg->memsw, RES_USAGE); > >>+ > >>+ return val; > >> } > > > >So you report kmem+user but we do not consider kmem during charge so one > >can easily end up with usage_in_bytes over limit but no reclaim is going > >on. Not good, I would say. I find this a problem and one of the reason I do not like !independent accounting. > > > >OK, so to sum it up. The biggest problem I see is the (non)independent > >accounting. We simply cannot mix user+kernel limits otherwise we would > >see issues (like kernel resource hog would force memcg-oom and innocent > >members would die because their rss is much bigger). > >It is also not clear to me what should happen when we hit the kmem > >limit. I guess it will be kmem cache dependent. > > So right now, tcp is completely independent, since it is not > accounted to kmem. So why do we need kmem accounting when tcp (the only user at the moment) doesn't use it? > In summary, we still never do non-independent accounting. When we > start doing it for the other caches, We will have to add a test at > charge time as well. So we shouldn't do it as a part of this patchset because the further usage is not clear and I think there will be some real issues with user+kmem accounting (e.g. …
creator: Michal Hocko
date: 2011-12-16T12:32:33-00:00

Re: [PATCH v9 1/9] Basic kernel memory functionality for the Memory Controller
type: item
description: On 12/16/2011 04:32 PM, Michal Hocko wrote: > On Thu 15-12-11 16:29:18, Glauber Costa wrote: >> On 12/14/2011 09:04 PM, Michal Hocko wrote: >>> [Now with the current patch version, I hope] >>> On Mon 12-12-11 11:47:01, Glauber Costa wrote: > [...] >>>> @@ -3848,10 +3862,17 @@ static inline u64 mem_cgroup_usage(struct mem_cgroup *memcg, bool swap) >>>> u64 val; >>>> >>>> if (!mem_cgroup_is_root(memcg)) { >>>> + val = 0; >>>> +#ifdef CONFIG_CGROUP_MEM_RES_CTLR_KMEM >>>> + if (!memcg->kmem_independent_accounting) >>>> + val = res_counter_read_u64(&memcg->kmem, RES_USAGE); >>>> +#endif >>>> if (!swap) >>>> - return res_counter_read_u64(&memcg->res, RES_USAGE); >>>> + val += res_counter_read_u64(&memcg->res, RES_USAGE); >>>> else >>>> - return res_counter_read_u64(&memcg->memsw, RES_USAGE); >>>> + val += res_counter_read_u64(&memcg->memsw, RES_USAGE); >>>> + >>>> + return val; >>>> } >>> >>> So you report kmem+user but we do not consider kmem during charge so one >>> can easily end up with usage_in_bytes over limit but no reclaim is going >>> on. Not good, I would say. > > I find this a problem and one of the reason I do not like !independent > accounting. > >>> >>> OK, so to sum it up. The biggest problem I see is the (non)independent >>> accounting. We simply cannot mix user+kernel limits otherwise we would >>> see issues (like kernel resource hog would force memcg-oom and innocent >>> members would die because their rss is much bigger). >>> It is also not clear to me what should happen when we hit the kmem >>> limit. I guess it will be kmem cache dependent. >> >> So right now, tcp is completely independent, since it is not >> accounted to kmem. > > So why do we need kmem accounting when tcp (the only user at the moment) > doesn't use it? Well, a bit historical. I needed a basic placeholder for it, since it tcp is officially kmem. As the time passed, I took most of the stuff out of this patch to leave just the basics I would need for tcp. Turns out I ended up focusing …
creator: Glauber Costa
date: 2011-12-16T13:02:51-00:00

Re: [PATCH v9 1/9] Basic kernel memory functionality for the Memory Controller
type: item
description: On Fri 16-12-11 17:02:51, Glauber Costa wrote: > On 12/16/2011 04:32 PM, Michal Hocko wrote: [...] > >So why do we need kmem accounting when tcp (the only user at the moment) > >doesn't use it? > > Well, a bit historical. I needed a basic placeholder for it, since > it tcp is officially kmem. As the time passed, I took most of the > stuff out of this patch to leave just the basics I would need for > tcp. > Turns out I ended up focusing on the rest, and some of the stuff was > left here. > > At one point I merged tcp data into kmem, but then reverted this > behavior. the kmem counter stayed. > > I agree deferring the whole behavior would be better. > > >>In summary, we still never do non-independent accounting. When we > >>start doing it for the other caches, We will have to add a test at > >>charge time as well. > > > >So we shouldn't do it as a part of this patchset because the further > >usage is not clear and I think there will be some real issues with > >user+kmem accounting (e.g. a proper memcg-oom implementation). > >Can you just drop this patch? > > Yes, but the whole set is in the net tree already. Isn't it only in some for-next branch? Can that one be updated? > (All other patches are tcp-related but this) Would you mind if I'd > send a follow up patch removing the kmem files, and leaving just the > registration functions and basic documentation? (And sorry for that as > well in advance) Yes a followup patch would work as well. -- Michal Hocko SUSE Labs SUSE LINUX s.r.o. Lihovarska 1060/12 190 00 Praha 9 Czech Republic
creator: Michal Hocko
date: 2011-12-16T13:30:49-00:00

Re: [PATCH v9 3/9] socket: initial cgroup code.
type: item
description: On Mon, Dec 12, 2011 at 11:47:03AM +0400, Glauber Costa wrote: > + > +static bool mem_cgroup_is_root(struct mem_cgroup *memcg); > +void sock_update_memcg(struct sock *sk) > +{ > + /* A socket spends its whole life in the same cgroup */ > + if (sk->sk_cgrp) { > + WARN_ON(1); > + return; > + } > + if (static_branch(&memcg_socket_limit_enabled)) { > + struct mem_cgroup *memcg; > + > + BUG_ON(!sk->sk_prot->proto_cgroup); > + > + rcu_read_lock(); > + memcg = mem_cgroup_from_task(current); > + if (!mem_cgroup_is_root(memcg)) { > + mem_cgroup_get(memcg); > + sk->sk_cgrp = sk->sk_prot->proto_cgroup(memcg); > + } > + rcu_read_unlock(); > + } > +} > +EXPORT_SYMBOL(sock_update_memcg); > + > +void sock_release_memcg(struct sock *sk) > +{ > + if (static_branch(&memcg_socket_limit_enabled) && sk->sk_cgrp) { > + struct mem_cgroup *memcg; > + WARN_ON(!sk->sk_cgrp->memcg); > + memcg = sk->sk_cgrp->memcg; > + mem_cgroup_put(memcg); > + } > +} Hi Glauber, I think for 'sock_release_memcg()', you want: static inline sock_release_memcg(sk) { if (static_branch()) __sock_release_memcg(); } And then re-define the current sock_release_memcg -> __sock_release_memcg(). In that way the straight line path is a single no-op. As currently written, there is function call and then an immediate return. Thanks, -Jason
creator: Jason Baron
date: 2011-12-22T21:10:29-00:00

Re: [PATCH v9 3/9] socket: initial cgroup code.
type: item
description: On 12/23/2011 01:10 AM, Jason Baron wrote: > On Mon, Dec 12, 2011 at 11:47:03AM +0400, Glauber Costa wrote: >> + >> +static bool mem_cgroup_is_root(struct mem_cgroup *memcg); >> +void sock_update_memcg(struct sock *sk) >> +{ >> + /* A socket spends its whole life in the same cgroup */ >> + if (sk->sk_cgrp) { >> + WARN_ON(1); >> + return; >> + } >> + if (static_branch(&memcg_socket_limit_enabled)) { >> + struct mem_cgroup *memcg; >> + >> + BUG_ON(!sk->sk_prot->proto_cgroup); >> + >> + rcu_read_lock(); >> + memcg = mem_cgroup_from_task(current); >> + if (!mem_cgroup_is_root(memcg)) { >> + mem_cgroup_get(memcg); >> + sk->sk_cgrp = sk->sk_prot->proto_cgroup(memcg); >> + } >> + rcu_read_unlock(); >> + } >> +} >> +EXPORT_SYMBOL(sock_update_memcg); >> + >> +void sock_release_memcg(struct sock *sk) >> +{ >> + if (static_branch(&memcg_socket_limit_enabled)&& sk->sk_cgrp) { >> + struct mem_cgroup *memcg; >> + WARN_ON(!sk->sk_cgrp->memcg); >> + memcg = sk->sk_cgrp->memcg; >> + mem_cgroup_put(memcg); >> + } >> +} > > Hi Glauber, > > I think for 'sock_release_memcg()', you want: > > static inline sock_release_memcg(sk) > { > if (static_branch()) > __sock_release_memcg(); > } > > And then re-define the current sock_release_memcg -> __sock_release_memcg(). > In that way the straight line path is a single no-op. As currently > written, there is function call and then an immediate return. > Hello Jason, Thanks for the tip. I may be wrong here, but I don't think that the release performance matters to that level. But your suggestion seems good nevertheless. Since this is already sitting on a tree, would you like to send a patch for that?
creator: Glauber Costa
date: 2011-12-23T08:57:49-00:00