linux-kernel - Re: [PATCH v4 8/8] serial: qcom-geni: Rework TX in FIFO mode to fix hangs/lockups

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: <Znlp1_F1u-70D3QQ@hovoldconsulting.com>
Date: Mon, 24 Jun 2024 14:43:03 +0200
From: Johan Hovold <johan@...nel.org>
To: Douglas Anderson <dianders@...omium.org>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Jiri Slaby <jirislaby@...nel.org>,
	Yicong Yang <yangyicong@...ilicon.com>,
	Tony Lindgren <tony@...mide.com>,
	Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
	Johan Hovold <johan+linaro@...nel.org>,
	John Ogness <john.ogness@...utronix.de>,
	linux-arm-msm@...r.kernel.org,
	Bjorn Andersson <andersson@...nel.org>,
	Konrad Dybcio <konrad.dybcio@...aro.org>,
	Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>,
	Stephen Boyd <swboyd@...omium.org>, linux-serial@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	Uwe Kleine-König <u.kleine-koenig@...gutronix.de>,
	Rob Herring <robh@...nel.org>
Subject: Re: [PATCH v4 8/8] serial: qcom-geni: Rework TX in FIFO mode to fix
 hangs/lockups

On Mon, Jun 10, 2024 at 03:24:26PM -0700, Douglas Anderson wrote:
> The fact that the Qualcomm GENI hardware interface is based around
> "packets" is really awkward to fit into Linux's UART design.
> Specifically, in order to send bytes you need to start up a new
> "command" saying how many bytes you want to send and then you need to
> send all those bytes. Once you've committed to sending that number of
> bytes it's very awkward to change your mind and send fewer, especially
> if you want to do so without dropping bytes on the ground.
> 
> There may be a few cases where you might want to send fewer bytes than
> you originally expected:
> 1. You might want to interrupt the transfer with something higher
>    priority, like the kernel console or kdb.
> 2. You might want to enter system suspend.
> 3. The user might have killed the program that had queued bytes for
>    sending over the UART.
> 
> Despite this awkwardness the Linux driver has still tried to send
> bytes using large transfers. Whenever the driver started a new
> transfer it would look at the number of bytes in the OS's queue and
> start a transfer for that many. The idea of using larger transfers is
> that it should be more efficient. When you're in the middle of a large
> transfer you can get interrupted when the hardware FIFO is close to
> empty and add more bytes in. Whenever you get to the end of a transfer
> you have to wait until the transfer is totally done before you can add
> more bytes and, depending on interrupt latency, that can cause the
> UART to idle a bit.

As I mentioned last week, the slowdown from this is quite noticeable
(e.g. 25% slowdown at @115200), but this may be the price we need to pay
for correctness, at least temporarily.

An alternative might be to switch to using a 16 byte fifo. This should
reduce console latency even further, and may be able avoid the idling
UART penalty by continuing to use the watermark interrupt for refilling
the FIFO.

Johan