Skip to content

Commit 0e90b31

Browse files
Glauber Costadavem330
authored andcommitted
net: introduce res_counter_charge_nofail() for socket allocations
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 raspberrypi#2 here, believing it to be more elegant, since raspberrypi#1 would require us to do basically that, but in a more obscure way. Signed-off-by: Glauber Costa <[email protected]> Cc: KAMEZAWA Hiroyuki <[email protected]> Cc: Johannes Weiner <[email protected]> Cc: Michal Hocko <[email protected]> CC: Tejun Heo <[email protected]> CC: Li Zefan <[email protected]> CC: Laurent Chavey <[email protected]> Acked-by: Tejun Heo <[email protected]> Signed-off-by: David S. Miller <[email protected]>
1 parent 8cfd14a commit 0e90b31

File tree

4 files changed

+37
-8
lines changed

4 files changed

+37
-8
lines changed

include/linux/res_counter.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,12 +109,18 @@ void res_counter_init(struct res_counter *counter, struct res_counter *parent);
109109
*
110110
* returns 0 on success and <0 if the counter->usage will exceed the
111111
* counter->limit _locked call expects the counter->lock to be taken
112+
*
113+
* charge_nofail works the same, except that it charges the resource
114+
* counter unconditionally, and returns < 0 if the after the current
115+
* charge we are over limit.
112116
*/
113117

114118
int __must_check res_counter_charge_locked(struct res_counter *counter,
115119
unsigned long val);
116120
int __must_check res_counter_charge(struct res_counter *counter,
117121
unsigned long val, struct res_counter **limit_fail_at);
122+
int __must_check res_counter_charge_nofail(struct res_counter *counter,
123+
unsigned long val, struct res_counter **limit_fail_at);
118124

119125
/*
120126
* uncharge - tell that some portion of the resource is released

include/net/sock.h

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1008,9 +1008,8 @@ static inline void memcg_memory_allocated_add(struct cg_proto *prot,
10081008
struct res_counter *fail;
10091009
int ret;
10101010

1011-
ret = res_counter_charge(prot->memory_allocated,
1012-
amt << PAGE_SHIFT, &fail);
1013-
1011+
ret = res_counter_charge_nofail(prot->memory_allocated,
1012+
amt << PAGE_SHIFT, &fail);
10141013
if (ret < 0)
10151014
*parent_status = OVER_LIMIT;
10161015
}
@@ -1054,12 +1053,11 @@ sk_memory_allocated_add(struct sock *sk, int amt, int *parent_status)
10541053
}
10551054

10561055
static inline void
1057-
sk_memory_allocated_sub(struct sock *sk, int amt, int parent_status)
1056+
sk_memory_allocated_sub(struct sock *sk, int amt)
10581057
{
10591058
struct proto *prot = sk->sk_prot;
10601059

1061-
if (mem_cgroup_sockets_enabled && sk->sk_cgrp &&
1062-
parent_status != OVER_LIMIT) /* Otherwise was uncharged already */
1060+
if (mem_cgroup_sockets_enabled && sk->sk_cgrp)
10631061
memcg_memory_allocated_sub(sk->sk_cgrp, amt);
10641062

10651063
atomic_long_sub(amt, prot->memory_allocated);

kernel/res_counter.c

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,31 @@ int res_counter_charge(struct res_counter *counter, unsigned long val,
6666
return ret;
6767
}
6868

69+
int res_counter_charge_nofail(struct res_counter *counter, unsigned long val,
70+
struct res_counter **limit_fail_at)
71+
{
72+
int ret, r;
73+
unsigned long flags;
74+
struct res_counter *c;
75+
76+
r = ret = 0;
77+
*limit_fail_at = NULL;
78+
local_irq_save(flags);
79+
for (c = counter; c != NULL; c = c->parent) {
80+
spin_lock(&c->lock);
81+
r = res_counter_charge_locked(c, val);
82+
if (r)
83+
c->usage += val;
84+
spin_unlock(&c->lock);
85+
if (r < 0 && ret == 0) {
86+
*limit_fail_at = c;
87+
ret = r;
88+
}
89+
}
90+
local_irq_restore(flags);
91+
92+
return ret;
93+
}
6994
void res_counter_uncharge_locked(struct res_counter *counter, unsigned long val)
7095
{
7196
if (WARN_ON(counter->usage < val))

net/core/sock.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1827,7 +1827,7 @@ int __sk_mem_schedule(struct sock *sk, int size, int kind)
18271827
/* Alas. Undo changes. */
18281828
sk->sk_forward_alloc -= amt * SK_MEM_QUANTUM;
18291829

1830-
sk_memory_allocated_sub(sk, amt, parent_status);
1830+
sk_memory_allocated_sub(sk, amt);
18311831

18321832
return 0;
18331833
}
@@ -1840,7 +1840,7 @@ EXPORT_SYMBOL(__sk_mem_schedule);
18401840
void __sk_mem_reclaim(struct sock *sk)
18411841
{
18421842
sk_memory_allocated_sub(sk,
1843-
sk->sk_forward_alloc >> SK_MEM_QUANTUM_SHIFT, 0);
1843+
sk->sk_forward_alloc >> SK_MEM_QUANTUM_SHIFT);
18441844
sk->sk_forward_alloc &= SK_MEM_QUANTUM - 1;
18451845

18461846
if (sk_under_memory_pressure(sk) &&

0 commit comments

Comments
 (0)