From: Glauber Costa <glommer@parallels•com>
To: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp•fujitsu.com>
Cc: <netdev@vger•kernel.org>, David Miller <davem@davemloft•net>,
Andrew Morton <akpm@linux-foundation•org>
Subject: Re: [PATCH] memcg/tcp: fix warning caused b res->usage go to negative.
Date: Fri, 6 Apr 2012 19:49:37 +0400 [thread overview]
Message-ID: <4F7F1091.9040204@parallels.com> (raw)
In-Reply-To: <4F750FE8.2030800@jp.fujitsu.com>
[-- Attachment #1: Type: text/plain, Size: 1297 bytes --]
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<kamezawa.hiroyu@jp•fujitsu.com>
> 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.
[-- Attachment #2: 0001-decrement-static-keys-on-real-destroy-time.patch --]
[-- Type: text/x-patch, Size: 3892 bytes --]
>From c40bbd69cbb655b6389c2398ce89abb06e64910d Mon Sep 17 00:00:00 2001
From: Glauber Costa <glommer@parallels•com>
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 <glommer@parallels•com>
---
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
next prev parent reply other threads:[~2012-04-06 15:51 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-03-29 7:01 [BUGFIX][PATCH 0/3] memcg: tcp memcontrol fixes KAMEZAWA Hiroyuki
2012-03-29 7:03 ` [PATCH 1/3] [BUGFIX] memcg/tcp : fix to see use_hierarchy in tcp memcontrol cgroup KAMEZAWA Hiroyuki
2012-03-29 9:14 ` Glauber Costa
2012-03-29 9:16 ` KAMEZAWA Hiroyuki
2012-03-29 7:07 ` [BUGFIX][PATCH 2/3] memcg/tcp: remove static_branch_slow_dec() at changing limit KAMEZAWA Hiroyuki
2012-03-29 10:58 ` Glauber Costa
2012-03-29 23:51 ` KAMEZAWA Hiroyuki
2012-03-30 6:18 ` Glauber Costa
2012-03-29 7:10 ` [BUGFIX][PATCH 3/3] memcg/tcp: ignore tcp usage before accounting started KAMEZAWA Hiroyuki
2012-03-29 9:21 ` Glauber Costa
2012-03-30 1:44 ` [PATCH] memcg/tcp: fix warning caused b res->usage go to negative KAMEZAWA Hiroyuki
2012-04-06 15:49 ` Glauber Costa [this message]
2012-04-10 2:37 ` KAMEZAWA Hiroyuki
2012-04-10 2:51 ` Glauber Costa
2012-04-10 3:01 ` Glauber Costa
2012-04-10 4:15 ` KAMEZAWA Hiroyuki
2012-04-11 2:22 ` Glauber Costa
2012-04-10 3:21 ` KAMEZAWA Hiroyuki
2012-04-13 17:33 ` Glauber Costa
2012-04-18 8:02 ` KAMEZAWA Hiroyuki
2012-04-18 16:32 ` Glauber Costa
2012-04-02 3:41 ` [BUGFIX][PATCH 3/3] memcg/tcp: ignore tcp usage before accounting started David Miller
2012-04-03 22:31 ` Glauber Costa
2012-04-09 0:58 ` KAMEZAWA Hiroyuki
2012-04-09 1:44 ` Glauber Costa
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4F7F1091.9040204@parallels.com \
--to=glommer@parallels$(echo .)com \
--cc=akpm@linux-foundation$(echo .)org \
--cc=davem@davemloft$(echo .)net \
--cc=kamezawa.hiroyu@jp$(echo .)fujitsu.com \
--cc=netdev@vger$(echo .)kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox