OpenVZ Forum - RDF feed
type: channel
items: [PATCH v7 0/8] Request for inclusion: tcp memory buffers
items: [PATCH v7 1/8] Basic kernel memory functionality for the Memory Controller
items: [PATCH v7 2/8] socket: initial cgroup code.
items: [PATCH v7 3/8] foundations of per-cgroup memory pressure controlling.
items: [PATCH v7 4/8] per-cgroup tcp buffers control
items: [PATCH v7 5/8] per-netns ipv4 sysctl_tcp_mem
items: [PATCH v7 6/8] tcp buffer limitation: per-cgroup limit
items: [PATCH v7 7/8] Display current tcp memory allocation in kmem cgroup
items: [PATCH v7 8/8] Disable task moving when using kernel memory accounting
items: Re: [PATCH v7 0/8] Request for inclusion: tcp memory buffers

[PATCH v7 0/8] Request for inclusion: tcp memory buffers
type: item
description: Hi Andrew, This series was extensively reviewed over the past month, and after all major comments were merged, I feel it is ready for inclusion when the next merge window opens. Minor fixes will be provided if they prove to be necessary. You can see the most recent past discussions at: https://lkml.org/lkml/2011/10/10/116 https://lkml.org/lkml/2011/10/4/116 https://lkml.org/lkml/2011/10/3/133 It basically lays the foundation for kernel memory limitation as a general framework, and uses it to control the existent tcp memory pressure thresholds in a per-cgroup way. Follow up patches are expected for soft limits, which are not handled. Please consider this for inclusion in your tree Thanks, Glauber Costa (8): Basic kernel memory functionality for the Memory Controller socket: initial cgroup code. foundations of per-cgroup memory pressure controlling. per-cgroup tcp buffers control per-netns ipv4 sysctl_tcp_mem tcp buffer limitation: per-cgroup limit Display current tcp memory allocation in kmem cgroup Disable task moving when using kernel memory accounting Documentation/cgroups/memory.txt | 38 ++++- crypto/af_alg.c | 8 +- include/linux/memcontrol.h | 49 +++++ include/net/netns/ipv4.h | 1 + include/net/sock.h | 130 +++++++++++++- include/net/tcp.h | 30 +++- include/net/udp.h | 4 +- include/trace/events/sock.h | 10 +- init/Kconfig | 14 ++ mm/memcontrol.c | 373 ++++++++++++++++++++++++++++++++++++-- net/core/sock.c | 104 ++++++++--- net/decnet/af_decnet.c | 22 ++- net/ipv4/proc.c | 7 +- net/ipv4/sysctl_net_ipv4.c | 71 +++++++- net/ipv4/tcp.c | 60 ++++--- net/ipv4/tcp_input.c | 12 +- net/ipv4/tcp_ipv4.c | 23 ++- net/ipv4/tcp_output.c | 2 +- net/ipv4/tcp_timer.c | 2 +- net/ipv4/udp.c | 21 ++- net/ipv6/tcp_ipv6.c | 20 ++- net/ipv6/udp.c | 4 +- net/sctp/socket.c | 37 +++- 23 files changed, 910 insertions(+), 132 deletions(-) -- 1.7.6.4
creator: Glauber Costa
date: 2011-10-13T13:09:34-00:00

[PATCH v7 1/8] 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> Reviewed-by: Kirill A. Shutemov <kirill@shutemov.name> Reviewed-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> CC: Paul Menage <paul@paulmenage.org> CC: Greg Thelen <gthelen@google.com> --- Documentation/cgroups/memory.txt | 36 ++++++++++++++- init/Kconfig | 14 ++++++ mm/memcontrol.c | 89 +++++++++++++++++++++++++++++++++++-- 3 files changed, 132 insertions(+), 7 deletions(-) diff --git a/Documentation/cgroups/memory.txt b/Documentation/cgroups/memory.txt index 06eb6d9..0dafd70 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 memory+…
creator: Glauber Costa
date: 2011-10-13T13:09:35-00:00

