mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: John Ogness <john.ogness@linutronix.de>
To: Jiri Slaby <jirislaby@kernel.org>,
	"Maciej W. Rozycki" <macro@orcam.me.uk>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Petr Mladek <pmladek@suse.com>,
	Sergey Senozhatsky <senozhatsky@chromium.org>,
	Steven Rostedt <rostedt@goodmis.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Esben Haabendal <esben@geanix.com>,
	linux-serial@vger.kernel.org, linux-kernel@vger.kernel.org,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	Rengarajan S <rengarajan.s@microchip.com>,
	Jeff Johnson <quic_jjohnson@quicinc.com>,
	Serge Semin <fancer.lancer@gmail.com>,
	Lino Sanfilippo <l.sanfilippo@kunbus.com>,
	Wander Lairson Costa <wander@redhat.com>
Subject: Re: [PATCH tty-next v3 1/6] serial: 8250: Adjust the timeout for FIFO mode
Date: Mon, 04 Nov 2024 15:19:18 +0106	[thread overview]
Message-ID: <84ldxzccjl.fsf@jogness.linutronix.de> (raw)
In-Reply-To: <2fab2ef8-d0d6-4b94-90b6-7c16641a2f68@kernel.org>

On 2024-11-04, Jiri Slaby <jirislaby@kernel.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

  reply	other threads:[~2024-11-04 14:13 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-10-25 10:57 [PATCH tty-next v3 0/6] convert 8250 to nbcon John Ogness
2024-10-25 10:57 ` [PATCH tty-next v3 1/6] serial: 8250: Adjust the timeout for FIFO mode John Ogness
2024-10-25 13:45   ` Andy Shevchenko
2024-10-25 13:51     ` Andy Shevchenko
2024-10-29 16:24     ` Wander Lairson Costa
2024-10-30  6:05   ` Jiri Slaby
2024-10-31  4:44     ` Maciej W. Rozycki
2024-10-31  8:49       ` John Ogness
2024-11-01  1:24         ` Maciej W. Rozycki
2024-11-01  8:21           ` Andy Shevchenko
2024-11-04  6:44         ` Jiri Slaby
2024-11-04  6:34       ` Jiri Slaby
2024-11-04 14:13         ` John Ogness [this message]
2024-12-02  6:12           ` Jiri Slaby
2024-12-02 16:41             ` John Ogness
2024-12-01  0:04         ` Maciej W. Rozycki
2024-10-25 10:57 ` [PATCH tty-next v3 2/6] serial: 8250: Use high-level write function for FIFO John Ogness
2024-10-25 13:50   ` Andy Shevchenko
2024-11-05 16:12   ` Petr Mladek
2024-10-25 10:57 ` [PATCH tty-next v3 3/6] serial: 8250: Split out rx stop/start code into helpers John Ogness
2024-10-25 13:55   ` Andy Shevchenko
2024-11-06 10:54   ` Petr Mladek
2024-10-25 10:57 ` [PATCH tty-next v3 4/6] serial: 8250: Specify console context for rs485_start/stop_tx John Ogness
2024-10-25 14:04   ` Andy Shevchenko
2024-10-25 14:25     ` John Ogness
2024-10-25 14:34       ` Andy Shevchenko
2024-10-30  6:13   ` Jiri Slaby
2024-10-31  9:13     ` John Ogness
2024-11-06 15:42   ` Petr Mladek
2024-11-29 17:45     ` John Ogness
2024-10-25 10:57 ` [PATCH tty-next v3 5/6] serial: 8250: Switch to nbcon console John Ogness
2024-10-25 14:22   ` Andy Shevchenko
2024-10-28 13:22     ` John Ogness
2024-11-07  9:48       ` Petr Mladek
2024-10-30  6:33   ` Jiri Slaby
2024-10-31  9:25     ` John Ogness
2024-10-25 10:57 ` [PATCH tty-next v3 6/6] serial: 8250: Revert "drop lockdep annotation from serial8250_clear_IER()" John Ogness
2024-10-25 14:05   ` Andy Shevchenko
2024-10-25 13:58 ` [PATCH tty-next v3 0/6] convert 8250 to nbcon Andy Shevchenko

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=84ldxzccjl.fsf@jogness.linutronix.de \
    --to=john.ogness@linutronix.de \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=esben@geanix.com \
    --cc=fancer.lancer@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=jirislaby@kernel.org \
    --cc=l.sanfilippo@kunbus.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-serial@vger.kernel.org \
    --cc=macro@orcam.me.uk \
    --cc=pmladek@suse.com \
    --cc=quic_jjohnson@quicinc.com \
    --cc=rengarajan.s@microchip.com \
    --cc=rostedt@goodmis.org \
    --cc=senozhatsky@chromium.org \
    --cc=tglx@linutronix.de \
    --cc=wander@redhat.com \
    /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

all inboxes | Powered by JetHome®