Re: Splicing to/from a tty

[Linus Torvalds]

On Mon, Jan 18, 2021 at 11:36 AM Linus Torvalds
<torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
>
> Of course, it probably would be really nice to try to convert
> tty_read() to use the same model that we have for tty_write(), and
> then make the ld->ops->read() function actually take a kernel buffer
> instead.
>
> I wonder how painful that would be.

Oh, that seems _much_ less painful than I thought it would be.

This is a COMPLETELY BROKEN patch that builds cleanly for me. I say
that it's completely broken for three reasons:

 (a) that extra "int dummy" argument is there purely to force build
errors if some user hasn't been converted.

 (b) I intentionally made the conversion truly stupid. It's not
"broken", but it needs cleanup for nasty variable names (ie things
like me changing that already badly named "b" variable to "kbp" to
again catch all users at build time)

 (c) the buffer handling in tty_read() is actively broken. Things like
HDLC rely on getting a proper buffer that can hold the whole packet,
and the max packet size is actually potentially quite big.

But the only real problem does seem to be HDLC, which has a default
maximum frame size of 4k, but it can be set all the way up to 64kB as
a module parameter.

So that tty_read() conversion in this patch is complete garbage, but I
really just wrote this to see how many painful places there are. And
there are not many.

The HDLC case could probably be done fairly well by

 - have a small on-stack buffer for the "small read" case

 - have a kmalloc() buffer that defaults to one page for bigger reads

 - grow the kmalloc() buffer if the ->read function returns -EOVERFLOW

Comments?

NOTE! This does *not* do the actual splice_read() implementation. No,
this literally is just the preparatory stage for that by starting the
whole "make the tty ldisc read path use a kernel buffer instead".

Anybody want to play with this? I'd suggest keeping that "dummy"
parameter around for a while - I did an allmodconfig build with this,
but if there are any architecture-specific non-x86-64 cases I wouldn't
have seen them.

             Linus

Attachment: patch
Description: Binary data