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

[David Miller <davem@davemloft.net>]

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri, 03 Sep 2010 11:09:32 +0200

> 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>

Ok, I see how this works now, thanks Eric.  But this looks to be a bit
too complicated to be worthwhile, I think, so let's hold off on this
optimization for a while.

Let me tell you why I was swimming around in this area and what my
ultimate motivation is :-)

In trying to eventually convert sk_buff to list_head one of the
troubling areas is this frag_list business.

Amusingly, if you look, virtually all of the code which constructs
frag_list SKBs wants a doubly linked list so it can insert at the tail
(all LRO gathering, GRO, you name it).

There are only two operations that make a double-linked list currently
impossible.  This pskb_expand_head() thing, and pskb_copy().

They cause the situation where the list is shared between multiple SKB
data shared areas.

And because of this a doubly-linked list like list_head won't work at
all.

For the optimized case of pskb_expand_head() you discovered we can avoid
this sharing.  And at this point I imagine that for the remaining cases
we can simply copy the full SKBs in the frag list to avoid this list
sharing.

So that's where I'm trying to go in all of this.