All of lore.kernel.org
 help / color / mirror / Atom feed
From: Al Viro <viro@ZenIV.linux.org.uk>
To: David Miller <davem@davemloft.net>
Cc: axboe@kernel.dk, sfrench@samba.org, sage@inktank.com,
	mfasheh@suse.com, xfs@oss.sgi.com, hch@infradead.org,
	jlbec@evilplan.org, linux-fsdevel@vger.kernel.org,
	torvalds@linux-foundation.org
Subject: Re: [PATCH 0/5] splice: locking changes and code refactoring
Date: Fri, 7 Feb 2014 17:10:23 +0000	[thread overview]
Message-ID: <20140207171023.GU10323@ZenIV.linux.org.uk> (raw)
In-Reply-To: <20140118.004453.1800341321580114709.davem@davemloft.net>

On Sat, Jan 18, 2014 at 12:44:53AM -0800, David Miller wrote:
> From: Al Viro <viro@ZenIV.linux.org.uk>
> Date: Sat, 18 Jan 2014 08:27:30 +0000
> 
> > BTW, would sockets benefit from having ->sendpages() that would take an
> > array of (page, offset, len) triples?  It would be trivial to do and
> > some of the helpers that are falling out of writing that writev-based
> > default_file_splice_write() look like they could be reused for
> > calling that one...  Dave?
> 
> That's originally how the sendpage method was implemented, but back then
> Linus asked us to only pass one page at a time.
> 
> I don't remember the details beyond that.

FWIW, I wonder if what we are doing with ->msg_iov is the right thing.
We modify the iovecs in array as we drain it.  And that's inconvenient
for at least some callers (see e.g. complaints in fs/ncpfs about the
need to copy the array, etc.).

What if we embed iov_iter into the sucker and replace memcpy_{to,from}iovec*
with variants taking iov_iter *?  If nothing else, it'll be marginally more
efficient (no more skipping the already-emptied iovecs) and it seems to be
more convenient for callers.  If we are lucky, that might even eliminate
the need of ->sendpage() - just set the iov_iter over <page,offset,size>
array instead of iovec one and let ->sendmsg() do the smart thing if it
knows how.  I hadn't done comparison of {tcp,udp}_send{page,msg}, though -
there might be dragons...  Even if that will turn out to be infeasible,
it will at least drive the kmap/kunmap done by sock_no_sendpage() down
into memcpy_from_iter(), turning them into kmap_atomic/kunmap_atomic.

The obvious price is that kernel-side msghdr diverges from the userland
one, so copy_msghdr_from_user() needs to deal with that, but I really
doubt that you'll find a load where the price of copying it in two
chunks instead of one would be measurable.  

What else am I missing?

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

WARNING: multiple messages have this Message-ID (diff)
From: Al Viro <viro@ZenIV.linux.org.uk>
To: David Miller <davem@davemloft.net>
Cc: torvalds@linux-foundation.org, hch@infradead.org,
	axboe@kernel.dk, mfasheh@suse.com, jlbec@evilplan.org,
	linux-fsdevel@vger.kernel.org, xfs@oss.sgi.com, sage@inktank.com,
	sfrench@samba.org
Subject: Re: [PATCH 0/5] splice: locking changes and code refactoring
Date: Fri, 7 Feb 2014 17:10:23 +0000	[thread overview]
Message-ID: <20140207171023.GU10323@ZenIV.linux.org.uk> (raw)
In-Reply-To: <20140118.004453.1800341321580114709.davem@davemloft.net>

On Sat, Jan 18, 2014 at 12:44:53AM -0800, David Miller wrote:
> From: Al Viro <viro@ZenIV.linux.org.uk>
> Date: Sat, 18 Jan 2014 08:27:30 +0000
> 
> > BTW, would sockets benefit from having ->sendpages() that would take an
> > array of (page, offset, len) triples?  It would be trivial to do and
> > some of the helpers that are falling out of writing that writev-based
> > default_file_splice_write() look like they could be reused for
> > calling that one...  Dave?
> 
> That's originally how the sendpage method was implemented, but back then
> Linus asked us to only pass one page at a time.
> 
> I don't remember the details beyond that.

FWIW, I wonder if what we are doing with ->msg_iov is the right thing.
We modify the iovecs in array as we drain it.  And that's inconvenient
for at least some callers (see e.g. complaints in fs/ncpfs about the
need to copy the array, etc.).

What if we embed iov_iter into the sucker and replace memcpy_{to,from}iovec*
with variants taking iov_iter *?  If nothing else, it'll be marginally more
efficient (no more skipping the already-emptied iovecs) and it seems to be
more convenient for callers.  If we are lucky, that might even eliminate
the need of ->sendpage() - just set the iov_iter over <page,offset,size>
array instead of iovec one and let ->sendmsg() do the smart thing if it
knows how.  I hadn't done comparison of {tcp,udp}_send{page,msg}, though -
there might be dragons...  Even if that will turn out to be infeasible,
it will at least drive the kmap/kunmap done by sock_no_sendpage() down
into memcpy_from_iter(), turning them into kmap_atomic/kunmap_atomic.

The obvious price is that kernel-side msghdr diverges from the userland
one, so copy_msghdr_from_user() needs to deal with that, but I really
doubt that you'll find a load where the price of copying it in two
chunks instead of one would be measurable.  

What else am I missing?

  reply	other threads:[~2014-02-07 17:10 UTC|newest]

