linux-kernel - Re: [PATCH tty-next v3 1/6] serial: 8250: Adjust the timeout for FIFO mode

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <84ldxzccjl.fsf@jogness.linutronix.de>
Date: Mon, 04 Nov 2024 15:19:18 +0106
From: John Ogness <john.ogness@...utronix.de>
To: Jiri Slaby <jirislaby@...nel.org>, "Maciej W. Rozycki" <macro@...am.me.uk>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>, Petr Mladek
 <pmladek@...e.com>, Sergey Senozhatsky <senozhatsky@...omium.org>, Steven
 Rostedt <rostedt@...dmis.org>, Thomas Gleixner <tglx@...utronix.de>, Esben
 Haabendal <esben@...nix.com>, linux-serial@...r.kernel.org,
 linux-kernel@...r.kernel.org, Andy Shevchenko
 <andriy.shevchenko@...ux.intel.com>, Rengarajan S
 <rengarajan.s@...rochip.com>, Jeff Johnson <quic_jjohnson@...cinc.com>,
 Serge Semin <fancer.lancer@...il.com>, Lino Sanfilippo
 <l.sanfilippo@...bus.com>, Wander Lairson Costa <wander@...hat.com>
Subject: Re: [PATCH tty-next v3 1/6] serial: 8250: Adjust the timeout for
 FIFO mode

On 2024-11-04, Jiri Slaby <jirislaby@...nel.org> wrote:
> Instead of looping fifosize multiplied by random timeout, can we
> re-use port->frame_time?

Rather than 10k loops, we could loop

	(port->frame_time * some_scaled_padding) / 1000

times. The padding is important because we should not timeout in the
normal scenario. Perhaps using ~2 as @some_padding. Something like:

	port->frame_time >> 9

?

>>   The difference between THRE and TEMT is the state of the shift register
>> only[2]:
>> 
>> "In the FIFO mode, TEMT is set when the transmitter FIFO and shift
>> register are both empty."
>
> But what's the purpose of spinning _here_? The kernel can run and
> FIFO too. Without the kernel waiting for the FIFO.
>
> If we want to wait for fifo to empty, why not *also* the TSR. Meaning:
>
> Did you want UART_LSR_TEMT?

Let us assume we have a line with 640 characters and a FIFO of 64
bytes. For this line, we must wait for the FIFO to empty 10 times. It is
enough to wait for THRE for each of the 64-byte blocks because we are
only interested in refilling the FIFO at this point. Only at the very
end (in the caller...  serial8250_console_write()) do we need to wait
for everything to flush to the wire (TEMT).

By waiting on TEMT for each of the 64-byte blocks, we are waiting longer
than necessary. This creates a small window where the FIFO is empty and
there is nothing being transmitted.

I did a simple test on my beaglebone-black hardware, sending 100 lines
of 924 bytes at 9600 bps. Since my hardware uses a 64-byte FIFO, each
line would have 14 such windows.

And indeed, waiting for TEMT rather than only THRE for the 64-byte
blocks resulted in an extra 30ms total transfer for all 92400
bytes. That is about 20us lost in each window by unnecessarily waiting
for TEMT.

Of course, we are only talking about console output, which is horribly
inefficient on system resources. But I would argue, if we do not care
about unnecessary waiting, then why even have the FIFO optimization in
the first place?

John