Messages in this thread | | | Date | Tue, 26 Jan 2021 06:07:20 +0000 | From | Al Viro <> | Subject | Re: Splicing to/from a tty |
| |
On Sun, Jan 24, 2021 at 11:11:42AM -0800, Linus Torvalds wrote: > Al, > coming back to this because rc5 is imminent.. > > On Mon, Jan 18, 2021 at 11:45 AM Al Viro <viro@zeniv-ca> wrote: > > > > do_splice_direct() does something that do_splice() won't - it > > handles non-pipe to non-pipe case. Which is how sendfile(2) is > > normally used, of course. > > > > I'll look into that in more details, but IMO bothering with > > internal pipe is just plain wrong for those cases. > > You clearly thought about this, with the emails about odd error cases, > but I get the feeling that for fixing the current "you can't > sendfile() to a pipe" regression (including stable) we should do the > one-liner. No? > > I agree that it would be better fixed by just having sendfile() > basically turn into splice() for the pipe target case, but I haven't > seen any patches from you, so I assume it wasn't 100% trivial. > > Hmm?
Just to make clear - sendfile() regular-to-pipe is *not* the same issue as splice to/from tty. The latter needs ->splice_read() and ->splice_write() in tty_fops; the former can be dealt with by teaching do_sendifile() to use the guts of do_splice() in case when in or out happens to be a pipe (with some rearrangement of checks) instead of bothering with do_splice_direct().
The only commonality between these two is that both got broken by the same patch series. Johannes' patch is an attempt to deal with regular-to-pipe sendfile(2), and it's not a good way to handle that. Neither it, nor the variant I proposed would do a damn thing for tty (and sendfile(2) never worked for source other than regular or block anyway). FWIW, the real check in do_splice_direct() should be "has FMODE_PREAD", regardless of the position we are asking to read from - do_splice_direct() is basically while left to copy splice_read from in to internal pipe splice_write from internal pipe to out and if splice_write ends up with short copy, we advance the position by the amount written and discard everything left in the internal pipe. Using it for something non-seekable would end up with data silently lost on short copy.
Note that decision against splice(2) between non-pipes appears to have been deliberate and unless Jens (and mingo?) decide they are OK with that change, I'm *not* adding that functionality to do_splice().
Anyway, the series is in vfs.git #work.sendfile, 5.11-rc1-based
Shortlog: Al Viro (3): do_splice_to(): move the logics for limiting the read length in take the guts of file-to-pipe splice into a helper function teach sendfile(2) to handle send-to-pipe directly
Diffstat: fs/internal.h | 9 +++++++++ fs/read_write.c | 19 +++++++++++++------ fs/splice.c | 42 +++++++++++++++++++++++------------------- 3 files changed, 45 insertions(+), 25 deletions(-)
Individual patches in followups; first two are equivalent transformations (fairly obvious cleanup and taking a part of do_splice() into a helper), while the third one makes do_sendfile() use that new helper for file-to-pipe case.
| |