From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Christoph Hellwig <hch@lst.de>,
Oliver Giles <ohw.giles@gmail.com>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
Al Viro <viro@zeniv.linux.org.uk>,
Jiri Slaby <jirislaby@kernel.org>
Subject: Re: Splicing to/from a tty
Date: Tue, 19 Jan 2021 12:53:58 +0100 [thread overview]
Message-ID: <YAbIVgGt1Qz8ItMh@kroah.com> (raw)
In-Reply-To: <CAHk-=whW7t=3B=iCwYkJ3W-FH08wZNCFO7EJ5qQSqD9Z_tBxrQ@mail.gmail.com>
On Mon, Jan 18, 2021 at 05:38:55PM -0800, Linus Torvalds wrote:
> On Mon, Jan 18, 2021 at 2:20 PM Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
> >
> > So it's not a "real" patch, but with improved buffer handling in
> > tty_read(), I think this is actually quite close.
>
> Hmm.
>
> I somehow ended up working on this all because it's a Monday, and I
> don't see a lot of pull requests early in the week.
>
> And I think I have a solution for the HDLC "we may need to copy a
> packet that might be up to 64kB" issue, that isn't really all that
> ugly.
>
> We can just iterate over a random "cookie" that the line discipline
> can use any way it wants to. In the case of n_hdlc, it can just put
> the 'rbuf' thing it has into that cookie, and then it can copy it all
> piece-meal until it is all used up. And if it runs out of space in the
> middle, it will return -EOVERFLOW, and we're all good.
>
> The only other thing such a line discipline needs is the offset into
> the cookie, but the iterator has to maintain that anyway, so that's
> simple enough.
>
> So here's a fourth patch for this thing today, this time with what I
> think is actually a working model for the buffer handling.
>
> Other line disciplines *could* use the cookie if they want to. I
> didn't do any of that, though.
>
> The normal n_tty line discipline, for example, could easily just loop
> over the data. It doesn't need an offset or that 'rbuf' pointer, but
> it still needs to know whether the call is the first one or not,
> because the first time the n_tty line discipline is called it may need
> to wait for a minimum number of characters or whatever the termios
> settings say - but obviously once it has waited for it once, it
> shouldn't wait for it again the next time around (only on the next
> actual full read()). IOW, it would be wrong if the termios said "wait
> for 5 characters", and then it saw 68 characters, copied the first 64,
> in the first iteration, and than saw "oh, now there are only 4
> characters left so now I have to wait for a fifth".
>
> So n_tty could use the cookie purely to see whether it's the first
> iteration or not, and it could just set the cookie to a random value
> (it always starts out as NULL) to just show what state it is in.
>
> I did *NOT* do that, because it's not technically necessary - unlike
> the hdlc packet case, n_tty returning a partial result is not wrong
> per se even if we might have reasons to improve on it later.
>
> What do people think about this?
>
> Also, does anybody have any test-code for the HDLC case? I did find an
> interesting comment when doing a Debian code search:
>
> /* Bloody hell... readv doesn't work with N_HDLC line discipline... GRR! */
>
> and yes, this model would allow us to handle readv() properly for hdlc
> (and no, the old one did not, because it really wanted to see the
> whole packet in *one* user buffer).
>
> But I have no idea if hdlc is even relevant any more, and if anybody
> really cares.
>
> Anybody?
This looks sane, but I'm still missing what the goal of this is here.
It's nice from a "don't make the ldisc do the userspace copy", point of
view, but what is the next step in order to tie that into splice?
I ask as I also have reports that sysfs binary files are now failing for
this same reason, so I need to make the same change for them and it's
not excatly obvious what to do:
https://lore.kernel.org/r/1adf9aa4-ed7e-8f05-a354-57419d61ec18@codeaurora.org
thanks,
greg k-h
next prev parent reply other threads:[~2021-01-19 12:07 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 ` Splicing to/from a tty Al Viro
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 [this message]
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=YAbIVgGt1Qz8ItMh@kroah.com \
--to=gregkh@linuxfoundation.org \
--cc=hch@lst.de \
--cc=jirislaby@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=ohw.giles@gmail.com \
--cc=torvalds@linux-foundation.org \
--cc=viro@zeniv.linux.org.uk \
/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).