[PATCH] serial: sh-sci: prevent lockup on full TTY buffers

[PATCH] serial: sh-sci: prevent lockup on full TTY buffers

[Ulrich Hecht]

When the TTY buffers fill up to the configured maximum, a system lockup
occurs:

[  598.820128] INFO: rcu_preempt detected stalls on CPUs/tasks:
[  598.825796]  0-...!: (1 GPs behind) idle=5a6/2/0 softirq=1974/1974 fqs=1
[  598.832577]  (detected by 3, t=62517 jiffies, g=296, c=295, q=126)
[  598.838755] Task dump for CPU 0:
[  598.841977] swapper/0       R  running task        0     0      0 0x00000022
[  598.849023] Call trace:
[  598.851476]  __switch_to+0x98/0xb0
[  598.854870]            (null)

This can be prevented by doing a dummy read of the RX data register.

This issue affects both HSCIF and SCIF ports. Reported for R-Car H3 ES2.0;
reproduced and fixed on H3 ES1.1. Probably affects other R-Car platforms
as well.

Reported-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
---
 drivers/tty/serial/sh-sci.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index d9f399c..db2ebec 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -885,6 +885,8 @@ static void sci_receive_chars(struct uart_port *port)
 		/* Tell the rest of the system the news. New characters! */
 		tty_flip_buffer_push(tport);
 	} else {
+		/* TTY buffers full; read from RX reg to prevent lockup */
+		serial_port_in(port, SCxRDR);
 		serial_port_in(port, SCxSR); /* dummy read */
 		sci_clear_SCxSR(port, SCxSR_RDxF_CLEAR(port));
 	}
-- 
2.7.4

Re: [PATCH] serial: sh-sci: prevent lockup on full TTY buffers

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 215 bytes --]


> This can be prevented by doing a dummy read of the RX data register.

Just so I understand the issue correctly: We are reading the register to
throw the content away to prevent it being used in the TTY buffers?


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] serial: sh-sci: prevent lockup on full TTY buffers

[Ulrich Hecht]

On Thu, Feb 15, 2018 at 2:12 PM, Wolfram Sang <wsa@the-dreams.de> wrote:
>
>> This can be prevented by doing a dummy read of the RX data register.
>
> Just so I understand the issue correctly: We are reading the register to
> throw the content away to prevent it being used in the TTY buffers?

