On Thu, Jun 23, 2022 at 04:21:52PM +0100, David Howells wrote:
> Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote:
>
> >
> > 13/44: splice: stop abusing iov_iter_advance() to flush a pipe
> > A really odd (ab)use of iov_iter_advance() - in case of error
> > generic_file_splice_read() wants to free all pipe buffers ->read_iter()
> > has produced. Yes, forcibly resetting ->head and ->iov_offset to
> > original values and calling iov_iter_advance(i, 0) will trigger
> > pipe_advance(), which will trigger pipe_truncate(), which will free
> > buffers. Or we could just go ahead and free the same buffers;
> > pipe_discard_from() does exactly that, no iov_iter stuff needs to
> > be involved.
>
> Can ->splice_read() and ->splice_write() be given pipe-class iov_iters rather
> than pipe_inode_info structs?
Huh?
First of all, ->splice_write() is given a pipe as data _source_, which makes
ITER_PIPE completely irrelevant - those suckers are data destinations.
What's more, it will unlock the pipe, wait and relock once somebody writes
to that pipe. And ITER_PIPE very much relies upon the pipe being locked
and staying locked.
As for ->splice_read()... We could create the iov_iter in the caller, but...
Look at those callers:
pipe_lock(opipe);
ret = wait_for_space(opipe, flags);
if (!ret)
ret = do_splice_to(in, offset, opipe, len, flags);
pipe_unlock(opipe);
You can't set it up before wait_for_pace(), obviously - if there's no
empty slots, what would that sucker work with? And you can't keep
it past pipe_unlock(). So it's limited to do_splice_to(), which
is a very thin wrapper for ->splice_read(). So what's the point?
