From: Al Viro <viro@zeniv.linux.org.uk>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Johannes Berg <johannes@sipsolutions.net>,
Christoph Hellwig <hch@lst.de>,
Oliver Giles <ohw.giles@gmail.com>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Subject: Re: Splicing to/from a tty
Date: Tue, 26 Jan 2021 06:07:20 +0000 [thread overview]
Message-ID: <20210126060720.GJ740243@zeniv-ca> (raw)
In-Reply-To: <CAHk-=wgrkX4KM-kbEzs+Bap4v13RQ-ymwwAxgwjhTz6VVtp8Cw@mail.gmail.com>
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.
next prev parent reply other threads:[~2021-01-26 17:17 UTC|newest]
Thread overview: 65+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-16 7:35 Splicing to/from a tty Oliver Giles
2021-01-16 16:46 ` Johannes Berg
2021-01-17 6:12 ` Oliver Giles
2021-01-18 8:53 ` Christoph Hellwig
2021-01-18 8:58 ` Johannes Berg
2021-01-18 19:26 ` Linus Torvalds
2021-01-18 19:45 ` Al Viro
2021-01-18 19:49 ` Linus Torvalds
2021-01-18 19:56 ` Al Viro
2021-01-24 19:11 ` Linus Torvalds
2021-01-25 9:16 ` [PATCH] fs/pipe: allow sendfile() to pipe again Johannes Berg
2021-01-25 10:16 ` Christoph Hellwig
2021-01-25 20:34 ` Linus Torvalds
2021-01-26 6:07 ` Al Viro [this message]
2021-01-26 6:08 ` [PATCH 1/3] do_splice_to(): move the logics for limiting the read length in Al Viro
2021-01-26 6:09 ` [PATCH 2/3] take the guts of file-to-pipe splice into a helper function Al Viro
2021-01-26 6:09 ` [PATCH 3/3] teach sendfile(2) to handle send-to-pipe directly Al Viro
2021-01-26 18:57 ` Linus Torvalds
2021-01-26 19:33 ` Al Viro
2021-01-26 18:49 ` Splicing to/from a tty Linus Torvalds
2021-01-26 19:42 ` Al Viro
2021-01-18 19:34 ` Al Viro
2021-01-18 19:46 ` Linus Torvalds
2021-01-18 19:54 ` Al Viro
2021-01-20 16:26 ` Al Viro
2021-01-20 19:11 ` Al Viro
2021-01-20 19:27 ` Linus Torvalds
2021-01-20 22:25 ` David Laight
2021-01-20 23:02 ` Al Viro
2021-01-20 23:14 ` Al Viro
2021-01-20 23:40 ` Linus Torvalds
2021-01-21 0:38 ` Al Viro
2021-01-21 1:04 ` Linus Torvalds
2021-01-21 1:45 ` Al Viro
2021-01-21 3:38 ` Linus Torvalds
2021-01-21 6:05 ` Willy Tarreau
2021-01-21 8:04 ` Johannes Berg
2021-01-21 10:08 ` David Laight
2021-01-18 8:16 ` Christoph Hellwig
2021-01-18 19:36 ` Linus Torvalds
2021-01-18 20:24 ` Linus Torvalds
2021-01-18 21:35 ` Linus Torvalds
2021-01-18 21:54 ` Linus Torvalds
2021-01-18 22:03 ` Linus Torvalds
2021-01-18 22:20 ` Linus Torvalds
2021-01-19 1:38 ` Linus Torvalds
2021-01-19 11:53 ` Greg Kroah-Hartman
2021-01-19 16:56 ` Robert Karszniewicz
2021-01-19 17:10 ` Robert Karszniewicz
2021-01-19 22:09 ` Oliver Giles
2021-01-19 17:25 ` Linus Torvalds
2021-01-19 20:24 ` Linus Torvalds
2021-01-19 20:38 ` Christoph Hellwig
2021-01-20 1:25 ` Oliver Giles
2021-01-20 4:44 ` Linus Torvalds
2021-01-20 8:15 ` Oliver Giles
2021-01-21 1:18 ` tty splice branch (was "Re: Splicing to/from a tty") Linus Torvalds
2021-01-21 8:44 ` Greg Kroah-Hartman
2021-01-21 8:50 ` Jiri Slaby
2021-01-21 8:58 ` Jiri Slaby
2021-01-21 17:52 ` Linus Torvalds
2021-01-21 8:58 ` Greg Kroah-Hartman
2021-01-21 17:03 ` Splicing to/from a tty Robert Karszniewicz
2021-01-21 18:43 ` Linus Torvalds
2021-01-19 11:52 ` Greg Kroah-Hartman
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=20210126060720.GJ740243@zeniv-ca \
--to=viro@zeniv.linux.org.uk \
--cc=gregkh@linuxfoundation.org \
--cc=hch@lst.de \
--cc=johannes@sipsolutions.net \
--cc=linux-kernel@vger.kernel.org \
--cc=ohw.giles@gmail.com \
--cc=torvalds@linux-foundation.org \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).