[PATCH v7 2/8] socket: initial cgroup code.
type: item
description: We aim to control the amount of kernel memory pinned at any time by tcp sockets. To lay the foundations for this work, this patch adds a pointer to the kmem_cgroup to the socket structure. Signed-off-by: Glauber Costa <glommer@parallels.com> Acked-by: Kirill A. Shutemov<kirill@shutemov.name> Reviewed-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujtsu.com> CC: David S. Miller <davem@davemloft.net> CC: Eric W. Biederman <ebiederm@xmission.com> --- include/linux/memcontrol.h | 15 +++++++++++++++ include/net/sock.h | 2 ++ mm/memcontrol.c | 37 +++++++++++++++++++++++++++++++++++++ net/core/sock.c | 3 +++ 4 files changed, 57 insertions(+), 0 deletions(-) diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h index 343bd76..88aea1b 100644 --- a/include/linux/memcontrol.h +++ b/include/linux/memcontrol.h @@ -376,5 +376,20 @@ mem_cgroup_print_bad_page(struct page *page) } #endif +#ifdef CONFIG_INET +struct sock; +#ifdef CONFIG_CGROUP_MEM_RES_CTLR_KMEM +void sock_update_memcg(struct sock *sk); +void sock_release_memcg(struct sock *sk); + +#else +static inline void sock_update_memcg(struct sock *sk) +{ +} +static inline void sock_release_memcg(struct sock *sk) +{ +} +#endif /* CONFIG_CGROUP_MEM_RES_CTLR_KMEM */ +#endif /* CONFIG_INET */ #endif /* _LINUX_MEMCONTROL_H */ diff --git a/include/net/sock.h b/include/net/sock.h index 8e4062f..afe1467 100644 --- a/include/net/sock.h +++ b/include/net/sock.h @@ -228,6 +228,7 @@ struct sock_common { * @sk_security: used by security modules * @sk_mark: generic packet mark * @sk_classid: this socket's cgroup classid + * @sk_cgrp: this socket's kernel memory (kmem) cgroup * @sk_write_pending: a write to stream socket waits to start * @sk_state_change: callback to indicate change in the state of the sock * @sk_data_ready: callback to indicate there is data to be processed @@ -339,6 +340,7 @@ struct sock { #endif __u32 sk_mark; u32 sk_classid; + struct mem_cgroup *sk_cgrp; void (*sk_state_change)(struct sock *sk); void (*sk_data_…
creator: Glauber Costa
date: 2011-10-13T13:09:36-00:00

[PATCH v7 3/8] foundations of per-cgroup memory pressure controlling.
type: item
description: This patch converts struct sock fields memory_pressure, memory_allocated, sockets_allocated, and sysctl_mem (now prot_mem) to function pointers, receiving a struct mem_cgroup parameter. enter_memory_pressure is kept the same, since all its callers have socket a context, and the kmem_cgroup can be derived from the socket itself. To keep things working, the patch convert all users of those fields to use acessor functions. In my benchmarks I didn't see a significant performance difference with this patch applied compared to a baseline (around 1 % diff, thus inside error margin). 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> --- crypto/af_alg.c | 8 +++- include/linux/memcontrol.h | 22 ++++++++- include/net/sock.h | 114 ++++++++++++++++++++++++++++++++++++++++-- include/net/tcp.h | 12 +++-- include/net/udp.h | 4 +- include/trace/events/sock.h | 10 ++-- mm/memcontrol.c | 19 ++++++- net/core/sock.c | 62 ++++++++++++++--------- net/decnet/af_decnet.c | 22 +++++++- net/ipv4/proc.c | 7 ++- net/ipv4/tcp.c | 28 +++++++++- net/ipv4/tcp_input.c | 12 ++-- net/ipv4/tcp_ipv4.c | 12 ++-- net/ipv4/tcp_output.c | 2 +- net/ipv4/tcp_timer.c | 2 +- net/ipv4/udp.c | 21 ++++++-- net/ipv6/tcp_ipv6.c | 10 ++-- net/ipv6/udp.c | 4 +- net/sctp/socket.c | 37 +++++++++++--- 19 files changed, 320 insertions(+), 88 deletions(-) diff --git a/crypto/af_alg.c b/crypto/af_alg.c index ac33d5f..09cdf11 100644 --- a/crypto/af_alg.c +++ b/crypto/af_alg.c @@ -29,10 +29,16 @@ struct alg_type_list { static atomic_long_t alg_memory_allocated; +static long memory_allocated_alg(struct mem_cgroup *memcg, long val, + int *parent_status) +{ + return atomic_long_add_return(val, &alg_memory_allocated); +} + static struct proto alg_proto = { .name = "ALG", .owner = THIS_MODULE, - .memory_allocated = &alg_memory_allocated, + .mem_allocated_add = memory_allocated_al…
creator: Glauber Costa
date: 2011-10-13T13:09:37-00:00

