From: Eric Dumazet <eric.dumazet@gmail.com>
To: Eric Dumazet <edumazet@google.com>,
"David S . Miller" <davem@davemloft.net>
Cc: netdev <netdev@vger.kernel.org>,
Eric Dumazet <eric.dumazet@gmail.com>,
Sagi Grimberg <sagi@lightbitslabs.com>
Subject: Re: [PATCH net-next] net: use indirect call wrappers for skb_copy_datagram_iter()
Date: Tue, 24 Mar 2020 21:22:16 -0700 [thread overview]
Message-ID: <19793d41-acbc-e289-7728-6b77a2f1962c@gmail.com> (raw)
In-Reply-To: <20200325022321.21944-1-edumazet@google.com>
On 3/24/20 7:23 PM, Eric Dumazet wrote:
> TCP recvmsg() calls skb_copy_datagram_iter(), which
> calls an indirect function (cb pointing to simple_copy_to_iter())
> for every MSS (fragment) present in the skb.
>
> CONFIG_RETPOLINE=y forces a very expensive operation
> that we can avoid thanks to indirect call wrappers.
>
> This patch gives a 13% increase of performance on
> a single flow, if the bottleneck is the thread reading
> the TCP socket.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
BTW, the expensive indirect call came with :
So we could also add a Fixes: tag eventually
Fixes: 950fcaecd5cc ("datagram: consolidate datagram copy to iter helpers")
commit 950fcaecd5cc6c014bb96506fd0652a501c85276
Author: Sagi Grimberg <sagi@lightbitslabs.com>
Date: Mon Dec 3 17:52:08 2018 -0800
datagram: consolidate datagram copy to iter helpers
skb_copy_datagram_iter and skb_copy_and_csum_datagram are essentialy
the same but with a couple of differences: The first is the copy
operation used which either a simple copy or a csum_and_copy, and the
second are the behavior on the "short copy" path where simply copy
needs to return the number of bytes successfully copied while csum_and_copy
needs to fault immediately as the checksum is partial.
Introduce __skb_datagram_iter that additionally accepts:
1. copy operation function pointer
2. private data that goes with the copy operation
3. fault_short flag to indicate the action on short copy
Suggested-by: David S. Miller <davem@davemloft.net>
Acked-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Sagi Grimberg <sagi@lightbitslabs.com>
Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> net/core/datagram.c | 14 +++++++++++---
> 1 file changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/net/core/datagram.c b/net/core/datagram.c
> index 4213081c6ed3d4fda69501641a8c76e041f26b42..639745d4f3b94a248da9a685f45158410a85bec7 100644
> --- a/net/core/datagram.c
> +++ b/net/core/datagram.c
> @@ -51,6 +51,7 @@
> #include <linux/slab.h>
> #include <linux/pagemap.h>
> #include <linux/uio.h>
> +#include <linux/indirect_call_wrapper.h>
>
> #include <net/protocol.h>
> #include <linux/skbuff.h>
> @@ -403,6 +404,11 @@ int skb_kill_datagram(struct sock *sk, struct sk_buff *skb, unsigned int flags)
> }
> EXPORT_SYMBOL(skb_kill_datagram);
>
> +INDIRECT_CALLABLE_DECLARE(static size_t simple_copy_to_iter(const void *addr,
> + size_t bytes,
> + void *data __always_unused,
> + struct iov_iter *i));
> +
> static int __skb_datagram_iter(const struct sk_buff *skb, int offset,
> struct iov_iter *to, int len, bool fault_short,
> size_t (*cb)(const void *, size_t, void *,
> @@ -416,7 +422,8 @@ static int __skb_datagram_iter(const struct sk_buff *skb, int offset,
> if (copy > 0) {
> if (copy > len)
> copy = len;
> - n = cb(skb->data + offset, copy, data, to);
> + n = INDIRECT_CALL_1(cb, simple_copy_to_iter,
> + skb->data + offset, copy, data, to);
> offset += n;
> if (n != copy)
> goto short_copy;
> @@ -438,8 +445,9 @@ static int __skb_datagram_iter(const struct sk_buff *skb, int offset,
>
> if (copy > len)
> copy = len;
> - n = cb(vaddr + skb_frag_off(frag) + offset - start,
> - copy, data, to);
> + n = INDIRECT_CALL_1(cb, simple_copy_to_iter,
> + vaddr + skb_frag_off(frag) + offset - start,
> + copy, data, to);
> kunmap(page);
> offset += n;
> if (n != copy)
>
next prev parent reply other threads:[~2020-03-25 4:22 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-25 2:23 [PATCH net-next] net: use indirect call wrappers for skb_copy_datagram_iter() Eric Dumazet
2020-03-25 4:22 ` Eric Dumazet [this message]
2020-03-25 11:52 ` Paolo Abeni
2020-03-25 14:55 ` Willem de Bruijn
2020-03-25 16:00 ` Paolo Abeni
2020-03-25 16:07 ` Eric Dumazet
2020-03-25 16:24 ` Paolo Abeni
[not found] ` <CANn89iKotU9Tkd6KBgyicHFV72K9gZ+eeKwkPU097=gZZYCjrA@mail.gmail.com>
2020-03-25 16:46 ` Eric Dumazet
2020-03-25 20:58 ` Willem de Bruijn
2020-03-25 15:14 ` Eric Dumazet
2020-03-25 18:31 ` 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=19793d41-acbc-e289-7728-6b77a2f1962c@gmail.com \
--to=eric.dumazet@gmail.com \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=netdev@vger.kernel.org \
--cc=sagi@lightbitslabs.com \
/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;
as well as URLs for NNTP newsgroup(s).