Re: [PATCH] serial: imx: reduce RX interrupt frequency

From: Sergey Organov <sorganov@gmail.com>
To: Marc Kleine-Budde <mkl@pengutronix.de>
Cc: "Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	"Fabio Estevam" <festevam@gmail.com>,
	linux-serial@vger.kernel.org, "Jiri Slaby" <jirislaby@kernel.org>,
	"Tomasz Moń" <tomasz.mon@camlingroup.com>,
	k.drobinski@camlintechnologies.com,
	"NXP Linux Team" <linux-imx@nxp.com>,
	"Pengutronix Kernel Team" <kernel@pengutronix.de>,
	"Shawn Guo" <shawnguo@kernel.org>,
	"Sascha Hauer" <s.hauer@pengutronix.de>
Subject: Re: [PATCH] serial: imx: reduce RX interrupt frequency
Date: Wed, 05 Jan 2022 17:37:39 +0300	[thread overview]
Message-ID: <87iluy45ak.fsf@osv.gnss.ru> (raw)
In-Reply-To: <20220105134059.rcvegrjghgm7ryeh@pengutronix.de> (Marc Kleine-Budde's message of "Wed, 5 Jan 2022 14:40:59 +0100")

Marc Kleine-Budde <mkl@pengutronix.de> writes:

> On 05.01.2022 16:37:22, Sergey Organov wrote:
>> Marc Kleine-Budde <mkl@pengutronix.de> writes:
>> 
>> > On 05.01.2022 16:00:34, Sergey Organov wrote:
>> >> Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
>> >> 
>> >> > On Tue, Jan 04, 2022 at 11:32:03AM +0100, Tomasz Moń wrote:
>> >> >> Triggering RX interrupt for every byte defeats the purpose of aging
>> >> >> timer and leads to interrupt starvation at high baud rates.
>> >> >> 
>> >> >> Increase receiver trigger level to 8 to increase the minimum period
>> >> >> between RX interrupts to 8 characters time. The tradeoff is increased
>> >> >> latency. In the worst case scenario, where RX data has intercharacter
>> >> >> delay slightly less than aging timer (8 characters time), it can take
>> >> >> up to 63 characters time for the interrupt to be raised since the
>> >> >> reception of the first character.
>> >> >
>> >> > 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?
>> >> 
>> >> I don't like the idea of dynamic threshold as I don't think increased
>> >> complexity is worth it.
>> >> 
>> >> In fact the threshold works "properly" on any baud rate, as maximum
>> >> latency is proportional to the current baud rate, and if somebody does
>> >> care about *absolute* latency, increasing baud rate is the primary
>> >> solution.
>> >
>> > Nope - this only works if you have both sides under control.... Which is
>> > not the case in our $CUSTROMER's use case.
>> 
>> Yep, if one can't use primary solution, they need to come up with
>> something else.
>
> Please don't break existing use cases while improving the kernel.

If we aim at strict backward compatibility, the default value of water
level mark should not be changed, and I'm afraid it can't be changed at
any baud rate that is currently supported, as we can't be sure nobody
uses that feature of being "low latency" at any given baud rate.

>
>> Where is that historical "low-latency" bit, by the
>> way?
>
> ...has been removed:
>
> https://lore.kernel.org/all/20210105120239.28031-11-jslaby@suse.cz/
>
> Is there an option to bring that back?

It had different meaning as far as I recall, but as a way out of current
situation, I think something similar could be introduced as an
additional bit for tty parameters structure that is "true" by default,
meaning "minimize latency", and can be set to "false" through ioctl().

Alternatively, we might want to introduce "threshold baud rate"
parameter for tty, for drivers to be free to set high watermarks above
that baud rate.

Thanks,
-- Sergey Organov

>
> Marc