Not quite. The problem was that if the buffers are full,
sci_receive_chars() returned immediately without reading anything from
the data register, and that led to a lockup. I am not fully sure why
that is so (I arrived at the fix by examining how the different code
paths look from the serial controller's perspective), but dropping
data here fixes it. At this point buffers are full, so any data
received will have to be discarded anyway.

CU
Uli

Re: [PATCH] serial: sh-sci: prevent lockup on full TTY buffers

[Geert Uytterhoeven]

Hi Uli,

On Fri, Feb 16, 2018 at 3:27 PM, Ulrich Hecht
<ulrich.hecht+renesas@gmail.com> wrote:
> On Thu, Feb 15, 2018 at 2:12 PM, Wolfram Sang <wsa@the-dreams.de> wrote:
>>> This can be prevented by doing a dummy read of the RX data register.
>>
>> Just so I understand the issue correctly: We are reading the register to
>> throw the content away to prevent it being used in the TTY buffers?
>
> Not quite. The problem was that if the buffers are full,
> sci_receive_chars() returned immediately without reading anything from
> the data register, and that led to a lockup. I am not fully sure why
> that is so (I arrived at the fix by examining how the different code
> paths look from the serial controller's perspective), but dropping
> data here fixes it. At this point buffers are full, so any data
> received will have to be discarded anyway.

Do you get an interrupt storm, from the receive or overrun interrupt?

Anyway, the patch makes sense to me, so
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] serial: sh-sci: prevent lockup on full TTY buffers

[Greg KH]

On Thu, Feb 15, 2018 at 01:02:27PM +0100, Ulrich Hecht wrote:
> When the TTY buffers fill up to the configured maximum, a system lockup
> occurs:
> 
> [  598.820128] INFO: rcu_preempt detected stalls on CPUs/tasks:
> [  598.825796]  0-...!: (1 GPs behind) idle=5a6/2/0 softirq=1974/1974 fqs=1
> [  598.832577]  (detected by 3, t=62517 jiffies, g=296, c=295, q=126)
> [  598.838755] Task dump for CPU 0:
> [  598.841977] swapper/0       R  running task        0     0      0 0x00000022
> [  598.849023] Call trace:
> [  598.851476]  __switch_to+0x98/0xb0
> [  598.854870]            (null)
> 
> This can be prevented by doing a dummy read of the RX data register.
> 
> This issue affects both HSCIF and SCIF ports. Reported for R-Car H3 ES2.0;
> reproduced and fixed on H3 ES1.1. Probably affects other R-Car platforms
> as well.
> 
> Reported-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>

Should this go to older kernel versions as well?
If so, how far back?

thanks,

greg k-h

Re: [PATCH] serial: sh-sci: prevent lockup on full TTY buffers

[Geert Uytterhoeven]

Hi Greg,

On Fri, Feb 16, 2018 at 7:44 PM, Greg KH <greg@kroah.com> wrote:
> On Thu, Feb 15, 2018 at 01:02:27PM +0100, Ulrich Hecht wrote:
>> When the TTY buffers fill up to the configured maximum, a system lockup
>> occurs:
>>
>> [  598.820128] INFO: rcu_preempt detected stalls on CPUs/tasks:
>> [  598.825796]  0-...!: (1 GPs behind) idle=5a6/2/0 softirq=1974/1974 fqs=1
>> [  598.832577]  (detected by 3, t=62517 jiffies, g=296, c=295, q=126)
>> [  598.838755] Task dump for CPU 0:
>> [  598.841977] swapper/0       R  running task        0     0      0 0x00000022
>> [  598.849023] Call trace:
>> [  598.851476]  __switch_to+0x98/0xb0
>> [  598.854870]            (null)
>>
>> This can be prevented by doing a dummy read of the RX data register.
>>
>> This issue affects both HSCIF and SCIF ports. Reported for R-Car H3 ES2.0;
>> reproduced and fixed on H3 ES1.1. Probably affects other R-Car platforms
>> as well.
>>
>> Reported-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
>> Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
>
> Should this go to older kernel versions as well?
> If so, how far back?

This code path dates back to full-history-linux commit 2898a0e08c6ffb63
("[PATCH] SH Merge") in 2.6.2, back in 2004.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

RE: [PATCH] serial: sh-sci: prevent lockup on full TTY buffers

[Yoshihiro Shimoda]

Hi,

> From: Ulrich Hecht, Sent: Thursday, February 15, 2018 9:02 PM
> 
> When the TTY buffers fill up to the configured maximum, a system lockup
> occurs:
> 
> [  598.820128] INFO: rcu_preempt detected stalls on CPUs/tasks:
> [  598.825796]  0-...!: (1 GPs behind) idle=5a6/2/0 softirq=1974/1974 fqs=1
> [  598.832577]  (detected by 3, t=62517 jiffies, g=296, c=295, q=126)
> [  598.838755] Task dump for CPU 0:
> [  598.841977] swapper/0       R  running task        0     0      0 0x00000022
> [  598.849023] Call trace:
> [  598.851476]  __switch_to+0x98/0xb0
> [  598.854870]            (null)
> 
> This can be prevented by doing a dummy read of the RX data register.
> 
> This issue affects both HSCIF and SCIF ports. Reported for R-Car H3 ES2.0;
> reproduced and fixed on H3 ES1.1. Probably affects other R-Car platforms
> as well.
> 
> Reported-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
> ---

Thank you for the patch. Our team tested this patch and the issue disappeared.
So,

Tested-by: Nguyen Viet Dung <dung.nguyen.aj@renesas.com>

Best regards,
Yoshihiro Shimoda