[PATCH v7 4/8] per-cgroup tcp buffers control
type: item
description: With all the infrastructure in place, this patch implements per-cgroup control for tcp memory pressure handling. 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/linux/memcontrol.h | 4 + include/net/sock.h | 14 ++++ include/net/tcp.h | 17 +++++ mm/memcontrol.c | 141 ++++++++++++++++++++++++++++++++++++++++++++ net/core/sock.c | 39 +++++++++++- net/ipv4/tcp.c | 47 +++++++-------- net/ipv4/tcp_ipv4.c | 11 ++++ net/ipv6/tcp_ipv6.c | 10 +++- 8 files changed, 255 insertions(+), 28 deletions(-) diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h index 99a8ba2..a27dad9 100644 --- a/include/linux/memcontrol.h +++ b/include/linux/memcontrol.h @@ -393,6 +393,10 @@ void sock_update_memcg(struct sock *sk); void sock_release_memcg(struct sock *sk); void memcg_sockets_allocated_dec(struct mem_cgroup *memcg, struct proto *prot); void memcg_sockets_allocated_inc(struct mem_cgroup *memcg, struct proto *prot); +int tcp_init_cgroup(const struct proto *prot, struct cgroup *cgrp, + struct cgroup_subsys *ss); +void tcp_destroy_cgroup(const struct proto *prot, struct cgroup *cgrp, + struct cgroup_subsys *ss); #else /* memcontrol includes sockets.h, that includes memcontrol.h ... */ static inline void memcg_sockets_allocated_dec(struct mem_cgroup *memcg, diff --git a/include/net/sock.h b/include/net/sock.h index 163f87b..efd7664 100644 --- a/include/net/sock.h +++ b/include/net/sock.h @@ -64,6 +64,8 @@ #include <net/dst.h> #include <net/checksum.h> +int sockets_populate(struct cgroup *cgrp, struct cgroup_subsys *ss); +void sockets_destroy(struct cgroup *cgrp, struct cgroup_subsys *ss); /* * This structure really needs to be cleaned up. * Most of it is for TCP, and not used by any of @@ -819,6 +821,18 @@ struct proto { /* Pointer to the per-cgroup version of the the sysctl_mem field */ long *(*prot_mem)(const …
creator: Glauber Costa
date: 2011-10-13T13:09:38-00:00

[PATCH v7 5/8] 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 - mm/memcontrol.c | 8 ++++-- net/ipv4/sysctl_net_ipv4.c | 51 +++++++++++++++++++++++++++++++++++++------ net/ipv4/tcp.c | 13 ++-------- 5 files changed, 53 insertions(+), 21 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 ec57cf2..3609d87 100644 --- a/include/net/tcp.h +++ b/include/net/tcp.h @@ -232,7 +232,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/mm/memcontrol.c b/mm/memcontrol.c index 4e79171..f953b32 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -390,6 +390,7 @@ static struct mem_cgroup *mem_cgroup_from_cont(struct cgroup *cont); #ifdef CONFIG_INET #include <net/sock.h> #include <net/ip.h> +#include <linux/nsproxy.h> void sock_update_memcg(struct sock *sk) { @@ -526,14 +527,15 @@ int tcp_init_cgroup(const struct proto *prot, struct cgroup *cgrp, struct cgroup_subsys *ss) { struct mem_cgroup *memcg = mem_cgroup_from_cont(cgrp); + struct net *net = current->nsproxy->net_n…
creator: Glauber Costa
date: 2011-10-13T13:09:39-00:00

