From patchwork Fri Apr 6 15:49:37 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Glauber Costa X-Patchwork-Id: 151197 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 312EEB7086 for ; Sat, 7 Apr 2012 01:51:16 +1000 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756636Ab2DFPvN (ORCPT ); Fri, 6 Apr 2012 11:51:13 -0400 Received: from mx2.parallels.com ([64.131.90.16]:33346 "EHLO mx2.parallels.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754168Ab2DFPvM (ORCPT ); Fri, 6 Apr 2012 11:51:12 -0400 Received: from [96.31.168.206] (helo=mail.parallels.com) by mx2.parallels.com with esmtps (TLSv1:AES128-SHA:128) (Exim 4.77) (envelope-from ) id 1SGBRO-00044n-Cl; Fri, 06 Apr 2012 11:51:10 -0400 Received: from straightjacket.localdomain (195.166.51.3) by mail.parallels.com (10.255.249.32) with Microsoft SMTP Server (TLS) id 14.2.247.3; Fri, 6 Apr 2012 08:51:08 -0700 Message-ID: <4F7F1091.9040204@parallels.com> Date: Fri, 6 Apr 2012 19:49:37 +0400 From: Glauber Costa User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:10.0.1) Gecko/20120216 Thunderbird/10.0.1 MIME-Version: 1.0 To: KAMEZAWA Hiroyuki CC: , David Miller , Andrew Morton Subject: Re: [PATCH] memcg/tcp: fix warning caused b res->usage go to negative. References: <4F7408B7.9090706@jp.fujitsu.com> <4F740AEF.7090900@jp.fujitsu.com> <4F742983.1080402@parallels.com> <4F750FE8.2030800@jp.fujitsu.com> In-Reply-To: <4F750FE8.2030800@jp.fujitsu.com> X-Originating-IP: [195.166.51.3] Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On 03/30/2012 05:44 AM, KAMEZAWA Hiroyuki wrote: > Maybe what we can do before lsf/mm summit will be this (avoid warning.) > This patch is onto linus's git tree. Patch description is updated. > > Thanks. > -Kame > == > From 4ab80f84bbcb02a790342426c1de84aeb17fcbe9 Mon Sep 17 00:00:00 2001 > From: KAMEZAWA Hiroyuki > Date: Thu, 29 Mar 2012 14:59:04 +0900 > Subject: [PATCH] memcg/tcp: fix warning caused b res->usage go to negative. > > tcp memcontrol starts accouting after res->limit is set. So, if a sockets > starts before setting res->limit, there are already used resource. > At setting res->limit, accounting starts. The resource will be uncharged > and make res_counter below 0 because they are not charged. > This causes warning. > Kame, Please test the following patch and see if it fixes your problems (I tested locally, and it triggers me no warnings running the test script you provided + an inbound scp -r copy of an iso directory from a remote machine) When you are reviewing, keep in mind that we're likely to have the same problems with slab jump labels - since the slab pages will outlive the cgroup as well, and it might be worthy to keep this in mind, and provide a central point for the jump labels to be set of on cgroup destruction. From c40bbd69cbb655b6389c2398ce89abb06e64910d Mon Sep 17 00:00:00 2001 From: Glauber Costa Date: Wed, 4 Apr 2012 21:08:38 +0400 Subject: [PATCH] decrement static keys on real destroy time We call the destroy function when a cgroup starts to be removed, such as by a rmdir event. However, because of our reference counters, some objects are still inflight. Right now, we are decrementing the static_keys at destroy() time, meaning that if we get rid of the last static_key reference, some objects will still have charges, but the code to properly uncharge them won't be run. This becomes a problem specially if it is ever enabled again, because now new charges will be added to the staled charges making keeping it pretty much impossible. Signed-off-by: Glauber Costa --- include/net/tcp_memcontrol.h | 2 ++ mm/memcontrol.c | 15 +++++++++++++++ net/ipv4/tcp_memcontrol.c | 10 ++++------ 3 files changed, 21 insertions(+), 6 deletions(-) diff --git a/include/net/tcp_memcontrol.h b/include/net/tcp_memcontrol.h index 7df18bc..5a2b915 100644 --- a/include/net/tcp_memcontrol.h +++ b/include/net/tcp_memcontrol.h @@ -9,6 +9,8 @@ struct tcp_memcontrol { /* those two are read-mostly, leave them at the end */ long tcp_prot_mem[3]; int tcp_memory_pressure; + /* if this cgroup was ever limited, having static_keys activated */ + bool limited; }; struct cg_proto *tcp_proto_cgroup(struct mem_cgroup *memcg); diff --git a/mm/memcontrol.c b/mm/memcontrol.c index 64a1bcd..74b757b 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -442,6 +442,15 @@ void sock_release_memcg(struct sock *sk) } } +static void disarm_static_keys(struct mem_cgroup *memcg) +{ +#ifdef CONFIG_INET + if (memcg->tcp_mem.limited) + static_key_slow_dec(&memcg_socket_limit_enabled); +#endif +} + + #ifdef CONFIG_INET struct cg_proto *tcp_proto_cgroup(struct mem_cgroup *memcg) { @@ -452,6 +461,11 @@ struct cg_proto *tcp_proto_cgroup(struct mem_cgroup *memcg) } EXPORT_SYMBOL(tcp_proto_cgroup); #endif /* CONFIG_INET */ +#else +static inline void disarm_static_keys(struct mem_cgroup *memcg) +{ +} + #endif /* CONFIG_CGROUP_MEM_RES_CTLR_KMEM */ static void drain_all_stock_async(struct mem_cgroup *memcg); @@ -4883,6 +4897,7 @@ static void __mem_cgroup_put(struct mem_cgroup *memcg, int count) { if (atomic_sub_and_test(count, &memcg->refcnt)) { struct mem_cgroup *parent = parent_mem_cgroup(memcg); + disarm_static_keys(memcg); __mem_cgroup_free(memcg); if (parent) mem_cgroup_put(parent); diff --git a/net/ipv4/tcp_memcontrol.c b/net/ipv4/tcp_memcontrol.c index 1517037..93555ab 100644 --- a/net/ipv4/tcp_memcontrol.c +++ b/net/ipv4/tcp_memcontrol.c @@ -41,6 +41,7 @@ int tcp_init_cgroup(struct mem_cgroup *memcg, struct cgroup_subsys *ss) tcp->tcp_prot_mem[1] = net->ipv4.sysctl_tcp_mem[1]; tcp->tcp_prot_mem[2] = net->ipv4.sysctl_tcp_mem[2]; tcp->tcp_memory_pressure = 0; + tcp->limited = false; parent_cg = tcp_prot.proto_cgroup(parent); if (parent_cg) @@ -74,9 +75,6 @@ void tcp_destroy_cgroup(struct mem_cgroup *memcg) percpu_counter_destroy(&tcp->tcp_sockets_allocated); val = res_counter_read_u64(&tcp->tcp_memory_allocated, RES_LIMIT); - - if (val != RESOURCE_MAX) - static_key_slow_dec(&memcg_socket_limit_enabled); } EXPORT_SYMBOL(tcp_destroy_cgroup); @@ -107,10 +105,10 @@ static int tcp_update_limit(struct mem_cgroup *memcg, u64 val) tcp->tcp_prot_mem[i] = min_t(long, val >> PAGE_SHIFT, net->ipv4.sysctl_tcp_mem[i]); - if (val == RESOURCE_MAX && old_lim != RESOURCE_MAX) - static_key_slow_dec(&memcg_socket_limit_enabled); - else if (old_lim == RESOURCE_MAX && val != RESOURCE_MAX) + if (old_lim == RESOURCE_MAX && !tcp->limited) { static_key_slow_inc(&memcg_socket_limit_enabled); + tcp->limited = true; + } return 0; } -- 1.7.7.6