Skip to content

Commit

Permalink
net: introduce res_counter_charge_nofail() for socket allocations
Browse files Browse the repository at this point in the history
There is a case in __sk_mem_schedule(), where an allocation
is beyond the maximum, but yet we are allowed to proceed.
It happens under the following condition:

	sk->sk_wmem_queued + size >= sk->sk_sndbuf

The network code won't revert the allocation in this case,
meaning that at some point later it'll try to do it. Since
this is never communicated to the underlying res_counter
code, there is an inbalance in res_counter uncharge operation.

I see two ways of fixing this:

1) storing the information about those allocations somewhere
   in memcg, and then deducting from that first, before
   we start draining the res_counter,
2) providing a slightly different allocation function for
   the res_counter, that matches the original behavior of
   the network code more closely.

I decided to go for #2 here, believing it to be more elegant,
since #1 would require us to do basically that, but in a more
obscure way.

Signed-off-by: Glauber Costa <glommer@parallels.com>
Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Cc: Johannes Weiner <hannes@cmpxchg.org>
Cc: Michal Hocko <mhocko@suse.cz>
CC: Tejun Heo <tj@kernel.org>
CC: Li Zefan <lizf@cn.fujitsu.com>
CC: Laurent Chavey <chavey@google.com>
Acked-by: Tejun Heo <tj@kernel.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
Glauber Costa authored and davem330 committed Jan 22, 2012
1 parent 8cfd14a commit 0e90b31
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 8 deletions.
6 changes: 6 additions & 0 deletions include/linux/res_counter.h
Original file line number Diff line number Diff line change
Expand Up @@ -109,12 +109,18 @@ void res_counter_init(struct res_counter *counter, struct res_counter *parent);
*
* returns 0 on success and <0 if the counter->usage will exceed the
* counter->limit _locked call expects the counter->lock to be taken
*
* charge_nofail works the same, except that it charges the resource
* counter unconditionally, and returns < 0 if the after the current
* charge we are over limit.
*/

int __must_check res_counter_charge_locked(struct res_counter *counter,
unsigned long val);
int __must_check res_counter_charge(struct res_counter *counter,
unsigned long val, struct res_counter **limit_fail_at);
int __must_check res_counter_charge_nofail(struct res_counter *counter,
unsigned long val, struct res_counter **limit_fail_at);

/*
* uncharge - tell that some portion of the resource is released
Expand Down
10 changes: 4 additions & 6 deletions include/net/sock.h
Original file line number Diff line number Diff line change
Expand Up @@ -1008,9 +1008,8 @@ static inline void memcg_memory_allocated_add(struct cg_proto *prot,
struct res_counter *fail;
int ret;

ret = res_counter_charge(prot->memory_allocated,
amt << PAGE_SHIFT, &fail);

ret = res_counter_charge_nofail(prot->memory_allocated,
amt << PAGE_SHIFT, &fail);
if (ret < 0)
*parent_status = OVER_LIMIT;
}
Expand Down Expand Up @@ -1054,12 +1053,11 @@ sk_memory_allocated_add(struct sock *sk, int amt, int *parent_status)
}

static inline void
sk_memory_allocated_sub(struct sock *sk, int amt, int parent_status)
sk_memory_allocated_sub(struct sock *sk, int amt)
{
struct proto *prot = sk->sk_prot;

if (mem_cgroup_sockets_enabled && sk->sk_cgrp &&
parent_status != OVER_LIMIT) /* Otherwise was uncharged already */
if (mem_cgroup_sockets_enabled && sk->sk_cgrp)
memcg_memory_allocated_sub(sk->sk_cgrp, amt);

atomic_long_sub(amt, prot->memory_allocated);
Expand Down
25 changes: 25 additions & 0 deletions kernel/res_counter.c
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,31 @@ int res_counter_charge(struct res_counter *counter, unsigned long val,
return ret;
}

int res_counter_charge_nofail(struct res_counter *counter, unsigned long val,
struct res_counter **limit_fail_at)
{
int ret, r;
unsigned long flags;
struct res_counter *c;

r = ret = 0;
*limit_fail_at = NULL;
local_irq_save(flags);
for (c = counter; c != NULL; c = c->parent) {
spin_lock(&c->lock);
r = res_counter_charge_locked(c, val);
if (r)
c->usage += val;
spin_unlock(&c->lock);
if (r < 0 && ret == 0) {
*limit_fail_at = c;
ret = r;
}
}
local_irq_restore(flags);

return ret;
}
void res_counter_uncharge_locked(struct res_counter *counter, unsigned long val)
{
if (WARN_ON(counter->usage < val))
Expand Down
4 changes: 2 additions & 2 deletions net/core/sock.c
Original file line number Diff line number Diff line change
Expand Up @@ -1827,7 +1827,7 @@ int __sk_mem_schedule(struct sock *sk, int size, int kind)
/* Alas. Undo changes. */
sk->sk_forward_alloc -= amt * SK_MEM_QUANTUM;

sk_memory_allocated_sub(sk, amt, parent_status);
sk_memory_allocated_sub(sk, amt);

return 0;
}
Expand All @@ -1840,7 +1840,7 @@ EXPORT_SYMBOL(__sk_mem_schedule);
void __sk_mem_reclaim(struct sock *sk)
{
sk_memory_allocated_sub(sk,
sk->sk_forward_alloc >> SK_MEM_QUANTUM_SHIFT, 0);
sk->sk_forward_alloc >> SK_MEM_QUANTUM_SHIFT);
sk->sk_forward_alloc &= SK_MEM_QUANTUM - 1;

if (sk_under_memory_pressure(sk) &&
Expand Down

0 comments on commit 0e90b31

Please sign in to comment.