Skip to content

Commit

Permalink
net: fix sock_clone reference mismatch with tcp memcontrol
Browse files Browse the repository at this point in the history
Sockets can also be created through sock_clone. Because it copies
all data in the sock structure, it also copies the memcg-related pointer,
and all should be fine. However, since we now use reference counts in
socket creation, we are left with some sockets that have no reference
counts. It matters when we destroy them, since it leads to a mismatch.

Signed-off-by: Glauber Costa <glommer@parallels.com>
CC: David S. Miller <davem@davemloft.net>
CC: Greg Thelen <gthelen@google.com>
CC: Hiroyouki Kamezawa <kamezawa.hiroyu@jp.fujitsu.com>
CC: Laurent Chavey <chavey@google.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
Glauber Costa authored and davem330 committed Jan 7, 2012
1 parent 356b954 commit f3f511e
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 5 deletions.
6 changes: 6 additions & 0 deletions include/net/sock.h
Original file line number Diff line number Diff line change
Expand Up @@ -1103,6 +1103,12 @@ sk_sockets_allocated_read_positive(struct sock *sk)
return percpu_counter_sum_positive(prot->sockets_allocated);
}

static inline void sk_update_clone(const struct sock *sk, struct sock *newsk)
{
if (mem_cgroup_sockets_enabled && sk->sk_cgrp)
sock_update_memcg(newsk);
}

static inline int
proto_sockets_allocated_sum_positive(struct proto *prot)
{
Expand Down
19 changes: 14 additions & 5 deletions mm/memcontrol.c
Original file line number Diff line number Diff line change
Expand Up @@ -381,16 +381,25 @@ static void mem_cgroup_put(struct mem_cgroup *memcg);
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);

/* Socket cloning can throw us here with sk_cgrp already
* filled. It won't however, necessarily happen from
* process context. So the test for root memcg given
* the current task's memcg won't help us in this case.
*
* Respecting the original socket's memcg is a better
* decision in this case.
*/
if (sk->sk_cgrp) {
BUG_ON(mem_cgroup_is_root(sk->sk_cgrp->memcg));
mem_cgroup_get(sk->sk_cgrp->memcg);
return;
}

rcu_read_lock();
memcg = mem_cgroup_from_task(current);
if (!mem_cgroup_is_root(memcg)) {
Expand Down
2 changes: 2 additions & 0 deletions net/core/sock.c
Original file line number Diff line number Diff line change
Expand Up @@ -1362,6 +1362,8 @@ struct sock *sk_clone_lock(const struct sock *sk, const gfp_t priority)
sk_set_socket(newsk, NULL);
newsk->sk_wq = NULL;

sk_update_clone(sk, newsk);

if (newsk->sk_prot->sockets_allocated)
sk_sockets_allocated_inc(newsk);

Expand Down

0 comments on commit f3f511e

Please sign in to comment.