Messages in this thread | | | Date | Tue, 19 Jan 2021 12:53:58 +0100 | From | Greg Kroah-Hartman <> | Subject | Re: Splicing to/from a tty |
| |
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
| |