Re: 2.6.24-rc2: Network commit causes SLUB performance regression with tbench

!MAILaRCHIVE_VOTE_RePLACE
Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]
To: David Miller <davem@...>
Cc: <nickpiggin@...>, <clameter@...>, <netdev@...>, <linux-kernel@...>
Date: Wednesday, November 14, 2007 - 7:48 am

On Wed, Nov 14, 2007 at 03:10:22AM -0800, David Miller wrote:

Thanks for the pointer.  Indeed there is a bug in that area.
I'm not sure whether it's causing the problem at hand but it's
certainly suboptimal.

[TCP]: Fix size calculation in sk_stream_alloc_pskb

We round up the header size in sk_stream_alloc_pskb so that
TSO packets get zero tail room.  Unfortunately this rounding
up is not coordinated with the select_size() function used by
TCP to calculate the second parameter of sk_stream_alloc_pskb.

As a result, we may allocate more than a page of data in the
non-TSO case when exactly one page is desired.

In fact, rounding up the head room is detrimental in the non-TSO
case because it makes memory that would otherwise be available to
the payload head room.  TSO doesn't need this either, all it wants
is the guarantee that there is no tail room.

So this patch fixes this by adjusting the skb_reserve call so that
exactly the requested amount (which all callers have calculated in
a precise way) is made available as tail room.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
diff --git a/include/net/sock.h b/include/net/sock.h
index 5504fb9..567e468 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -1235,14 +1235,16 @@ static inline struct sk_buff *sk_stream_alloc_pskb(struct sock *sk,
 						   gfp_t gfp)
 {
 	struct sk_buff *skb;
-	int hdr_len;
 
-	hdr_len = SKB_DATA_ALIGN(sk->sk_prot->max_header);
-	skb = alloc_skb_fclone(size + hdr_len, gfp);
+	skb = alloc_skb_fclone(size + sk->sk_prot->max_header, gfp);
 	if (skb) {
 		skb->truesize += mem;
 		if (sk_stream_wmem_schedule(sk, skb->truesize)) {
-			skb_reserve(skb, hdr_len);
+			/*
+			 * Make sure that we have exactly size bytes
+			 * available to the caller, no more, no less.
+			 */
+			skb_reserve(skb, skb_tailroom(skb) - size);
 			return skb;
 		}
 		__kfree_skb(skb);
-
Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]

Messages in current thread:
2.6.24-rc2 slab vs slob tbench numbers, Nick Piggin, (Fri Nov 9, 8:36 am)
Re: 2.6.24-rc2 slab vs slob tbench numbers, Matt Mackall, (Mon Nov 12, 4:13 pm)
Re: 2.6.24-rc2 slab vs slob tbench numbers, Nick Piggin, (Tue Nov 13, 7:44 am)
Re: 2.6.24-rc2 slab vs slob tbench numbers, Christoph Lameter, (Fri Nov 9, 11:15 am)
Re: 2.6.24-rc2 slab vs slob tbench numbers, Christoph Lameter, (Fri Nov 9, 1:49 pm)
Re: 2.6.24-rc2: Network commit causes SLUB performance regre..., Christoph Lameter, (Mon Nov 12, 3:44 pm)
Re: 2.6.24-rc2: Network commit causes SLUB performance regre..., Herbert Xu, (Wed Nov 14, 7:48 am)
Re: 2.6.24-rc2: Network commit causes SLUB performance regre..., Christoph Lameter, (Wed Nov 14, 9:03 pm)
Re: 2.6.24-rc2: Network commit causes SLUB performance regre..., Christoph Lameter, (Wed Nov 14, 2:33 pm)