[PATCH v7 6/8] tcp buffer limitation: per-cgroup limit
type: item
description: This patch uses the "tcp_max_mem" field of the kmem_cgroup to effectively control the amount of kernel memory pinned by a cgroup. We have to make sure that none of the memory pressure thresholds specified in the namespace are bigger than the current cgroup. 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> --- Documentation/cgroups/memory.txt | 1 + include/linux/memcontrol.h | 10 +++++ mm/memcontrol.c | 79 ++++++++++++++++++++++++++++++++++---- net/ipv4/sysctl_net_ipv4.c | 20 ++++++++++ 4 files changed, 102 insertions(+), 8 deletions(-) diff --git a/Documentation/cgroups/memory.txt b/Documentation/cgroups/memory.txt index 0dafd70..e773bd7 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/linux/memcontrol.h b/include/linux/memcontrol.h index a27dad9..e0ccec5 100644 --- a/include/linux/memcontrol.h +++ b/include/linux/memcontrol.h @@ -397,6 +397,9 @@ int tcp_init_cgroup(const struct proto *prot, struct cgroup *cgrp, struct cgroup_subsys *ss); void tcp_destroy_cgroup(const struct proto *prot, 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); #else /* memcontrol includes sockets.h, that includes memcontrol.h ... */ static inline void memcg_sockets_allocated_dec(struct mem_cgroup *memcg, @@ -413,6 +416,13 @@ static inline void sock_update_memcg(struct sock *sk) static inline void sock_release_memcg(struct sock *sk) { } +static inline unsigned long long tcp_max_memory(const struct mem_cgroup *memcg)…
creator: Glauber Costa
date: 2011-10-13T13:09:40-00:00

[PATCH v7 7/8] Display current tcp memory allocation in kmem cgroup
type: item
description: This patch introduces kmem.tcp_current_memory 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: KAMEZAWA Hiroyuki<kamezawa.hiroyu@jp.fujitsu.com> CC: David S. Miller <davem@davemloft.net> CC: Eric W. Biederman <ebiederm@xmission.com> --- Documentation/cgroups/memory.txt | 1 + mm/memcontrol.c | 5 +++++ 2 files changed, 6 insertions(+), 0 deletions(-) diff --git a/Documentation/cgroups/memory.txt b/Documentation/cgroups/memory.txt index e773bd7..b937a99 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/mm/memcontrol.c b/mm/memcontrol.c index b696267..1ba318d 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -543,6 +543,11 @@ static struct cftype tcp_files[] = { .read_u64 = mem_cgroup_read, .private = MEMFILE_PRIVATE(_KMEM_TCP, RES_LIMIT), }, + { + .name = "kmem.tcp.usage_in_bytes", + .read_u64 = mem_cgroup_read, + .private = MEMFILE_PRIVATE(_KMEM_TCP, RES_USAGE), + }, }; static void tcp_create_cgroup(struct mem_cgroup *cg, struct cgroup_subsys *ss) -- 1.7.6.4
creator: Glauber Costa
date: 2011-10-13T13:09:41-00:00

