On Wed, Sep 14, 2016 at 05:25:59AM +0100, Al Viro wrote:
> On Tue, Sep 13, 2016 at 08:49:58PM -0700, Linus Torvalds wrote:
>
> > I'd also like to simplify the splice code if at all possible.
>
> Then pipe_buffer it is; it will take a bit of surgery, but I'd expect
> the end result to be much simpler. OK, so splice_pipe_desc switches
> from the pages/partial_pages/ops/spd_release to pipe_bufs, and I'm
> actually tempted to replace nr_pages with "the rest of ->pipe_bufs[] has
> NULL ->page". Then it becomes simply
> struct splice_pipe_desc {
> struct pipe_buffer *bufs;
> int nbufs;
> unsigned flags;
> }, perhaps with struct pipe_buffer _bufs[INLINE_SPLICE_BUFS]; in the end.
> struct partial_page simply dies...
Actually, we can do even better, and kill the sodding splice_pipe_desc
entirely, along with skb_splice_bits() callback.
1) make splice_to_pipe() return on pipe overflow, flags be damned. And
lift pipe_lock()/looping/waking the readers up into callers. Basically,
what you've suggested earlier in the thread. There are 2 kinds of callers -
vmsplice_to_pipe() and assorted ->splice_read(), called from do_splice_to().
pipe_lock and loop is lifted into vmsplice_to_pipe() and into do_splice();
another caller of do_splice_to() already has a loop *and* couldn't wait
on the pipe anyway - it uses an internal one.
2) fuse_dev_splice_read() checks the amount of space in the pipe and
either buggers off or calls splice_to_pipe().
3) since the pipe is locked, skb_splice_bits() callbacks don't need to
unlock/relock any socket locks. All those callbacks are simply
splice_to_pipe() and can be replaced with direct call of that sucker.
4) since the pipe is locked, there's no point feeding the bits in one go;
we can as well send them one by one. That kills splice_to_pipe(),
splice_pipe_desc and these on-stack arrays, along with the questions about
their size.
5) that iov_iter flavour is backed by pipe. {__,}generic_file_splice_read()
is gone - we simply set an iov_iter over our locked pipe and pass it to
->read_iter(). That serves as ->splice_read() where generic_file_splice_read()
used to be used, as well as nfs/ocfs2/gfs2/shmem instances.
|