Hi
Below version is tested with 3.18 kernel on 16550 UART.
- Added a new variable 'writing_to_con' to 'struct uart_8250_port'.
This would be set to 1 inside serial8250_console_write() only if
port_sysrq is 0 and oops_in_progress is 0 and "receive interrupts" are
enabled.
- uart_console_write() is called to send out whole printk message as
earlier. wait_for_xmitr() invokes serial8250_rx_chars() only if
writing_to_con is set and if UART_LSR_DR is set. so instead of waiting
with usleep(1) it now buffers chars from receive queue.
- if serial_8250_chars() is called with writing_to_con=1,
serial8250_rx_chars() would read one char from UART rx buffer.
- as long as serial8250_rx_chars() does not see brk i.e., sysrq is not
set, it could buffer. if we split this functionality, driver would have
to buffer each char and flags associated with it. with this approach,
serial8250_rx_chars() will buffer "regular" chars but anything after
sysrq is left to be processed after serial8250_console_write()
returns.
Thanks.
>From 46d5da6069296132d3e9f505715ab7f30701b2a5 Mon Sep 17 00:00:00 2001
From: Prasad Koya <prasad@xxxxxxxxxx>
Date: Tue, 9 Dec 2014 22:53:31 -0800
Subject: [PATCH 1/1] tty, serial, 8250: to avoid recv fifo overrun, read rx
fifo during xmit
At slow baud rates, say 9600, it is easy to run into tty buffer overrun
as interrupts could get disabled for a long time. To avoid that, if
UART_LSR_DR is set, buffer char from rx fifo. Stop buffering if
port->sysrq is true. wait_for_xmitr() could pick up a byte from FIFO
(if UART_LSR_DR is set) rather than calling udelay(1).
Signed-off-by: Prasad Koya <prasad@xxxxxxxxxx>
---
drivers/tty/serial/8250/8250_core.c | 26 ++++++++++++++++++++++++--
include/linux/serial_8250.h | 2 ++
2 files changed, 26 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_core.c
b/drivers/tty/serial/8250/8250_core.c
index ca5cfdc..b3d3eda 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -1422,7 +1422,7 @@ serial8250_rx_chars(struct uart_8250_port *up,
unsigned char lsr)
{
struct uart_port *port = &up->port;
unsigned char ch;
- int max_count = 256;
+ int max_count = up->writing_to_con ? 1 : 256;
char flag;
do {
@@ -1482,6 +1482,12 @@ serial8250_rx_chars(struct uart_8250_port *up,
unsigned char lsr)
uart_insert_char(port, lsr, UART_LSR_OE, ch, flag);
ignore_char:
+ /*
+ * We see BRK while buffering rx bytes. Stop buffering and
+ * let SysRq or SAK handling be done after console_write
+ */
+ if (port->sysrq && up->writing_to_con)
+ break;
lsr = serial_in(up, UART_LSR);
} while ((lsr & (UART_LSR_DR | UART_LSR_BI)) && (max_count-- > 0));
spin_unlock(&port->lock);
@@ -1959,7 +1965,15 @@ static void wait_for_xmitr(struct
uart_8250_port *up, int bits)
break;
if (--tmout == 0)
break;
- udelay(1);
+ /*
+ * We are writing to console with interrupts disabled.
+ * read recv FIFO to avoid buffer overrun
+ */
+ if (up->writing_to_con && (status & UART_LSR_DR))
+ serial8250_rx_chars(up, status);
+ else
+ udelay(1);
+
}
/* Wait up to 1s for flow control if necessary */
@@ -3102,6 +3116,7 @@ static void __init serial8250_isa_init_ports(void)
init_timer(&up->timer);
up->timer.function = serial8250_timeout;
up->cur_iotype = 0xFF;
+ up->writing_to_con = 0;
/*
* ALPHA_KLUDGE_MCR needs to be killed.
@@ -3213,8 +3228,15 @@ serial8250_console_write(struct console *co,
const char *s, unsigned int count)
else
serial_port_out(port, UART_IER, 0);
+ /* Is UART configured to send interrupts for received data */
+ if (((ier & UART_IER_RDI) == UART_IER_RDI) && !oops_in_progress &&
+ !port->sysrq)
+ up->writing_to_con = 1;
+
uart_console_write(port, s, count, serial8250_console_putchar);
+ if (up->writing_to_con)
+ up->writing_to_con = 0;
/*
* Finally, wait for transmitter to become empty
* and restore the IER
diff --git a/include/linux/serial_8250.h b/include/linux/serial_8250.h
index 3df10d5..1ad79e2 100644
--- a/include/linux/serial_8250.h
+++ b/include/linux/serial_8250.h
@@ -96,6 +96,8 @@ struct uart_8250_port {
#define MSR_SAVE_FLAGS UART_MSR_ANY_DELTA
unsigned char msr_saved_flags;
+ /* Write to console on this port now */
+ unsigned char writing_to_con;
struct uart_8250_dma *dma;
struct serial_rs485 rs485;
On Fri, Oct 3, 2014 at 5:57 AM, Peter Hurley <peter@xxxxxxxxxxxxxxxxxx> wrote:
> Hi Prasad,
>
> On 09/30/2014 01:30 PM, Prasad Koya wrote:
>> Hi Ted
>>
>> At 9600 baud, we are seeing buffer overruns on our 16550A UARTs. If a
>> command, say 40 chars long, is sent to console and if printk to
>> console happens at same time, we running into receive buffer overrun.
>> Thats because, at 9600 baud, it takes about a millisecond to xmit 1
>> char and with receive FIFO only 16 chars deep, it is very easy to
>> cause receive buffer overrun.
>>
>> To get around that, I am trying below patch on 3.17 kernel. This is
>> tested and working on 3.4 kernel. I wanted to get some comments from
>> linux-serial mailing list.
>>
>> If UART_IER_RDI is enabled, for every quarter of fifo size bytes sent,
>> console_write() checks to see if there is data in receive fifo. If so,
>> it lets serial8250_rx_chars() pick those bytes and buffer them in tty
>> layer.
>
> The issue here occurs irrespective of whether a UART has a FIFO; it's
> even easier to get an overrun without a FIFO.
>
>> --- linux-3.17rc6-orig/linux-3.17-rc6/drivers/tty/serial/8250/8250_core.c
>> 2014-09-21 15:43:02.000000000 -0700
>> +++ linux-3.17-rc6/drivers/tty/serial/8250/8250_core.c 2014-09-30
>> 10:08:06.401242782 -0700
>> @@ -3004,6 +3004,7 @@
>> unsigned long flags;
>> unsigned int ier;
>> int locked = 1;
>> + int to_send, offset, fifo = count;
>>
>> touch_nmi_watchdog();
>>
>> @@ -3022,8 +3023,27 @@
>> else
>> serial_port_out(port, UART_IER, 0);
>>
>> - uart_console_write(port, s, count, serial8250_console_putchar);
>> + offset = 0;
>> + if (uart_config[up->port.type].fcr & UART_FCR_ENABLE_FIFO) {
>> + if (!oops_in_progress && ((ier & UART_IER_RDI) == UART_IER_RDI))
>> + fifo = port->fifosize >> 2;
>> + }
>> +
>> +send_rest:
>> + to_send = count > fifo ? fifo : count;
>>
>> + uart_console_write(port, s + offset, to_send,
>> serial8250_console_putchar);
>> +
>> + count -= to_send;
>> + if (count > 0) {
>> + unsigned int status;
>> + status = serial_in(up, UART_LSR);
>> + if (status & (UART_LSR_DR | UART_LSR_BI))
>> + serial8250_rx_chars(up, status);
>
> This will potentially take up to 256 chars in a loop, all with interrupts
> still off, and then drops the port lock which isn't ok in the context of
> a console message (with the way the driver works now).
>
> serial8250_rx_chars() doesn't need to drop the port lock anymore; so that
> solves that problem.
>
> Also, serial8250_rx_chars() is where sysrq and SAK handling is done;
> this would allow it to be re-entered, which would be bad. So rx cannot/
> should not be happening if either oops_in_progress or port->sysrq is true.
>
> I think a better way to do this would be to train wait_for_xmitr() to
> receive data to a temp buffer (if bits is set to the requisite LSR bits and
> not if oops_in_progress or port->sysrq). serial8250_rx_chars() would
> need refactoring to split out the per-char handling so it could work on
> the temp buffer once the console message was written.
>
> Regards,
> Peter Hurley
>
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
