Patchwork [v2,1/4] tcp: Fix truesize accounting in tcp_try_coalesce

login
register
mail settings
Submitter Alexander Duyck
Date May 3, 2012, 7:18 a.m.
Message ID <20120503071859.13636.30050.stgit@gitlad.jf.intel.com>
Download mbox | patch
Permalink /patch/156619/
State Accepted
Delegated to: David Miller
Headers show

Comments

Alexander Duyck - May 3, 2012, 7:18 a.m.
This patch addresses several issues in the way we were tracking the
truesize in tcp_try_coalesce.

First it was using ksize which prevents us from having a 0 sized head frag
and getting a usable result.  To resolve that this patch uses the end
pointer which is set based off either ksize, or the frag_size supplied in
build_skb.  This allows us to compute the original truesize of the entire
buffer and remove that value leaving us with just what was added as pages.

The second issue was the use of skb->len if there is a mergeable head frag.
We should only need to remove the size of an data aligned sk_buff from our
current skb->truesize to compute the delta for a buffer with a reused head.
By using skb->len the value of truesize was being artificially reduced
which means that head frags could use more memory than buffers using
standard allocations.

Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---

 net/ipv4/tcp_input.c |   10 ++++------
 1 files changed, 4 insertions(+), 6 deletions(-)


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Eric Dumazet - May 3, 2012, 7:48 a.m.
On Thu, 2012-05-03 at 00:18 -0700, Alexander Duyck wrote:
> This patch addresses several issues in the way we were tracking the
> truesize in tcp_try_coalesce.
> 
> First it was using ksize which prevents us from having a 0 sized head frag
> and getting a usable result.  To resolve that this patch uses the end
> pointer which is set based off either ksize, or the frag_size supplied in
> build_skb.  This allows us to compute the original truesize of the entire
> buffer and remove that value leaving us with just what was added as pages.
> 
> The second issue was the use of skb->len if there is a mergeable head frag.
> We should only need to remove the size of an data aligned sk_buff from our
> current skb->truesize to compute the delta for a buffer with a reused head.
> By using skb->len the value of truesize was being artificially reduced
> which means that head frags could use more memory than buffers using
> standard allocations.
> 
> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
> Cc: Eric Dumazet <edumazet@google.com>
> Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> ---
> 
>  net/ipv4/tcp_input.c |   10 ++++------
>  1 files changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> index c6f78e2..3cb273a 100644
> --- a/net/ipv4/tcp_input.c
> +++ b/net/ipv4/tcp_input.c
> @@ -4565,12 +4565,10 @@ merge:
>  	if (skb_headlen(from) == 0 &&
>  	    (skb_shinfo(to)->nr_frags +
>  	     skb_shinfo(from)->nr_frags <= MAX_SKB_FRAGS)) {
> -		WARN_ON_ONCE(from->head_frag);
> -		delta = from->truesize - ksize(from->head) -
> -			SKB_DATA_ALIGN(sizeof(struct sk_buff));
> -
> -		WARN_ON_ONCE(delta < len);
> +		delta = from->truesize -
> +			SKB_TRUESIZE(skb_end_pointer(from) - from->head);
>  copyfrags:
> +		WARN_ON_ONCE(delta < len);
>  		memcpy(skb_shinfo(to)->frags + skb_shinfo(to)->nr_frags,
>  		       skb_shinfo(from)->frags,
>  		       skb_shinfo(from)->nr_frags * sizeof(skb_frag_t));
> @@ -4600,7 +4598,7 @@ copyfrags:
>  		skb_fill_page_desc(to, skb_shinfo(to)->nr_frags,
>  				   page, offset, skb_headlen(from));
>  		*fragstolen = true;
> -		delta = len; /* we dont know real truesize... */
> +		delta = from->truesize - SKB_DATA_ALIGN(sizeof(struct sk_buff));
>  		goto copyfrags;
>  	}
>  	return false;
> 
> --

Acked-by: Eric Dumazet <edumazet@google.com>



--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller - May 3, 2012, 8:21 a.m.
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 03 May 2012 09:48:54 +0200

> On Thu, 2012-05-03 at 00:18 -0700, Alexander Duyck wrote:
>> This patch addresses several issues in the way we were tracking the
>> truesize in tcp_try_coalesce.
>> 
>> First it was using ksize which prevents us from having a 0 sized head frag
>> and getting a usable result.  To resolve that this patch uses the end
>> pointer which is set based off either ksize, or the frag_size supplied in
>> build_skb.  This allows us to compute the original truesize of the entire
>> buffer and remove that value leaving us with just what was added as pages.
>> 
>> The second issue was the use of skb->len if there is a mergeable head frag.
>> We should only need to remove the size of an data aligned sk_buff from our
>> current skb->truesize to compute the delta for a buffer with a reused head.
>> By using skb->len the value of truesize was being artificially reduced
>> which means that head frags could use more memory than buffers using
>> standard allocations.
>> 
>> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
 ...
> Acked-by: Eric Dumazet <edumazet@google.com>

Applied.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index c6f78e2..3cb273a 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -4565,12 +4565,10 @@  merge:
 	if (skb_headlen(from) == 0 &&
 	    (skb_shinfo(to)->nr_frags +
 	     skb_shinfo(from)->nr_frags <= MAX_SKB_FRAGS)) {
-		WARN_ON_ONCE(from->head_frag);
-		delta = from->truesize - ksize(from->head) -
-			SKB_DATA_ALIGN(sizeof(struct sk_buff));
-
-		WARN_ON_ONCE(delta < len);
+		delta = from->truesize -
+			SKB_TRUESIZE(skb_end_pointer(from) - from->head);
 copyfrags:
+		WARN_ON_ONCE(delta < len);
 		memcpy(skb_shinfo(to)->frags + skb_shinfo(to)->nr_frags,
 		       skb_shinfo(from)->frags,
 		       skb_shinfo(from)->nr_frags * sizeof(skb_frag_t));
@@ -4600,7 +4598,7 @@  copyfrags:
 		skb_fill_page_desc(to, skb_shinfo(to)->nr_frags,
 				   page, offset, skb_headlen(from));
 		*fragstolen = true;
-		delta = len; /* we dont know real truesize... */
+		delta = from->truesize - SKB_DATA_ALIGN(sizeof(struct sk_buff));
 		goto copyfrags;
 	}
 	return false;