[PATCH net-next] net: skb_peek()/skb_peek_tail() cleanups

[PATCH net-next] net: skb_peek()/skb_peek_tail() cleanups

[Eric Dumazet]

From: Eric Dumazet <edumazet@google.com>

remove useless casts and rename variables for less confusion.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 include/linux/skbuff.h |   20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 2c75e98..988fc49 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -883,10 +883,11 @@ static inline struct sk_buff *skb_unshare(struct sk_buff *skb,
  */
 static inline struct sk_buff *skb_peek(const struct sk_buff_head *list_)
 {
-	struct sk_buff *list = ((const struct sk_buff *)list_)->next;
-	if (list == (struct sk_buff *)list_)
-		list = NULL;
-	return list;
+	struct sk_buff *skb = list_->next;
+
+	if (skb == (struct sk_buff *)list_)
+		skb = NULL;
+	return skb;
 }
 
 /**
@@ -902,6 +903,7 @@ static inline struct sk_buff *skb_peek_next(struct sk_buff *skb,
 		const struct sk_buff_head *list_)
 {
 	struct sk_buff *next = skb->next;
+
 	if (next == (struct sk_buff *)list_)
 		next = NULL;
 	return next;
@@ -922,10 +924,12 @@ static inline struct sk_buff *skb_peek_next(struct sk_buff *skb,
  */
 static inline struct sk_buff *skb_peek_tail(const struct sk_buff_head *list_)
 {
-	struct sk_buff *list = ((const struct sk_buff *)list_)->prev;
-	if (list == (struct sk_buff *)list_)
-		list = NULL;
-	return list;
+	struct sk_buff *skb = list_->prev;
+
+	if (skb == (struct sk_buff *)list_)
+		skb = NULL;
+	return skb;
+
 }
 
 /**

Re: [PATCH net-next] net: skb_peek()/skb_peek_tail() cleanups

[David Miller]

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 01 May 2012 04:31:46 +0200

> From: Eric Dumazet <edumazet@google.com>
> 
> remove useless casts and rename variables for less confusion.
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Applied, I really need to get back to completing the list_head
conversion :-/

Re: [PATCH net-next] net: skb_peek()/skb_peek_tail() cleanups

[Eric Dumazet]

On Tue, 2012-05-01 at 09:41 -0400, David Miller wrote:
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Tue, 01 May 2012 04:31:46 +0200
> 
> > From: Eric Dumazet <edumazet@google.com>
> > 
> > remove useless casts and rename variables for less confusion.
> > 
> > Signed-off-by: Eric Dumazet <edumazet@google.com>
> 
> Applied, I really need to get back to completing the list_head
> conversion :-/

Actually doubly linked lists everywhere has a performance issue.

When we dequeue one skb, we must bring in cpu cache the next skb as
well, to perform the unlink.

In some places it would be better to not have a double linked list, and
only perform a prefetch(skb->next).

Re: [PATCH net-next] net: skb_peek()/skb_peek_tail() cleanups

[David Miller]

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri, 04 May 2012 15:01:31 +0200

> On Tue, 2012-05-01 at 09:41 -0400, David Miller wrote:
>> From: Eric Dumazet <eric.dumazet@gmail.com>
>> Date: Tue, 01 May 2012 04:31:46 +0200
>> 
>> > From: Eric Dumazet <edumazet@google.com>
>> > 
>> > remove useless casts and rename variables for less confusion.
>> > 
>> > Signed-off-by: Eric Dumazet <edumazet@google.com>
>> 
>> Applied, I really need to get back to completing the list_head
>> conversion :-/
> 
> Actually doubly linked lists everywhere has a performance issue.
> 
> When we dequeue one skb, we must bring in cpu cache the next skb as
> well, to perform the unlink.
> 
> In some places it would be better to not have a double linked list, and
> only perform a prefetch(skb->next).

Indeed, I'll keep this in mind.