On Thu, Dec 13, 2018 at 07:44:41PM +0100, Geert Uytterhoeven wrote: > Some callers of sci_submit_rx() hold the port spinlock, others don't. > During fallback to PIO, the driver needs to obtain the port spinlock. > If the lock was already held, spinlock recursion is detected, causing a > deadlock: BUG: spinlock recursion on CPU#0. > > Fix this by adding a flag parameter to sci_submit_rx() for the caller to > indicate the port spinlock is already held, so spinlock recursion can be > avoided. > > Move the spin_lock_irqsave() up, so all DMA disable steps are protected, > which is safe as the recently introduced dmaengine_terminate_async() can > be called in atomic context. > > Signed-off-by: Geert Uytterhoeven <[email protected]>
Reviewed-by: Simon Horman <[email protected]> > --- > v4: > - No changes, > > v3: > - Split in multiple patches. > --- > drivers/tty/serial/sh-sci.c | 16 +++++++++------- > 1 file changed, 9 insertions(+), 7 deletions(-) > > diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c > index 8146d9cef0cbe2d0..2a08039f792235e5 100644 > --- a/drivers/tty/serial/sh-sci.c > +++ b/drivers/tty/serial/sh-sci.c > @@ -1331,7 +1331,7 @@ static void sci_tx_dma_release(struct sci_port *s) > dma_release_channel(chan); > } > > -static void sci_submit_rx(struct sci_port *s) > +static void sci_submit_rx(struct sci_port *s, bool port_lock_held) > { > struct dma_chan *chan = s->chan_rx; > struct uart_port *port = &s->port; > @@ -1362,16 +1362,18 @@ static void sci_submit_rx(struct sci_port *s) > return; > > fail: > + /* Switch to PIO */ > + if (!port_lock_held) > + spin_lock_irqsave(&port->lock, flags); > if (i) > dmaengine_terminate_async(chan); > for (i = 0; i < 2; i++) > s->cookie_rx[i] = -EINVAL; > s->active_rx = -EINVAL; > - /* Switch to PIO */ > - spin_lock_irqsave(&port->lock, flags); > s->chan_rx = NULL; > sci_start_rx(port); > - spin_unlock_irqrestore(&port->lock, flags); > + if (!port_lock_held) > + spin_unlock_irqrestore(&port->lock, flags); > } > > static void work_fn_tx(struct work_struct *work) > @@ -1491,7 +1493,7 @@ static enum hrtimer_restart rx_timer_fn(struct hrtimer > *t) > } > > if (port->type == PORT_SCIFA || port->type == PORT_SCIFB) > - sci_submit_rx(s); > + sci_submit_rx(s, true); > > /* Direct new serial port interrupts back to CPU */ > scr = serial_port_in(port, SCSCR); > @@ -1617,7 +1619,7 @@ static void sci_request_dma(struct uart_port *port) > s->chan_rx_saved = s->chan_rx = chan; > > if (port->type == PORT_SCIFA || port->type == PORT_SCIFB) > - sci_submit_rx(s); > + sci_submit_rx(s, false); > } > } > > @@ -1667,7 +1669,7 @@ static irqreturn_t sci_rx_interrupt(int irq, void *ptr) > scr |= SCSCR_RDRQE; > } else { > scr &= ~SCSCR_RIE; > - sci_submit_rx(s); > + sci_submit_rx(s, false); > } > serial_port_out(port, SCSCR, scr); > /* Clear current interrupt */ > -- > 2.17.1 >
