Two other, deeper issues with your patch:
1. It doesn't compile if SUPPORT_SYSRQ is not set.
2. The
spin_unlock(&port->lock);
tty_flip_buffer_push(&port->state->port);
spin_lock(&port->lock);
at the end of serial8250_rx_chars is risky during polled console
output. To start with, serial8250_console_write doesn't necessarily
lock the port in the first place! Fortunately, the condition
under which it might not (port->sysrq || oops_in_progress) is
exactly that during which input isn't processed.
Should the code instead just buffer to the flip_buffer and flip it when
returning from serial8250_console_write?
Looking at it, I'm not sure the spin_unlock is even required.
tty_flip_buffer_push(port)
-> tty_schedule_flip(port)
-> schedule_work(work)
-> queue_work(system_wq, work)
-> queue_work_on(WORK_CPU_UNBOUND, system_wq, work)
-> __queue_work()
and I don't see anything that needs the port unlocked.
Anyway, my suggestion to pass the flag down is illustrated in the following
(untested) patch. Also note the emergency_situation() function, which
ensures the logic is the same in the various locations.
diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index ca5cfdc..3a7af36 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -1416,9 +1416,13 @@ static void serial8250_enable_ms(struct uart_port *port)
* serial8250_rx_chars: processes according to the passed in LSR
* value, and returns the remaining LSR bits not handled
* by this Rx routine.
+ *
+ * no_sysrq is normally false, but if set indicates we should stop
+ * processing input after receiving a break, rather than passing the
+ * next character to uart_handle_sysrq_char.
*/
unsigned char
-serial8250_rx_chars(struct uart_8250_port *up, unsigned char lsr)
+serial8250_rx_chars(struct uart_8250_port *up, unsigned char lsr, bool no_sysrq)
{
struct uart_port *port = &up->port;
unsigned char ch;
@@ -1454,8 +1458,11 @@ serial8250_rx_chars(struct uart_8250_port *up, unsigned char lsr)
* may get masked by ignore_status_mask
* or read_status_mask.
*/
- if (uart_handle_break(port))
+ if (uart_handle_break(port)) {
+ if (no_sysrq)
+ max_count = 0;
goto ignore_char;
+ }
} else if (lsr & UART_LSR_PE)
port->icount.parity++;
else if (lsr & UART_LSR_FE)
@@ -1591,7 +1598,7 @@ int serial8250_handle_irq(struct uart_port *port, unsigned int iir)
dma_err = serial8250_rx_dma(up, iir);
if (!up->dma || dma_err)
- status = serial8250_rx_chars(up, status);
+ status = serial8250_rx_chars(up, status, false);
}
serial8250_modem_status(up);
if (!up->dma && (status & UART_LSR_THRE))
@@ -1943,9 +1950,30 @@ static void serial8250_break_ctl(struct uart_port *port, int break_state)
}
/*
+ * In some special situations, we allow console output without necessarily
+ * locking the port. The same situations also suppress processing input
+ * during polling output, because we aren't ready to deal with it.
+ * So just hope the hardware FIFO can hold until interrupts are
+ * re-enabled.
+ */
+static bool emergency_situation(const struct uart_port *port)
+{
+#ifdef SUPPORT_SYSRQ
+ return up->port.sysrq != 0 || oops_in_progress;
+#else
+ (void)port;
+ return oops_in_progress;
+#endif
+}
+
+/*
* Wait for transmitter & holding register to empty
+ * If rx is true, then also poll for received characters if it's
+ * safe to process them. This is only used in the SUPPORT_SYSRQ
+ * case, and hopefully the compiler can figure out it's dead code
+ * if that's not the case.
*/
-static void wait_for_xmitr(struct uart_8250_port *up, int bits)
+static void __wait_for_xmitr(struct uart_8250_port *up, int bits, bool rx)
{
unsigned int status, tmout = 10000;
@@ -1959,6 +1987,9 @@ static void wait_for_xmitr(struct uart_8250_port *up, int bits)
break;
if (--tmout == 0)
break;
+ if (rx && status & (UART_LSR_DR | UART_LSR_BI) &&
+ !emergency_situation(&up->port))
+ serial8250_rx_chars(up, status, true);
udelay(1);
}
@@ -1970,12 +2001,24 @@ static void wait_for_xmitr(struct uart_8250_port *up, int bits)
up->msr_saved_flags |= msr & MSR_SAVE_FLAGS;
if (msr & UART_MSR_CTS)
break;
+ if (rx && !emergency_situation(&up->port)) {
+ status = serial_in(up, UART_LSR);
+
+ up->lsr_saved_flags |= status & LSR_SAVE_FLAGS;
+ if (status & (UART_LSR_DR | UART_LSR_BI))
+ serial8250_rx_chars(up, status, true);
+ }
udelay(1);
touch_nmi_watchdog();
}
}
}
+static void wait_for_xmitr(struct uart_8250_port *up, int bits)
+{
+ __wait_for_xmitr(up, bits, false);
+}
+
#ifdef CONFIG_CONSOLE_POLL
/*
* Console polling routines for writing and reading from the uart while
@@ -3175,7 +3218,7 @@ static void serial8250_console_putchar(struct uart_port *port, int ch)
{
struct uart_8250_port *up = up_to_u8250p(port);
- wait_for_xmitr(up, UART_LSR_THRE);
+ __wait_for_xmitr(up, UART_LSR_THRE, true);
serial_port_out(port, UART_TX, ch);
}
@@ -3183,6 +3226,10 @@ static void serial8250_console_putchar(struct uart_port *port, int ch)
* Print a string to the serial port trying not to disturb
* any possible real use of the port...
*
+ * In order to avoid dropped input if the output is longer than
+ * the available hardware FIFO space, we try to handle incoming
+ * characters during this polling loop.
+ *
* The console_lock must be held when we get here.
*/
static void
@@ -3198,7 +3245,7 @@ serial8250_console_write(struct console *co, const char *s, unsigned int count)
serial8250_rpm_get(up);
- if (port->sysrq || oops_in_progress)
+ if (emergency_situation(port))
locked = spin_trylock_irqsave(&port->lock, flags);
else
spin_lock_irqsave(&port->lock, flags);
@@ -3219,7 +3266,7 @@ serial8250_console_write(struct console *co, const char *s, unsigned int count)
* Finally, wait for transmitter to become empty
* and restore the IER
*/
- wait_for_xmitr(up, BOTH_EMPTY);
+ __wait_for_xmitr(up, BOTH_EMPTY, true);
serial_port_out(port, UART_IER, ier);
/*
diff --git a/drivers/tty/serial/8250/8250_fsl.c b/drivers/tty/serial/8250/8250_fsl.c
index c0533a5..25f0aa3 100644
--- a/drivers/tty/serial/8250/8250_fsl.c
+++ b/drivers/tty/serial/8250/8250_fsl.c
@@ -49,7 +49,7 @@ int fsl8250_handle_irq(struct uart_port *port)
lsr = orig_lsr = up->port.serial_in(&up->port, UART_LSR);
if (lsr & (UART_LSR_DR | UART_LSR_BI))
- lsr = serial8250_rx_chars(up, lsr);
+ lsr = serial8250_rx_chars(up, lsr, false);
serial8250_modem_status(up);
diff --git a/include/linux/serial_8250.h b/include/linux/serial_8250.h
index 3df10d5..8735c9c 100644
--- a/include/linux/serial_8250.h
+++ b/include/linux/serial_8250.h
@@ -131,7 +131,8 @@ extern void serial8250_do_pm(struct uart_port *port, unsigned int state,
unsigned int oldstate);
extern int fsl8250_handle_irq(struct uart_port *port);
int serial8250_handle_irq(struct uart_port *port, unsigned int iir);
-unsigned char serial8250_rx_chars(struct uart_8250_port *up, unsigned char lsr);
+unsigned char serial8250_rx_chars(struct uart_8250_port *up, unsigned char lsr,
+ bool no_sysrq);
void serial8250_tx_chars(struct uart_8250_port *up);
unsigned int serial8250_modem_status(struct uart_8250_port *up);
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 21c2e05..2739fac 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -389,7 +389,7 @@ uart_handle_sysrq_char(struct uart_port *port, unsigned int ch)
if (port->sysrq) {
if (ch && time_before(jiffies, port->sysrq)) {
handle_sysrq(ch);
- port->sysrq = 0;
+ port->sysrq = 0; /* Must be AFTER handle_sysrq */
return 1;
}
port->sysrq = 0;
--
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
