Re: [PATCH v2 3/4] sock: Consider memcg pressure when raising sockmem
From: Shakeel Butt <hidden>
Date: 2023-05-25 01:23:11
Also in:
lkml
On Mon, May 22, 2023 at 03:01:21PM +0800, Abel Wu wrote:
For now __sk_mem_raise_allocated() mainly considers global socket memory pressure and allows to raise if no global pressure observed, including the sockets whose memcgs are in pressure, which might result in longer memcg memstall. So take net-memcg's pressure into consideration when allocating socket memory to alleviate long tail latencies. Signed-off-by: Abel Wu <redacted>
Hi Abel, Have you seen any real world production issue which is fixed by this patch or is it more of a fix after reading code? This code is quite subtle and small changes can cause unintended behavior changes. At the moment the tcp memory accounting and memcg accounting is intermingled and I think we should decouple them.
quoted hunk ↗ jump to hunk
--- net/core/sock.c | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-)diff --git a/net/core/sock.c b/net/core/sock.c index 801df091e37a..7641d64293af 100644 --- a/net/core/sock.c +++ b/net/core/sock.c@@ -2977,21 +2977,30 @@ int __sk_mem_raise_allocated(struct sock *sk, int size, int amt, int kind) { bool memcg_charge = mem_cgroup_sockets_enabled && sk->sk_memcg; struct proto *prot = sk->sk_prot; - bool charged = true; + bool charged = true, pressured = false; long allocated; sk_memory_allocated_add(sk, amt); allocated = sk_memory_allocated(sk); - if (memcg_charge && - !(charged = mem_cgroup_charge_skmem(sk->sk_memcg, amt, - gfp_memcg_charge()))) - goto suppress_allocation; + + if (memcg_charge) { + charged = mem_cgroup_charge_skmem(sk->sk_memcg, amt, + gfp_memcg_charge()); + if (!charged) + goto suppress_allocation; + if (mem_cgroup_under_socket_pressure(sk->sk_memcg))
The memcg under pressure callback does a upward memcg tree walk, do please make sure you have tested the performance impact of this.
+ pressured = true;
+ }
/* Under limit. */
- if (allocated <= sk_prot_mem_limits(sk, 0)) {
+ if (allocated <= sk_prot_mem_limits(sk, 0))
sk_leave_memory_pressure(sk);
+ else
+ pressured = true;
+
+ /* No pressure observed in global/memcg. */
+ if (!pressured)
return 1;
- }
/* Under pressure. */
if (allocated > sk_prot_mem_limits(sk, 1))
--
2.37.3