On 03/11/2015 10:29 AM, Tim Kryger wrote: > On Wed, Mar 11, 2015 at 6:19 AM, Peter Hurley <peter@xxxxxxxxxxxxxxxxxx> wrote: >> Trying to write console output from within the serial console driver >> while the port->lock is held causes recursive deadlock: >> >> CPU 0 >> spin_lock_irqsave(&port->lock) >> printk() >> console_unlock() >> call_console_drivers() >> serial8250_console_write() >> spin_lock_irqsave(&port->lock) >> ** DEADLOCK ** >> >> The 8250_dw i/o accessors try to write a console error message if the >> LCR workaround was unsuccessful. When the port->lock is already held >> (eg., when called from serial8250_set_termios()), this deadlocks. >> >> Make the error message a FIXME until a general solution is devised. >> >> Cc: Tim Kryger <tim.kryger@xxxxxxxxx> >> Reported-by: Zhang Zhen <zhenzhang.zhang@xxxxxxxxxx> >> Signed-off-by: Peter Hurley <peter@xxxxxxxxxxxxxxxxxx> >> --- >> drivers/tty/serial/8250/8250_dw.c | 15 ++++++++++++--- >> 1 file changed, 12 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c >> index f7f5371..48a8bef 100644 >> --- a/drivers/tty/serial/8250/8250_dw.c >> +++ b/drivers/tty/serial/8250/8250_dw.c >> @@ -119,7 +119,10 @@ static void dw8250_serial_out(struct uart_port *p, int offset, int value) >> dw8250_force_idle(p); >> writeb(value, p->membase + (UART_LCR << p->regshift)); >> } >> - dev_err(p->dev, "Couldn't set LCR to %d\n", value); >> + /* >> + * FIXME: this deadlocks if port->lock is already held >> + * dev_err(p->dev, "Couldn't set LCR to %d\n", value); >> + */ >> } >> } >> >> @@ -163,7 +166,10 @@ static void dw8250_serial_outq(struct uart_port *p, int offset, int value) >> __raw_writeq(value & 0xff, >> p->membase + (UART_LCR << p->regshift)); >> } >> - dev_err(p->dev, "Couldn't set LCR to %d\n", value); >> + /* >> + * FIXME: this deadlocks if port->lock is already held >> + * dev_err(p->dev, "Couldn't set LCR to %d\n", value); >> + */ >> } >> } >> #endif /* CONFIG_64BIT */ >> @@ -187,7 +193,10 @@ static void dw8250_serial_out32(struct uart_port *p, int offset, int value) >> dw8250_force_idle(p); >> writel(value, p->membase + (UART_LCR << p->regshift)); >> } >> - dev_err(p->dev, "Couldn't set LCR to %d\n", value); >> + /* >> + * FIXME: this deadlocks if port->lock is already held >> + * dev_err(p->dev, "Couldn't set LCR to %d\n", value); >> + */ >> } >> } >> >> -- >> 2.3.2 >> > > Why hide this valuable message when the uart port is not the console? > > In cases where it prints, the uart has likely become non-functional. Why? The initial port line settings come from the console line settings and the h/w was already initialized when the console was registered, so ISTM the worst that could happen there is UART_TX is written with the value intended for UART_DLL. > The driver knows there is a problem and should try to inform the user. Feel free to submit an alternative. A week has gone by since the initial report without any other fixes forthcoming, so I wanted to make sure at least _something_ addressed it in -rc4. 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
