Re: [PATCH net-next 1/2] tcp: add TCP_ZEROCOPY_RECEIVE support for zerocopy receive

On Wed, Apr 25, 2018 at 9:04 AM, Matthew Wilcox [off-list ref] wrote:

On Wed, Apr 25, 2018 at 06:01:02AM -0700, Eric Dumazet wrote:

quoted

On 04/24/2018 11:28 PM, Christoph Hellwig wrote:

quoted

On Tue, Apr 24, 2018 at 10:27:21PM -0700, Eric Dumazet wrote:

quoted

When adding tcp mmap() implementation, I forgot that socket lock
had to be taken before current->mm->mmap_sem. syzbot eventually caught
the bug.

Since we can not lock the socket in tcp mmap() handler we have to
split the operation in two phases.

1) mmap() on a tcp socket simply reserves VMA space, and nothing else.
  This operation does not involve any TCP locking.

2) setsockopt(fd, IPPROTO_TCP, TCP_ZEROCOPY_RECEIVE, ...) implements
 the transfert of pages from skbs to one VMA.
  This operation only uses down_read(&current->mm->mmap_sem) after
  holding TCP lock, thus solving the lockdep issue.

This new implementation was suggested by Andy Lutomirski with great details.

Thanks, this looks much more sensible to me.

Thanks Christoph

Note the high cost of zap_page_range(), needed to avoid -EBUSY being returned
from vm_insert_page() the second time TCP_ZEROCOPY_RECEIVE is used on one VMA.

Ideally a vm_replace_page() would avoid this cost ?

If you don't zap the page range, any of the CPUs in the system where
any thread in this task have ever run may have a TLB entry pointing to
this page ... if the page is being recycled into the page allocator,
then that page might end up as a slab page or page table or page cache
while the other CPU still have access to it.

Indeed.  This is one of the reasons that Linus has generally been
quite vocal that he doesn't like MMU-based zerocopy schemes.

You could hang onto the page until you've built up a sufficiently large
batch, then bulk-invalidate all of the TLB entries, but we start to get
into weirdnesses on different CPU architectures.

The existing mmu_gather code should already handle this at least
moderately well.  If it's not, then it should be fixed.

On x86, there is no operation to flush a range of addresses.  You can
flush one address or you can flush all of them.  If you flush one page
at a time, then you might never recover the performance of a plain old
memcpy().  If you flush all of them, then you're hurting the
performance of everything else in the task.

In general, I suspect that the zerocopy receive mechanism will only
really be a win in single-threaded applications that consume large
amounts of receive bandwidth on a single TCP socket using lots of
memory and don't do all that much else.