net/udp: do not touch skb->peeked unless really needed
authorEric Dumazet <edumazet@google.com>
Mon, 5 Dec 2016 17:57:19 +0000 (09:57 -0800)
committerDavid S. Miller <davem@davemloft.net>
Tue, 6 Dec 2016 15:41:49 +0000 (10:41 -0500)
In UDP recvmsg() path we currently access 3 cache lines from an skb
while holding receive queue lock, plus another one if packet is
dequeued, since we need to change skb->next->prev

1st cache line (contains ->next/prev pointers, offsets 0x00 and 0x08)
2nd cache line (skb->len & skb->peeked, offsets 0x80 and 0x8e)
3rd cache line (skb->truesize/users, offsets 0xe0 and 0xe4)

skb->peeked is only needed to make sure 0-length packets are properly
handled while MSG_PEEK is operated.

I had first the intent to remove skb->peeked but the "MSG_PEEK at
non-zero offset" support added by Sam Kumar makes this not possible.

This patch avoids one cache line miss during the locked section, when
skb->len and skb->peeked do not have to be read.

It also avoids the skb_set_peeked() cost for non empty UDP datagrams.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Acked-by: Paolo Abeni <pabeni@redhat.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
net/core/datagram.c

index 49816af8586bb832e806972b486588041a99524c..9482037a5c8c64aec79e42c65bd2691bdd9450a3 100644 (file)
@@ -214,6 +214,7 @@ struct sk_buff *__skb_try_recv_datagram(struct sock *sk, unsigned int flags,
        if (error)
                goto no_packet;
 
+       *peeked = 0;
        do {
                /* Again only user level code calls this function, so nothing
                 * interrupt level will suddenly eat the receive_queue.
@@ -227,22 +228,22 @@ struct sk_buff *__skb_try_recv_datagram(struct sock *sk, unsigned int flags,
                spin_lock_irqsave(&queue->lock, cpu_flags);
                skb_queue_walk(queue, skb) {
                        *last = skb;
-                       *peeked = skb->peeked;
                        if (flags & MSG_PEEK) {
                                if (_off >= skb->len && (skb->len || _off ||
                                                         skb->peeked)) {
                                        _off -= skb->len;
                                        continue;
                                }
-
-                               skb = skb_set_peeked(skb);
-                               error = PTR_ERR(skb);
-                               if (IS_ERR(skb)) {
-                                       spin_unlock_irqrestore(&queue->lock,
-                                                              cpu_flags);
-                                       goto no_packet;
+                               if (!skb->len) {
+                                       skb = skb_set_peeked(skb);
+                                       if (IS_ERR(skb)) {
+                                               error = PTR_ERR(skb);
+                                               spin_unlock_irqrestore(&queue->lock,
+                                                                      cpu_flags);
+                                               goto no_packet;
+                                       }
                                }
-
+                               *peeked = 1;
                                atomic_inc(&skb->users);
                        } else {
                                __skb_unlink(skb, queue);