Hi Simon, On Wed, Jul 11, 2018 at 4:52 PM Simon Horman <horms@xxxxxxxxxxxx> wrote: > On Fri, Jul 06, 2018 at 11:05:42AM +0200, Geert Uytterhoeven wrote: > > The transmit DMA workqueue is never stopped, hence the work function may > > be called after the port has been shut down. > > > > Fix this race condition by cancelling queued work, if any, before DMA > > release. Don't initialize the work if DMA initialization failed, as it > > won't be used anyway. > > > > Signed-off-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx> > > --- a/drivers/tty/serial/sh-sci.c > > +++ b/drivers/tty/serial/sh-sci.c > > @@ -1293,6 +1293,7 @@ static void sci_tx_dma_release(struct sci_port *s) > > { > > struct dma_chan *chan = s->chan_tx_saved; > > > > + cancel_work_sync(&s->work_tx); > > s->chan_tx_saved = s->chan_tx = NULL; > > s->cookie_tx = -EINVAL; > > dmaengine_terminate_all(chan); > > @@ -1548,10 +1549,9 @@ static void sci_request_dma(struct uart_port *port) > > __func__, UART_XMIT_SIZE, > > port->state->xmit.buf, &s->tx_dma_addr); > > > > + INIT_WORK(&s->work_tx, work_fn_tx); > > s->chan_tx_saved = s->chan_tx = chan; > > Is it ok that work_fn_tx reads and writes s->work_tx which writes s->chan_tx? > is set after the call to INIT_WORK() ? Yes, unlike interrupt handlers, it isn't called until software kicks the workqueue using schedule_work(&s->work_tx); > > } > > - > > - INIT_WORK(&s->work_tx, work_fn_tx); > > } > > > > chan = sci_request_dma_chan(port, DMA_DEV_TO_MEM); Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds