mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: David Howells <dhowells@redhat.com>
To: David Laight <David.Laight@ACULAB.COM>,
	Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Cc: dhowells@redhat.com, "David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Jens Axboe <axboe@kernel.dk>, Al Viro <viro@zeniv.linux.org.uk>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Christoph Hellwig <hch@lst.de>,
	Christian Brauner <christian@brauner.io>,
	Matthew Wilcox <willy@infradead.org>,
	Jeff Layton <jlayton@kernel.org>,
	linux-fsdevel@vger.kernel.org, linux-block@vger.kernel.org,
	linux-mm@kvack.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v5 00/11] iov_iter: Convert the iterator macros into inline funcs
Date: Fri, 22 Sep 2023 13:01:07 +0100	[thread overview]
Message-ID: <1173637.1695384067@warthog.procyon.org.uk> (raw)
In-Reply-To: <591a70bf016b4317add2d936696abc0f@AcuMS.aculab.com>

David Laight <David.Laight@ACULAB.COM> wrote:

> >  (8) Move the copy-and-csum code to net/ where it can be in proximity with
> >      the code that uses it.  This eliminates the code if CONFIG_NET=n and
> >      allows for the slim possibility of it being inlined.
> > 
> >  (9) Fold memcpy_and_csum() in to its two users.
> > 
> > (10) Move csum_and_copy_from_iter_full() out of line and merge in
> >      csum_and_copy_from_iter() since the former is the only caller of the
> >      latter.
> 
> I thought that the real idea behind these was to do the checksum
> at the same time as the copy to avoid loading the data into the L1
> data-cache twice - especially for long buffers.
> I wonder how often there are multiple iov[] that actually make
> it better than just check summing the linear buffer?

It also reduces the overhead for finding the data to checksum in the case the
packet gets split since we're doing the checksumming as we copy - but with a
linear buffer, that's negligible.

> I had a feeling that check summing of udp data was done during
> copy_to/from_user, but the code can't be the copy-and-csum here
> for that because it is missing support form odd-length buffers.

Is there a bug there?

> Intel x86 desktop chips can easily checksum at 8 bytes/clock
> (But probably not with the current code!).
> (I've got ~12 bytes/clock using adox and adcx but that loop
> is entirely horrid and it would need run-time patching.
> Especially since I think some AMD cpu execute them very slowly.)
> 
> OTOH 'rep movs[bq]' copy will copy 16 bytes/clock (32 if the
> destination is 32 byte aligned - it pretty much won't be).
> 
> So you'd need a csum-and-copy loop that did 16 bytes every
> three clocks to get the same throughput for long buffers.
> In principle splitting the 'adc memory' into two instructions
> is the same number of u-ops - but I'm sure I've tried to do
> that and failed and the extra memory write can happen in
> parallel with everything else.
> So I don't think you'll get 16 bytes in two clocks - but you
> might get it is three.
> 
> OTOH for a cpu where memcpy is code loop summing the data in
> the copy loop is likely to be a gain.
> 
> But I suspect doing the checksum and copy at the same time
> got 'all to complicated' to actually implement fully.
> With most modern ethernet chips checksumming receive pacakets
> does it really get used enough for the additional complexity?

You may be right.  That's more a question for the networking folks than for
me.  It's entirely possible that the checksumming code is just not used on
modern systems these days.

Maybe Willem can comment since he's the UDP maintainer?

David


  parent reply	other threads:[~2023-09-22 12:02 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-20 22:22 David Howells
2023-09-20 22:22 ` [PATCH v5 01/11] sound: Fix snd_pcm_readv()/writev() to use iov access functions David Howells
2023-09-21  6:08   ` Jaroslav Kysela
2023-09-21 13:14   ` Takashi Iwai
2023-09-21 15:03   ` David Howells
2023-09-21 15:10     ` Takashi Iwai
2023-09-20 22:22 ` [PATCH v5 02/11] infiniband: Use user_backed_iter() to see if iterator is UBUF/IOVEC David Howells
2023-09-20 22:22 ` [PATCH v5 03/11] iov_iter: Renumber ITER_* constants David Howells
2023-09-20 22:22 ` [PATCH v5 04/11] iov_iter: Derive user-backedness from the iterator type David Howells
2023-09-20 22:22 ` [PATCH v5 05/11] iov_iter: Convert iterate*() to inline funcs David Howells
2023-09-22  9:32   ` Simon Horman
2023-09-22 11:38   ` David Howells
2023-09-20 22:22 ` [PATCH v5 06/11] iov_iter: Don't deal with iter->copy_mc in memcpy_from_iter_mc() David Howells
2023-09-20 22:22 ` [PATCH v5 07/11] iov_iter: Add a kernel-type iterator-only iteration function David Howells
2023-09-22  9:34   ` Simon Horman
2023-09-20 22:22 ` [PATCH v5 08/11] iov_iter, net: Move csum_and_copy_to/from_iter() to net/ David Howells
2023-09-20 22:22 ` [PATCH v5 09/11] iov_iter, net: Fold in csum_and_memcpy() David Howells
2023-09-20 22:22 ` [PATCH v5 10/11] iov_iter, net: Merge csum_and_copy_from_iter{,_full}() together David Howells
2023-09-20 22:22 ` [PATCH v5 11/11] iov_iter, net: Move hash_and_copy_to_iter() to net/ David Howells
2023-09-21 14:04 ` [PATCH v5 00/11] iov_iter: Convert the iterator macros into inline funcs David Laight
2023-09-22 12:01 ` David Howells [this message]
2023-09-23  6:59   ` Willem de Bruijn
2023-09-23 10:31     ` David Laight

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=1173637.1695384067@warthog.procyon.org.uk \
    --to=dhowells@redhat.com \
    --cc=David.Laight@ACULAB.COM \
    --cc=axboe@kernel.dk \
    --cc=christian@brauner.io \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=hch@lst.de \
    --cc=jlayton@kernel.org \
    --cc=kuba@kernel.org \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=torvalds@linux-foundation.org \
    --cc=viro@zeniv.linux.org.uk \
    --cc=willemdebruijn.kernel@gmail.com \
    --cc=willy@infradead.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

all inboxes | Powered by JetHome®