Skip to contentSkip to conversation

serial: imx: reduce irq-latency after rx overflow

PATCH In review Manfred Schlaegl 2 participants 4 messages Jun 20, 2015 – Jun 22, 2015

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 serial: imx: reduce irq-latency after rx overflow

To prevent problems with interrupt latency, and due to the fact, that the error will be counted anyway (icount.overrun), the dev_err is simply removed.

Background:
If an rx-fifo overflow occurs a dev_err message was called in interrupt context. Since dev_err messages are written to console in a synchronous way (unbuffered), and console may be a serial terminal, this leads to a highly increased interrupt-latency (several milliseconds).
As a result of the high latency more rx-fifo overflows will happen, and therefore a feedback loop of errors is created.

---
 drivers/tty/serial/imx.c |    1 -
 1 file changed, 1 deletion(-)
drivers/tty/serial/imx.c +0 -1
index 384cf1d..40fd32c 100644
@@ -767,7 +767,6 @@ static irqreturn_t imx_int(int irq, void *dev_id)
 		writel(USR1_AWAKE, sport->port.membase + USR1);
 
 	if (sts2 & USR2_ORE) {
-		dev_err(sport->port.dev, "Rx FIFO overrun\n");
 		sport->port.icount.overrun++;
 		writel(USR2_ORE, sport->port.membase + USR2);
 	}
Signature · 1 line
1.7.10.4
#1 lore raw

Am Samstag, 20. Juni 2015, 19:25:52 schrieb Manfred Schlaegl:

Quoted 11 lines

To prevent problems with interrupt latency, and due to the fact, that the error will be counted anyway (icount.overrun), the dev_err is simply removed.

Background:
If an rx-fifo overflow occurs a dev_err message was called in interrupt context. Since dev_err messages are written to console in a synchronous way (unbuffered), and console may be a serial terminal, this leads to a highly increased interrupt-latency (several milliseconds).
As a result of the high latency more rx-fifo overflows will happen, and therefore a feedback loop of errors is created.

I understand your rationale but removing this error message from kernel log removes the possibility to detect serial overruns by simply check the kernel log or output on kernel console. AFAICS you have to use TIOCGICOUNT to get the error counters. How about introducing a rate limit for this kernel message?

Best regards,
Alexander

Signature · 12 lines
Dipl.-Inf. Alexander Stein

SYS TEC electronic GmbH
Am Windrad 2
08468 Heinsdorfergrund
Tel.: 03765 38600-1156
Fax: 03765 38600-4100
Email: [email protected]
Website: www.systec-electronic.com
 
Managing Director: Dipl.-Phys. Siegmar Schmidt
Commercial registry: Amtsgericht Chemnitz, HRB 28082
#2 lore raw
Quoted 13 lines · Alexander Stein

Am Samstag, 20. Juni 2015, 19:25:52 schrieb Manfred Schlaegl:

Quoted 11 lines

To prevent problems with interrupt latency, and due to the fact, that the error will be counted anyway (icount.overrun), the dev_err is simply removed.

Background:
If an rx-fifo overflow occurs a dev_err message was called in interrupt context. Since dev_err messages are written to console in a synchronous way (unbuffered), and console may be a serial terminal, this leads to a highly increased interrupt-latency (several milliseconds).
As a result of the high latency more rx-fifo overflows will happen, and therefore a feedback loop of errors is created.

Quoted 3 lines

I understand your rationale but removing this error message from kernel log removes the possibility to detect serial overruns by simply check the kernel log or output on kernel console. AFAICS you have to use TIOCGICOUNT to get the error counters. How about introducing a rate limit for this kernel message?

Hello!

I understand your argument, but:
1. In my personal opinion kernel error messages should only be used on internal errors (missing resources, asserts, ...) and in cases where no other way is (yet) available to report errors (by counters, return values, ...). Lost RX bytes on uarts seem more like a communication error and should be silently handled by higher layers using error counters, or protocol internal mechanisms.
2. I have found no other serial driver (except serial-tegra and imx) that reports this kind of errors using kernel messages.
3. Error counters for serial interfaces can also be retrieved from userspace by using procfs -> implemented in serial_core; e.g. /proc/tty/driver/IMX-uart.

best regards,
manfred

#3 lore raw

Hello Manfred,

Quoted 18 lines · Manfred Schlaegl
Quoted 13 lines · Alexander Stein

Am Samstag, 20. Juni 2015, 19:25:52 schrieb Manfred Schlaegl:

Quoted 11 lines

To prevent problems with interrupt latency, and due to the fact, that the error will be counted anyway (icount.overrun), the dev_err is simply removed.

Background:
If an rx-fifo overflow occurs a dev_err message was called in interrupt context. Since dev_err messages are written to console in a synchronous way (unbuffered), and console may be a serial terminal, this leads to a highly increased interrupt-latency (several milliseconds).
As a result of the high latency more rx-fifo overflows will happen, and therefore a feedback loop of errors is created.

Quoted 3 lines

I understand your rationale but removing this error message from kernel log removes the possibility to detect serial overruns by simply check the kernel log or output on kernel console. AFAICS you have to use TIOCGICOUNT to get the error counters. How about introducing a rate limit for this kernel message?

Quoted 6 lines

Hello!

I understand your argument, but:
1. In my personal opinion kernel error messages should only be used on internal errors (missing resources, asserts, ...) and in cases where no other way is (yet) available to report errors (by counters, return values, ...). Lost RX bytes on uarts seem more like a communication error and should be silently handled by higher layers using error counters, or protocol internal mechanisms.
2. I have found no other serial driver (except serial-tegra and imx) that reports this kind of errors using kernel messages.
3. Error counters for serial interfaces can also be retrieved from userspace by using procfs -> implemented in serial_core; e.g. /proc/tty/driver/IMX-uart.

Ah, I've just noticed those errors will only be written when > 0. I think this is fine. A bit cumbersome for automatic parsing, but reading manually will be ok.

Best regards,
Alexander Stein

Signature · 12 lines
Dipl.-Inf. Alexander Stein

SYS TEC electronic GmbH
Am Windrad 2
08468 Heinsdorfergrund
Tel.: 03765 38600-1156
Fax: 03765 38600-4100
Email: [email protected]
Website: www.systec-electronic.com
 
Managing Director: Dipl.-Phys. Siegmar Schmidt
Commercial registry: Amtsgericht Chemnitz, HRB 28082