Currently the memcg's status is also accounted into the socket's
memory pressure to alleviate the memcg's memstall. But there are
still cases that can be improved. Please check the patches for
detailed info.
v4:
- Per Shakeel's suggestion, removed the patch that suppresses
allocation under net-memcg pressure to avoid further keeping
the senders waiting if SACKed segments get dropped from the
OFO queue.
v3:
- Fixed some coding style issues pointed out by Simon
- Fold dependency into memcg pressure func to improve readability
v2:
- Splited into several patches and modified commit log for
better readability.
- Make memcg's pressure consideration function-wide in
__sk_mem_raise_allocated().
v1: https://lore.kernel.org/lkml/[email protected]/
v2: https://lore.kernel.org/lkml/[email protected]/
v3: https://lore.kernel.org/lkml/[email protected]/
Abel Wu (4):
net-memcg: Fold dependency into memcg pressure cond
sock: Always take memcg pressure into consideration
sock: Fix misuse of sk_under_memory_pressure()
sock: Remove redundant cond of memcg pressure
include/linux/memcontrol.h | 2 ++
include/net/sock.h | 14 ++++++++------
include/net/tcp.h | 3 +--
net/core/sock.c | 10 ++++++++--
4 files changed, 19 insertions(+), 10 deletions(-)
--
2.37.3
The sk_under_memory_pressure() is called to check whether there is
memory pressure related to this socket. But now it ignores the net-
memcg's pressure if the proto of the socket doesn't care about the
global pressure, which may put burden on its memcg compaction or
reclaim path (also remember that socket memory is un-reclaimable).
So always check the memcg's vm status to alleviate memstalls when
it's in pressure.
Signed-off-by: Abel Wu <[email protected]>
---
include/net/sock.h | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/include/net/sock.h b/include/net/sock.h
index 641c9373b44b..b0e5533e5909 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -1411,13 +1411,11 @@ static inline bool sk_has_memory_pressure(const struct sock *sk)
static inline bool sk_under_memory_pressure(const struct sock *sk)
{
- if (!sk->sk_prot->memory_pressure)
- return false;
-
if (mem_cgroup_under_socket_pressure(sk->sk_memcg))
return true;
- return !!*sk->sk_prot->memory_pressure;
+ return sk->sk_prot->memory_pressure &&
+ *sk->sk_prot->memory_pressure;
}
static inline long
--
2.37.3
The status of global socket memory pressure is updated when:
a) __sk_mem_raise_allocated():
enter: sk_memory_allocated(sk) > sysctl_mem[1]
leave: sk_memory_allocated(sk) <= sysctl_mem[0]
b) __sk_mem_reduce_allocated():
leave: sk_under_memory_pressure(sk) &&
sk_memory_allocated(sk) < sysctl_mem[0]
So the conditions of leaving global pressure are inconstant, which
may lead to the situation that one pressured net-memcg prevents the
global pressure from being cleared when there is indeed no global
pressure, thus the global constrains are still in effect unexpectedly
on the other sockets.
This patch fixes this by ignoring the net-memcg's pressure when
deciding whether should leave global memory pressure.
Fixes: e1aab161e013 ("socket: initial cgroup code.")
Signed-off-by: Abel Wu <[email protected]>
---
include/net/sock.h | 9 +++++++--
net/core/sock.c | 2 +-
2 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/include/net/sock.h b/include/net/sock.h
index b0e5533e5909..257706710be5 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -1409,13 +1409,18 @@ static inline bool sk_has_memory_pressure(const struct sock *sk)
return sk->sk_prot->memory_pressure != NULL;
}
+static inline bool sk_under_global_memory_pressure(const struct sock *sk)
+{
+ return sk->sk_prot->memory_pressure &&
+ *sk->sk_prot->memory_pressure;
+}
+
static inline bool sk_under_memory_pressure(const struct sock *sk)
{
if (mem_cgroup_under_socket_pressure(sk->sk_memcg))
return true;
- return sk->sk_prot->memory_pressure &&
- *sk->sk_prot->memory_pressure;
+ return sk_under_global_memory_pressure(sk);
}
static inline long
diff --git a/net/core/sock.c b/net/core/sock.c
index 5440e67bcfe3..801df091e37a 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -3095,7 +3095,7 @@ void __sk_mem_reduce_allocated(struct sock *sk, int amount)
if (mem_cgroup_sockets_enabled && sk->sk_memcg)
mem_cgroup_uncharge_skmem(sk->sk_memcg, amount);
- if (sk_under_memory_pressure(sk) &&
+ if (sk_under_global_memory_pressure(sk) &&
(sk_memory_allocated(sk) < sk_prot_mem_limits(sk, 0)))
sk_leave_memory_pressure(sk);
}
--
2.37.3
Now with the previous patch, __sk_mem_raise_allocated() considers
the memory pressure of both global and the socket's memcg on a func-
wide level, making the condition of memcg's pressure in question
redundant.
Signed-off-by: Abel Wu <[email protected]>
---
net/core/sock.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/net/core/sock.c b/net/core/sock.c
index 801df091e37a..86735ad2f903 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -3020,9 +3020,15 @@ int __sk_mem_raise_allocated(struct sock *sk, int size, int amt, int kind)
if (sk_has_memory_pressure(sk)) {
u64 alloc;
- if (!sk_under_memory_pressure(sk))
+ if (!sk_under_global_memory_pressure(sk))
return 1;
+
alloc = sk_sockets_allocated_read_positive(sk);
+
+ /* If under global pressure, allow the sockets that are below
+ * average memory usage to raise, trying to be fair among all
+ * the sockets under global constrains.
+ */
if (sk_prot_mem_limits(sk, 2) > alloc *
sk_mem_pages(sk->sk_wmem_queued +
atomic_read(&sk->sk_rmem_alloc) +
--
2.37.3
The callers of mem_cgroup_under_socket_pressure() should always make
sure that (mem_cgroup_sockets_enabled && sk->sk_memcg) is true. So
instead of coding around all the callsites, put the dependencies into
mem_cgroup_under_socket_pressure() to avoid redundancy and possibly
bugs.
This change might also introduce slight function call overhead *iff*
the function gets expanded in the future. But for now this change
doesn't make binaries different (checked by vimdiff) except the one
net/ipv4/tcp_input.o (by scripts/bloat-o-meter), which is probably
negligible to performance:
add/remove: 0/0 grow/shrink: 1/2 up/down: 5/-5 (0)
Function old new delta
tcp_grow_window 573 578 +5
tcp_try_rmem_schedule 1083 1081 -2
tcp_check_space 324 321 -3
Total: Before=44647, After=44647, chg +0.00%
So folding the dependencies into mem_cgroup_under_socket_pressure()
is generally a good thing and provides better readablility.
Signed-off-by: Abel Wu <[email protected]>
---
include/linux/memcontrol.h | 2 ++
include/net/sock.h | 3 +--
include/net/tcp.h | 3 +--
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
index 222d7370134c..a1aead140ff8 100644
--- a/include/linux/memcontrol.h
+++ b/include/linux/memcontrol.h
@@ -1743,6 +1743,8 @@ void mem_cgroup_sk_alloc(struct sock *sk);
void mem_cgroup_sk_free(struct sock *sk);
static inline bool mem_cgroup_under_socket_pressure(struct mem_cgroup *memcg)
{
+ if (!mem_cgroup_sockets_enabled || !memcg)
+ return false;
if (!cgroup_subsys_on_dfl(memory_cgrp_subsys) && memcg->tcpmem_pressure)
return true;
do {
diff --git a/include/net/sock.h b/include/net/sock.h
index 8b7ed7167243..641c9373b44b 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -1414,8 +1414,7 @@ static inline bool sk_under_memory_pressure(const struct sock *sk)
if (!sk->sk_prot->memory_pressure)
return false;
- if (mem_cgroup_sockets_enabled && sk->sk_memcg &&
- mem_cgroup_under_socket_pressure(sk->sk_memcg))
+ if (mem_cgroup_under_socket_pressure(sk->sk_memcg))
return true;
return !!*sk->sk_prot->memory_pressure;
diff --git a/include/net/tcp.h b/include/net/tcp.h
index 04a31643cda3..3c5e3718b454 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -261,8 +261,7 @@ extern unsigned long tcp_memory_pressure;
/* optimized version of sk_under_memory_pressure() for TCP sockets */
static inline bool tcp_under_memory_pressure(const struct sock *sk)
{
- if (mem_cgroup_sockets_enabled && sk->sk_memcg &&
- mem_cgroup_under_socket_pressure(sk->sk_memcg))
+ if (mem_cgroup_under_socket_pressure(sk->sk_memcg))
return true;
return READ_ONCE(tcp_memory_pressure);
--
2.37.3
On Tue, 2023-05-30 at 19:40 +0800, Abel Wu wrote:
> Now with the previous patch, __sk_mem_raise_allocated() considers
> the memory pressure of both global and the socket's memcg on a func-
> wide level,
Since the "previous patch" (aka "sock: Consider memcg pressure when
raising sockmem") has been dropped in this series revision, I guess
this patch should be dropped, too?
Is this targeting the 'net-next' tree or the 'net' one? please specify
the target tree into the subj line. I think we could consider net-next
for this series, given the IMHO not trivial implications.
Cheers,
Paolo
On 6/1/23 5:10 PM, Paolo Abeni wrote:
> On Tue, 2023-05-30 at 19:40 +0800, Abel Wu wrote:
>> Now with the previous patch, __sk_mem_raise_allocated() considers
>> the memory pressure of both global and the socket's memcg on a func-
>> wide level,
>
> Since the "previous patch" (aka "sock: Consider memcg pressure when
> raising sockmem") has been dropped in this series revision, I guess
> this patch should be dropped, too?
Yes indeed, these two patches should go together. Sorry for my
carelessness.
>
> Is this targeting the 'net-next' tree or the 'net' one? please specify
> the target tree into the subj line. I think we could consider net-next
> for this series, given the IMHO not trivial implications.
Got it, I will resend this series based on net-next.
Thanks!
Abel