Skip to contentSkip to conversation

tcp: fix mem under-charging with zerocopy sendmsg()

PATCH Applied Eric Dumazet 3 participants 3 messages Feb 1, 2022 – Feb 2, 2022

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 tcp: fix mem under-charging with zerocopy sendmsg()

From: Eric Dumazet <[email protected]>

We got reports of following warning in inet_sock_destruct()

	WARN_ON(sk_forward_alloc_get(sk));

Whenever we add a non zero-copy fragment to a pure zerocopy skb, we have to anticipate that whole skb->truesize will be uncharged when skb is finally freed.

skb->data_len is the payload length. But the memory truesize estimated by __zerocopy_sg_from_iter() is page aligned.

Fixes 9b65b17db723 ("net: avoid double accounting for pure zerocopy skbs")
Cc Talal Ahmad <[email protected]>
Cc Arjun Roy <[email protected]>
Cc Soheil Hassas Yeganeh <[email protected]>
Cc Willem de Bruijn <[email protected]>
---
 net/ipv4/tcp.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)
net/ipv4/tcp.c +5 -2
index 78e81465f5f3632f54093495d2f2a064e60c7237..bdf108f544a45a2aa24bc962fb81dfd0ca1e0682 100644
@@ -1322,10 +1322,13 @@ int tcp_sendmsg_locked(struct sock *sk, struct msghdr *msg, size_t size)
 
 			/* skb changing from pure zc to mixed, must charge zc */
 			if (unlikely(skb_zcopy_pure(skb))) {
-				if (!sk_wmem_schedule(sk, skb->data_len))
+				u32 extra = skb->truesize -
+					    SKB_TRUESIZE(skb_end_offset(skb));
+
+				if (!sk_wmem_schedule(sk, extra))
 					goto wait_for_space;
 
-				sk_mem_charge(sk, skb->data_len);
+				sk_mem_charge(sk, extra);
 				skb_shinfo(skb)->flags &= ~SKBFL_PURE_ZEROCOPY;
 			}
 
Signature · 1 line
2.35.0.rc2.247.g8bbb082509-goog
Acked-by on v1 1/1
#1 lore raw
Quoted 14 lines · Eric Dumazet

From: Eric Dumazet <[email protected]>

We got reports of following warning in inet_sock_destruct()

        WARN_ON(sk_forward_alloc_get(sk));

Whenever we add a non zero-copy fragment to a pure zerocopy skb, we have to anticipate that whole skb->truesize will be uncharged when skb is finally freed.

skb->data_len is the payload length. But the memory truesize estimated by __zerocopy_sg_from_iter() is page aligned.

Quoted 6 lines
Fixes 9b65b17db723 ("net: avoid double accounting for pure zerocopy skbs")
Cc Talal Ahmad <[email protected]>
Cc Arjun Roy <[email protected]>
Cc Soheil Hassas Yeganeh <[email protected]>
Cc Willem de Bruijn <[email protected]>

Nice catch! and thank you for the fix!

Quoted 27 lines
---
 net/ipv4/tcp.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)
net/ipv4/tcp.c +5 -3
index 78e81465f5f3632f54093495d2f2a064e60c7237..bdf108f544a45a2aa24bc962fb81dfd0ca1e0682 100644
@@ -1322,10 +1322,13 @@ int tcp_sendmsg_locked(struct sock *sk, struct msghdr *msg, size_t size)
 
                        /* skb changing from pure zc to mixed, must charge zc */
                        if (unlikely(skb_zcopy_pure(skb))) {
-                               if (!sk_wmem_schedule(sk, skb->data_len))
+                               u32 extra = skb->truesize -
+                                           SKB_TRUESIZE(skb_end_offset(skb));
+
+                               if (!sk_wmem_schedule(sk, extra))
                                        goto wait_for_space;
 
-                               sk_mem_charge(sk, skb->data_len);
+                               sk_mem_charge(sk, extra);
                                skb_shinfo(skb)->flags &= ~SKBFL_PURE_ZEROCOPY;
                        }
 
--

2.35.0.rc2.247.g8bbb082509-goog

#2 lore raw
In reply to Eric Dumazet
This patch was applied to netdev/net.git (master) by Jakub Kicinski <[email protected]>:

Hello:

This patch was applied to netdev/net.git (master)
by Jakub Kicinski <[email protected]>:

Quoted 11 lines · On Mon, 31 Jan 2022 22:52:54 -0800 you wrote:

From: Eric Dumazet <[email protected]>

We got reports of following warning in inet_sock_destruct()

	WARN_ON(sk_forward_alloc_get(sk));

Whenever we add a non zero-copy fragment to a pure zerocopy skb, we have to anticipate that whole skb->truesize will be uncharged when skb is finally freed.

[...]

Here is the summary with links:
- [net] tcp: fix mem under-charging with zerocopy sendmsg()

    https://git.kernel.org/netdev/net/c/479f5547239d

You are awesome, thank you!

Signature · 2 lines
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html