Netdev List
 help / color / mirror / Atom feed
From: Eric Dumazet <eric.dumazet@gmail.com>
To: David Miller <davem@davemloft.net>
Cc: netdev@vger.kernel.org
Subject: [PATCH net-next-2.6] net: pskb_expand_head() optimization
Date: Fri, 03 Sep 2010 11:09:32 +0200	[thread overview]
Message-ID: <1283504972.2453.257.camel@edumazet-laptop> (raw)
In-Reply-To: <1283492880.3699.1437.camel@edumazet-laptop>

Le vendredi 03 septembre 2010 à 07:48 +0200, Eric Dumazet a écrit :

> David, I had this same idea some days ago when reviewing this code,
> but when I came to conclusion we could not avoid the get_page /
> put_page() on skb_shinfo(skb)->frags[i].page. I thought it was not worth
> trying to avoid the frag_list grab/release operation.
> 

Here is the patch I cooked, for net-next-2.6

[PATCH net-next-2.6] net: pskb_expand_head() optimization

pskb_expand_head() blindly takes references on fragments before calling
skb_release_data(), potentially releasing these references.

We can add a fast path, avoiding these atomic operations, if we own the
last reference on skb->head.

Based on a previous patch from David

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
 net/core/skbuff.c |   25 ++++++++++++++++++++-----
 1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 231dff0..59b96fe 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -781,6 +781,7 @@ int pskb_expand_head(struct sk_buff *skb, int nhead, int ntail,
 	u8 *data;
 	int size = nhead + (skb_end_pointer(skb) - skb->head) + ntail;
 	long off;
+	bool fastpath;
 
 	BUG_ON(nhead < 0);
 
@@ -802,14 +803,28 @@ int pskb_expand_head(struct sk_buff *skb, int nhead, int ntail,
 	       skb_shinfo(skb),
 	       offsetof(struct skb_shared_info, frags[skb_shinfo(skb)->nr_frags]));
 
-	for (i = 0; i < skb_shinfo(skb)->nr_frags; i++)
-		get_page(skb_shinfo(skb)->frags[i].page);
+	/* Check if we can avoid taking references on fragments if we own
+	 * the last reference on skb->head. (see skb_release_data())
+	 */
+	if (!skb->cloned)
+		fastpath = true;
+	else {
+		int delta = skb->nohdr ? (1 << SKB_DATAREF_SHIFT) + 1 : 1;
 
-	if (skb_has_frag_list(skb))
-		skb_clone_fraglist(skb);
+		fastpath = atomic_read(&skb_shinfo(skb)->dataref) == delta;
+	}
 
-	skb_release_data(skb);
+	if (fastpath) {
+		kfree(skb->head);
+	} else {
+		for (i = 0; i < skb_shinfo(skb)->nr_frags; i++)
+			get_page(skb_shinfo(skb)->frags[i].page);
 
+		if (skb_has_frag_list(skb))
+			skb_clone_fraglist(skb);
+
+		skb_release_data(skb);
+	}
 	off = (data + nhead) - skb->head;
 
 	skb->head     = data;



  parent reply	other threads:[~2010-09-03  9:10 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-09-03  3:43 [PATCH] net: Frag list lost on head expansion David Miller
2010-09-03  5:48 ` Eric Dumazet
2010-09-03  6:19   ` Eric Dumazet
2010-09-03  9:09   ` Eric Dumazet [this message]
2010-09-03 13:46     ` [PATCH net-next-2.6] net: pskb_expand_head() optimization David Miller
2010-09-07  2:20       ` David Miller
2010-09-07  5:02         ` Eric Dumazet
2010-09-07  5:05           ` David Miller
2010-09-07  9:16           ` Jarek Poplawski
2010-09-07  9:37             ` Eric Dumazet
2010-09-10 19:54               ` David Miller
2010-09-11 12:31                 ` Jarek Poplawski
2010-09-12  3:30                   ` David Miller
2010-09-12 10:45                     ` Jarek Poplawski
2010-09-12 10:58                       ` Jarek Poplawski
2010-09-12 15:58                       ` David Miller
2010-09-12 16:13                         ` David Miller
2010-09-12 20:57                           ` Jarek Poplawski
2010-09-12 22:08                             ` David Miller
2010-09-13  7:49                               ` Jarek Poplawski
2010-09-12 19:55                         ` Ben Pfaff
2010-09-12 20:24                           ` David Miller
2010-09-12 20:45                         ` Jarek Poplawski
2010-09-20  0:17                   ` David Miller
2010-09-20  7:21                     ` Jarek Poplawski
2010-09-20  9:02                       ` Eric Dumazet
2010-09-20  9:14                         ` Jarek Poplawski
2010-09-20 12:12                           ` Jarek Poplawski
2010-09-20 12:40                             ` Eric Dumazet
2010-09-20 16:59                       ` David Miller
2010-09-07  1:25     ` David Miller
  -- strict thread matches above, loose matches on Subject: below --
2010-07-22 19:12 [PATCH] Fix corruption of skb csum field in pskb_expand_head() of net/core/skbuff.c Andrea Shepard
2010-07-23  5:09 ` [PATCH net-next-2.6] net: pskb_expand_head() optimization Eric Dumazet
2010-07-25  4:06   ` David Miller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1283504972.2453.257.camel@edumazet-laptop \
    --to=eric.dumazet@gmail.com \
    --cc=davem@davemloft.net \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox