linux-serial.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Tomasz Moń" <tomasz.mon@camlingroup.com>
To: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>
Cc: linux-serial@vger.kernel.org, Jiri Slaby <jirislaby@kernel.org>,
	Fabio Estevam <festevam@gmail.com>,
	Sascha Hauer <s.hauer@pengutronix.de>,
	NXP Linux Team <linux-imx@nxp.com>,
	Pengutronix Kernel Team <kernel@pengutronix.de>,
	Shawn Guo <shawnguo@kernel.org>,
	k.drobinski@camlintechnologies.com
Subject: Re: [PATCH] serial: imx: reduce RX interrupt frequency
Date: Wed, 5 Jan 2022 08:59:09 +0100	[thread overview]
Message-ID: <4c48200b-cc2e-0766-a002-831a789d4879@camlingroup.com> (raw)
In-Reply-To: <20220104224900.u3omfbilejx2jawr@pengutronix.de>

On 04.01.2022 23:49, Uwe Kleine-König wrote:
> On Tue, Jan 04, 2022 at 12:38:01PM +0100, Greg Kroah-Hartman wrote:
>> On Tue, Jan 04, 2022 at 12:13:06PM +0100, Tomasz Moń wrote:
>>> On 04.01.2022 11:54, Greg Kroah-Hartman wrote:
>>>> Why can't you do this dynamically based on the baud rate so as to always
>>>> work properly for all speeds without increased delays for slower ones?
>>>
>>> Could you please advise on which baud rates to consider as slow? Does it
>>> sound good to have the old trigger level for rates up to and including
>>> 115200 and the new one for faster ones?
>>
>> You tell me, you are the one seeing this issue and are seeing delays on
>> slower values with your change.  Do some testing to see where the curve
>> is.

While the increased latency due to this change is undeniable, it is
important to note that latency is not everything. There are applications
where the latency is crucial, however using Linux for such applications
is questionable. Linux is not a Real Time Operating System after all.

Latency is simple to measure and argue based on the reception of single
character. That is only a corner case, not fully describing real world.
In the real world, it is important to consider the overall performance
improvement. It is hard to determine how much does the performance of
the system improve thanks to less time spent in interrupt handling.

> Maybe it's more sensible to make this even more dynamic: e.g. keep it at
> 1 as default and increase the water level when a certain irq frequency
> is reached?

The more I think about this, the dynamic part feels less reasonable,
i.e. all the extra complexity needed seems enormous compared to the
potential advantage.

I have hacked in a GPIO toggle at the end of interrupt handling. This
allows me to measure the latency increase with the oscilloscope in the
simple one character reception case. Latency here is defined as "time
between the stop bit and GPIO toggle". Note that this test completely
ignores the user space impact (process scheduling).

With RXTL being set to 1 (aging timer effectively disabled), latency is
within 10 to 20 us range. This time does not depend on the baud rate,
but on the system, i.e. cpu frequency, memory layout, caching, compiler
optimization level, other interrupts and so on.

With RXTL being set to 8, baud rate set to 1M and 10 bits per character
(8N1), latency is within 90 to 100 us range. The shift by approximately
80 us matches the expectations as it is directly induced by aging timer
(interrupt was raised only after aging timer expired).

When DMA is enabled, baud rate set to 1M and 10 bits per character
(8N1), latency is within 100 to 110 us range. This is somewhat expected
as the main latency contributing factor is the aging timer. The extra
few us is likely due to the DMA overhead (not really important in real
world where more than one byte is being transferred).

Usually serial transmission happens in various length bursts (frames).
For such transmissions the latency induced by the aging timer is
generally equal to 8 characters time (when the frame length is exact
multiple of RXTL there is no latency induced by the aging timer).

Worst case scenario is when the intercharacter interval is slighly less
than the aging timer timeout. While this is possible, in my opinion it
is highly unlikely. Aging timer latency upper bound, mentioned in commit
message, is equal to:
  (RXTL - 1) * (8 character time timeout + received 1 character time)

The real advantage of higher RXTL is avoiding interrupt storm. With RXTL
being set to 1, I am able to trigger interrupt storm at 1M and higher
baud rates when intercharacter interval is virtually zero. This is
expected because, as noted earlier, interrupt handling time is within 10
to 20 us range. As the interrupt handling time is more than the time it
takes to transmit single character (10 us), this inevitably leads to
line discipline workqueue starvation, and in turn to dropping the
characters, because eventually:
  * tty flip buffer cannot take any more characters,
  * tty throttle does not happen due to interrupt storm,
  * RX interrupt is fast enough to keep RxFIFO below autoRTS level.

With RXTL being set to 8, minimum RX interrupt period is 8 characters
time. This happens when there is continuous data being received (no
delay due to the aging timer).

If changing the default RXTL value does not sound right, then maybe RXTL
could be configured via a device tree property? That way it would be up
to the user to tune it for the application.

Best Regards,
Tomasz Mon


  reply	other threads:[~2022-01-05  7:59 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-04 10:32 [PATCH] serial: imx: reduce RX interrupt frequency Tomasz Moń
2022-01-04 10:43 ` Ahmad Fatoum
2022-01-04 11:05   ` Tomasz Moń
2022-01-04 10:54 ` Greg Kroah-Hartman
2022-01-04 11:13   ` Tomasz Moń
2022-01-04 11:38     ` Greg Kroah-Hartman
2022-01-04 22:49       ` Uwe Kleine-König
2022-01-05  7:59         ` Tomasz Moń [this message]
2022-01-05 10:37           ` Greg Kroah-Hartman
2022-01-05 10:56             ` Tomasz Moń
2022-01-05 10:57             ` Marc Kleine-Budde
2022-01-06 15:39               ` Sergey Organov
2022-01-05 13:34         ` Sergey Organov
2022-01-06 15:05           ` Uwe Kleine-König
2022-01-10  6:14             ` Tomasz Moń
2022-01-10  8:31               ` Uwe Kleine-König
2022-01-05 13:00   ` Sergey Organov
2022-01-05 13:04     ` Marc Kleine-Budde
2022-01-05 13:37       ` Sergey Organov
2022-01-05 13:40         ` Marc Kleine-Budde
2022-01-05 14:37           ` Sergey Organov

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=4c48200b-cc2e-0766-a002-831a789d4879@camlingroup.com \
    --to=tomasz.mon@camlingroup.com \
    --cc=festevam@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=jirislaby@kernel.org \
    --cc=k.drobinski@camlintechnologies.com \
    --cc=kernel@pengutronix.de \
    --cc=linux-imx@nxp.com \
    --cc=linux-serial@vger.kernel.org \
    --cc=s.hauer@pengutronix.de \
    --cc=shawnguo@kernel.org \
    --cc=u.kleine-koenig@pengutronix.de \
    /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).