[PATCH v7 8/8] Disable task moving when using kernel memory accounting
type: item
description: Since this code is still experimental, we are leaving the exact details of how to move tasks between cgroups when kernel memory accounting is used as future work. For now, we simply disallow movement if there are any pending accounted memory. Signed-off-by: Glauber Costa <glommer@parallels.com> Reviewed-by: KAMEZAWA Hiroyuki<kamezawa.hiroyu@jp.fujitsu.com> --- mm/memcontrol.c | 31 ++++++++++++++++++------------- 1 files changed, 18 insertions(+), 13 deletions(-) diff --git a/mm/memcontrol.c b/mm/memcontrol.c index 1ba318d..b46232b 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -408,23 +408,11 @@ void sock_update_memcg(struct sock *sk) rcu_read_lock(); sk->sk_cgrp = mem_cgroup_from_task(current); - - /* - * We don't need to protect against anything task-related, because - * we are basically stuck with the sock pointer that won't change, - * even if the task that originated the socket changes cgroups. - * - * What we do have to guarantee, is that the chain leading us to - * the top level won't change under our noses. Incrementing the - * reference count via cgroup_exclude_rmdir guarantees that. - */ - cgroup_exclude_rmdir(mem_cgroup_css(sk->sk_cgrp)); rcu_read_unlock(); } void sock_release_memcg(struct sock *sk) { - cgroup_release_and_wakeup_rmdir(mem_cgroup_css(sk->sk_cgrp)); } void memcg_sockets_allocated_dec(struct mem_cgroup *memcg, struct proto *prot) @@ -5634,10 +5622,17 @@ static int mem_cgroup_can_attach(struct cgroup_subsys *ss, { int ret = 0; struct mem_cgroup *mem = mem_cgroup_from_cont(cgroup); + struct mem_cgroup *from = mem_cgroup_from_task(p); + + if (from != mem && + res_counter_read_u64(&mem->tcp.tcp_memory_allocated, RES_USAGE)) { + printk(KERN_WARNING "Can't move tasks between cgroups: " + "Kernel memory held. task: %s\n", p->comm); + return 1; + } if (mem->move_charge_at_immigrate) { struct mm_struct *mm; - struct mem_cgroup *from = mem_cgroup_from_task(p); VM_BUG_ON(from == mem); @@ -5805,6 +5800,16 @@ static int mem_cgroup_can_attach(struc…
creator: Glauber Costa
date: 2011-10-13T13:09:42-00:00

Re: [PATCH v7 0/8] Request for inclusion: tcp memory buffers
type: item
description: From: Glauber Costa <glommer@parallels.com> Date: Thu, 13 Oct 2011 17:09:34 +0400 > This series was extensively reviewed over the past month, and after > all major comments were merged, I feel it is ready for inclusion when > the next merge window opens. Minor fixes will be provided if they > prove to be necessary. I'm not applying this. You're turning inline increments and decrements of the existing memory limits into indirect function calls. That imposes a new non-trivial cost, in fast paths, even when people do not use your feature. Make this evaluate into exactly the same exact code stream we have now when the memory cgroup feature is not in use, which will be the majority of users.
creator: davem
date: 2011-10-13T20:00:31-00:00

Re: [PATCH v7 0/8] Request for inclusion: tcp memory buffers
type: item
description: On 10/14/2011 12:00 AM, David Miller wrote: > From: Glauber Costa<glommer@parallels.com> > Date: Thu, 13 Oct 2011 17:09:34 +0400 > >> This series was extensively reviewed over the past month, and after >> all major comments were merged, I feel it is ready for inclusion when >> the next merge window opens. Minor fixes will be provided if they >> prove to be necessary. > > I'm not applying this. Thank you for letting me now about your view of this that early. > You're turning inline increments and decrements of the existing memory > limits into indirect function calls. Yes, indeed. > That imposes a new non-trivial cost, in fast paths, even when people > do not use your feature. Well, there is a cost, but all past submissions included round trip benchmarks. In none of them I could see any significant slowdown. > Make this evaluate into exactly the same exact code stream we have > now when the memory cgroup feature is not in use, which will be the > majority of users. What exactly do you mean by "not in use" ? Not compiled in or not actively being exercised ? If you mean the later, I appreciate tips on how to achieve it. Also, I kind of dispute the affirmation that !cgroup will encompass the majority of users, since cgroups is being enabled by default by most vendors. All systemd based systems use it extensively, for instance.
creator: Glauber Costa
date: 2011-10-13T20:05:58-00:00

Re: [PATCH v7 0/8] Request for inclusion: tcp memory buffers
type: item
description: From: Glauber Costa <glommer@parallels.com> Date: Fri, 14 Oct 2011 00:05:58 +0400 > On 10/14/2011 12:00 AM, David Miller wrote: >> That imposes a new non-trivial cost, in fast paths, even when people >> do not use your feature. > Well, there is a cost, but all past submissions included round trip > benchmarks. > In none of them I could see any significant slowdown. Did you try millions of sockets doing all kinds of different accesses? Did you check the nanosecond latency of operations over loopback so that the real cost of you change can be isolated and thus measured properly?
creator: davem
date: 2011-10-13T20:08:54-00:00

Re: [PATCH v7 0/8] Request for inclusion: tcp memory buffers
type: item
description: From: Glauber Costa <glommer@parallels.com> Date: Fri, 14 Oct 2011 00:05:58 +0400 > Also, I kind of dispute the affirmation that !cgroup will encompass > the majority of users, since cgroups is being enabled by default by > most vendors. All systemd based systems use it extensively, for > instance. I will definitely advise people against this, since the cost of having this on by default is absolutely non-trivial. People keep asking every few releases "where the heck has my performance gone" and it's because of creeping features like this. This socket cgroup feature is a prime example of where that kind of stuff comes from. I really get irritated when people go "oh, it's just one indirect function call" and "oh, it's just one more pointer in struct sock" We work really hard to _remove_ elements from structures and make them smaller, and to remove expensive operations from the fast paths. It might take someone weeks if not months to find a way to make a patch which compensates for the extra overhead your patches are adding. And I don't think you fully appreciate that.
creator: davem
date: 2011-10-13T20:12:21-00:00

Re: [PATCH v7 0/8] Request for inclusion: tcp memory buffers
type: item
description: On 10/14/2011 12:12 AM, David Miller wrote: > From: Glauber Costa<glommer@parallels.com> > Date: Fri, 14 Oct 2011 00:05:58 +0400 > >> Also, I kind of dispute the affirmation that !cgroup will encompass >> the majority of users, since cgroups is being enabled by default by >> most vendors. All systemd based systems use it extensively, for >> instance. > > I will definitely advise people against this, since the cost of having > this on by default is absolutely non-trivial. > > People keep asking every few releases "where the heck has my performance > gone" and it's because of creeping features like this. This socket > cgroup feature is a prime example of where that kind of stuff comes > from. > > I really get irritated when people go "oh, it's just one indirect > function call" and "oh, it's just one more pointer in struct sock" > > We work really hard to _remove_ elements from structures and make them > smaller, and to remove expensive operations from the fast paths. > > It might take someone weeks if not months to find a way to make a > patch which compensates for the extra overhead your patches are adding. > > And I don't think you fully appreciate that. Let's focus on this: Are you happy, or at least willing to accept, an approach that keep things as they were with cgroups *compiled out*, or were you referring to not in use == compiled in, but with no users?
creator: Glauber Costa
date: 2011-10-13T20:14:40-00:00

Re: [PATCH v7 0/8] Request for inclusion: tcp memory buffers
type: item
description: From: Glauber Costa <glommer@parallels.com> Date: Fri, 14 Oct 2011 00:05:58 +0400 > Thank you for letting me now about your view of this that early. I depend upon my colleagues to assist me in the large task that is reviewing the enormous number of networking patches that get submitted. Unfortunately, none of them got a chance to review this patch set seriously, since I know most of them (especially Eric Dumazet) would balk at the overhead you're proposing to add to our stack, just as I did. This is the reality of the situation, and I'm sorry to tell you that snippy retorts when someone does take the time out to review your work won't help at all.
creator: davem
date: 2011-10-13T20:16:08-00:00

Re: [PATCH v7 0/8] Request for inclusion: tcp memory buffers
type: item
description: From: Glauber Costa <glommer@parallels.com> Date: Fri, 14 Oct 2011 00:14:40 +0400 > Are you happy, or at least willing to accept, an approach that keep > things as they were with cgroups *compiled out*, or were you referring > to not in use == compiled in, but with no users? To me these are the same exact thing, because %99 of users will be running a kernel with every feature turned on in the Kconfig.
creator: davem
date: 2011-10-13T20:18:19-00:00

Re: [PATCH v7 0/8] Request for inclusion: tcp memory buffers
type: item
description: On 10/14/2011 12:16 AM, David Miller wrote: > From: Glauber Costa<glommer@parallels.com> > Date: Fri, 14 Oct 2011 00:05:58 +0400 > >> Thank you for letting me now about your view of this that early. > > I depend upon my colleagues to assist me in the large task that is reviewing > the enormous number of networking patches that get submitted. > > Unfortunately, none of them got a chance to review this patch set > seriously, since I know most of them (especially Eric Dumazet) would > balk at the overhead you're proposing to add to our stack, just as I > did. > > This is the reality of the situation, and I'm sorry to tell you that > snippy retorts when someone does take the time out to review your work > won't help at all. I understand that and appreciate your time. I'll try to come up with something that addresses this problem in the next submission.
creator: Glauber Costa
date: 2011-10-13T20:23:33-00:00

Re: [PATCH v7 0/8] Request for inclusion: tcp memory buffers
type: item
description: David Miller <davem@davemloft.net> writes: > > Make this evaluate into exactly the same exact code stream we have > now when the memory cgroup feature is not in use, which will be the > majority of users. One possible way may be to guard it with static_branch() for no limit per cgroup set. That should be as near as practically possible to the original code. BTW the thing that usually worries me more is the cache line behaviour when the feature is in use. In the past some of the namespace patches have created some extremly hot global cache lines, that hurt on larger systems (for example the Unix socket regression from uid namespaces that is still not completly fixed). It would be good to double check that all important state is distributed properly. -Andi -- ak@linux.intel.com -- Speaking for myself only
creator: Andi Kleen
date: 2011-10-14T02:12:29-00:00

Re: [PATCH v7 0/8] Request for inclusion: tcp memory buffers
type: item
description: On Fri, 14 Oct 2011 00:05:58 +0400, Glauber Costa said: > On 10/14/2011 12:00 AM, David Miller wrote: > > Make this evaluate into exactly the same exact code stream we have > > now when the memory cgroup feature is not in use, which will be the > > majority of users. > > What exactly do you mean by "not in use" ? Not compiled in or not > actively being exercised ? If you mean the later, I appreciate tips on > how to achieve it. > > Also, I kind of dispute the affirmation that !cgroup will encompass > the majority of users, since cgroups is being enabled by default by > most vendors. All systemd based systems use it extensively, for instance. Yes, systemd requires a kernel that includes cgroups. However, systemd does *not* require the memory cgroup feature. As a practical matter, if your patch doesn't generate equivalent code for the "have cgroups, but no memory cgroup" situation, it's a non-starter.
creator: Valdis.Kletnieks
date: 2011-10-14T02:55:59-00:00

Re: [PATCH v7 0/8] Request for inclusion: tcp memory buffers
type: item
description: On 10/14/2011 12:18 AM, David Miller wrote: > From: Glauber Costa<glommer@parallels.com> > Date: Fri, 14 Oct 2011 00:14:40 +0400 > >> Are you happy, or at least willing to accept, an approach that keep >> things as they were with cgroups *compiled out*, or were you referring >> to not in use == compiled in, but with no users? > > To me these are the same exact thing, because %99 of users will be running > a kernel with every feature turned on in the Kconfig. Ok. Please let me know what do you think of the following patch. It is still crude, and applies on top of what I had, for simplicity. I can of course rework all the series for the next submission. The main idea is: We basically don't care about accounting if all tasks are in the same, root, cgroup. So I am using static_branch to enable this code path when the first !root cgroup is created - a stronger statement than just enabled/disabled by a runtime option. This should cover every single user that is not *actively* using cgroups (I understand that most distros will not only compile it in, but also leave it enabled...). When this code path is disabled, we should be doing the same as before. If you think this approach is valid, I am not left with the problem of how to replace the fields when cgroups are enabled. But for that I guess I can just copy the struct proto (probably only 2, or at most 3 protos will need it anyway), and make the new sockets run on this new structure. Cheers diff --git a/include/net/sock.h b/include/net/sock.h index efd7664..2270e50 100644 --- a/include/net/sock.h +++ b/include/net/sock.h @@ -807,19 +807,35 @@ struct proto { * Add a value in pages to the current memory allocation, * and return the current value. */ - long (*mem_allocated_add)(struct mem_cgroup *memcg, - long val, int *parent_status); - /* Pointer to the current number of sockets in this cgroup. */ - struct percpu_counter *(*sockets_allocated)(const struct mem_cgroup *memcg); + union { + long (*mem_allocated_add)(struct mem…
creator: Glauber Costa
date: 2011-10-14T12:56:23-00:00

Re: [PATCH v7 0/8] Request for inclusion: tcp memory buffers
type: item
description: From: Glauber Costa <glommer@parallels.com> Date: Fri, 14 Oct 2011 16:56:23 +0400 > Please let me know what do you think of the following patch. It is > still crude, and applies on top of what I had, for simplicity. I can > of course rework all the series for the next submission. The main idea > is: Thanks, I promise to look at this soon.
creator: davem
date: 2011-10-19T21:09:16-00:00