Re: [PATCH V3] pch_uart: Add eg20t_port lock field, avoid recursivespinlocks

From: Alan Cox
Date: Tue Jun 19 2012 - 18:01:12 EST


On Tue, 19 Jun 2012 14:00:18 -0700
Darren Hart <dvhart@xxxxxxxxxxxxxxx> wrote:

> pch_uart_interrupt() takes priv->port.lock which leads to two recursive
> spinlock calls if low_latency==1 or CONFIG_PREEMPT_RT_FULL=y (one
> otherwise):
>
> pch_uart_interrupt
> spin_lock_irqsave(priv->port.lock, flags)
> case PCH_UART_IID_RDR_TO (data ready)
> handle_rx_to
> push_rx
> tty_port_tty_get
> spin_lock_irqsave(&port->lock, flags) <--- already hold this lock
> ...
> tty_flip_buffer_push
> ...
> flush_to_ldisc
> spin_lock_irqsave(&tty->buf.lock)
> spin_lock_irqsave(&tty->buf.lock)
> disc->ops->receive_buf(tty, char_buf)
> n_tty_receive_buf
> tty->ops->flush_chars()
> uart_flush_chars
> uart_start
> spin_lock_irqsave(&port->lock) <--- already hold this lock
>
> Avoid this by using a dedicated lock to protect the eg20t_port structure
> and IO access to its membase. This is more consistent with the 8250
> driver. Ensure priv->lock is always take prior to priv->port.lock when
> taken at the same time.
>
> V2: Remove inadvertent whitespace change.
> V3: Account for oops_in_progress for the private lock in
> pch_console_write().
>
> Note: Like the 8250 driver, if a printk is introduced anywhere inside
> the pch_console_write() critical section, the kernel will hang
> on a recursive spinlock on the private lock. The oops case is
> handled by using a trylock in the oops_in_progress case.
>
> Signed-off-by: Darren Hart <dvhart@xxxxxxxxxxxxxxx>
> CC: Tomoya MORINAGA <tomoya.rohm@xxxxxxxxx>
> CC: Feng Tang <feng.tang@xxxxxxxxx>
> CC: Alexander Stein <alexander.stein@xxxxxxxxxxxxxxxxxxxxx>
> CC: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> CC: Alan Cox <alan@xxxxxxxxxxxxxxx>
> CC: linux-serial@xxxxxxxxxxxxxxx

Acked-by: Alan Cox <alan@xxxxxxxxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/