Thread overview: 82+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-12-12 18:14 [PATCH 0/5] splice: locking changes and code refactoring Christoph Hellwig
2013-12-12 18:14 ` Christoph Hellwig
2013-12-12 18:15 ` [PATCH 1/5] splice: move balance_dirty_pages_ratelimited into pipe_to_file Christoph Hellwig
2013-12-12 18:15   ` Christoph Hellwig
2013-12-12 18:15 ` [PATCH 2/5] splice: nest i_mutex outside pipe_lock Christoph Hellwig
2013-12-12 18:15   ` Christoph Hellwig
2013-12-12 18:15 ` [PATCH 3/5] splice: use splice_from_pipe in generic_file_splice_write Christoph Hellwig
2013-12-12 18:15   ` Christoph Hellwig
2013-12-12 18:15 ` [PATCH 4/5] xfs: fix splice_write locking Christoph Hellwig
2013-12-12 18:15   ` Christoph Hellwig
2013-12-12 18:15 ` [PATCH 5/5] splice: stop exporting splice_from_pipe implementation details Christoph Hellwig
2013-12-12 18:15   ` Christoph Hellwig
2014-01-13 14:14 ` [PATCH 0/5] splice: locking changes and code refactoring Christoph Hellwig
2014-01-13 14:14   ` Christoph Hellwig
2014-01-13 23:56   ` Al Viro
2014-01-13 23:56     ` Al Viro
2014-01-14 13:22     ` Christoph Hellwig
2014-01-14 13:22       ` Christoph Hellwig
2014-01-14 17:20       ` Al Viro
2014-01-14 17:20         ` Al Viro
2014-01-15 18:10         ` Al Viro
2014-01-15 18:10           ` Al Viro
2014-01-18  6:40         ` Al Viro
2014-01-18  7:22           ` Linus Torvalds
2014-01-18  7:22             ` Linus Torvalds
2014-01-18  7:46             ` Al Viro
2014-01-18  7:56               ` Al Viro
2014-01-18  7:56                 ` Al Viro
2014-01-18  8:27               ` Al Viro
2014-01-18  8:44                 ` David Miller
2014-01-18  8:44                   ` David Miller
2014-02-07 17:10                   ` Al Viro [this message]
2014-02-07 17:10                     ` Al Viro
2014-01-18 19:59               ` Linus Torvalds
2014-01-18 20:10                 ` Al Viro
2014-01-18 20:27                   ` Al Viro
2014-01-18 20:27                     ` Al Viro
2014-01-18 20:30                     ` Al Viro
2014-01-18 20:30                       ` Al Viro
2014-01-19  5:13                   ` [RFC] unifying write variants for filesystems Al Viro
2014-01-19  5:13                     ` Al Viro
2014-01-20 13:55                     ` Christoph Hellwig
2014-01-20 13:55                       ` Christoph Hellwig
2014-01-20 20:32                       ` Linus Torvalds
2014-01-20 20:32                         ` Linus Torvalds
2014-02-01 22:43                         ` Al Viro
2014-02-01 22:43                           ` Al Viro
2014-02-02  0:13                           ` Linus Torvalds
2014-02-02  2:02                             ` Al Viro
2014-02-02  2:02                               ` Al Viro
2014-02-02 19:21                           ` Al Viro
2014-02-02 19:21                             ` Al Viro
2014-02-02 19:23                             ` Al Viro
2014-02-02 19:23                               ` Al Viro
2014-02-03 14:41                             ` Miklos Szeredi
2014-02-03 14:41                               ` Miklos Szeredi
2014-02-03 15:33                               ` Al Viro
2014-02-03 15:33                                 ` Al Viro
2014-02-02 23:16                           ` Anton Altaparmakov
2014-02-02 23:16                             ` Anton Altaparmakov
2014-02-03 15:12                           ` Christoph Hellwig
2014-02-03 16:24                             ` Al Viro
2014-02-03 16:50                             ` Dave Kleikamp
2014-02-03 16:23                           ` Dave Kleikamp
2014-02-04 12:44                             ` Al Viro
2014-02-04 12:44                               ` Al Viro
2014-02-04 12:52                               ` Kent Overstreet
2014-02-04 12:52                                 ` Kent Overstreet
2014-02-04 15:17                                 ` Al Viro
2014-02-04 15:17                                   ` Al Viro
2014-02-04 17:27                                   ` Zach Brown
2014-02-04 17:35                                     ` Kent Overstreet
2014-02-04 18:08                                       ` Al Viro
2014-02-04 18:08                                         ` Al Viro
2014-02-04 18:00                                     ` Al Viro
2014-02-04 18:00                                       ` Al Viro
2014-02-04 18:33                                       ` Zach Brown
2014-02-04 18:36                                         ` Al Viro
2014-02-04 18:36                                           ` Al Viro
2014-02-05 19:58                                           ` Al Viro
2014-02-05 20:42                                             ` Zach Brown
2014-02-06  9:08                                             ` Kent Overstreet

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=20140207171023.GU10323@ZenIV.linux.org.uk \
    --to=viro@zeniv.linux.org.uk \
    --cc=axboe@kernel.dk \
    --cc=davem@davemloft.net \
    --cc=hch@infradead.org \
    --cc=jlbec@evilplan.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=mfasheh@suse.com \
    --cc=sage@inktank.com \
    --cc=sfrench@samba.org \
    --cc=torvalds@linux-foundation.org \
    --cc=xfs@oss.sgi.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.