Hi James, > @@ -37,26 +40,35 @@ static void __dma_tx_complete(void *param) > struct uart_8250_port *p = param; > struct uart_8250_dma *dma = p->dma; > > guard(uart_port_lock_irqsave)(&p->port); > > - if (dma->shutting_down) > + if (serial8250_tx_dma_blocked(dma)) > return; > - > - if (!dma->tx_running) > - return; > - > serial8250_tx_dma_complete(p); > }
Removing `if (!dma->tx_running) return;` and relying solely on dmaengine_synchronize(dma->txchan) to drain completion callbacks breaks on DMAengine drivers used with 8250 UARTs that do not implement .device_synchronize (where dmaengine_synchronize() is a no-op), including: - drivers/dma/sun6i-dma.c (Allwinner H616, cited in the commit message) - drivers/dma/dw/core.c (dw_dmac) - drivers/dma/pl330.c - drivers/dma/mediatek/mtk-uart-apdma.c On those controllers, if Transfer #1's completion tasklet (vchan_complete, dw_dma_tasklet, or pl330_tasklet) has already dequeued the completed descriptor on another CPU, dropped the channel lock, and is waiting on port->lock at the start of __dma_tx_complete() when uart_flush_buffer() -> serial8250_tx_dma_flush() runs: 1. serial8250_tx_dma_flush_work() calls dmaengine_synchronize() (which returns immediately because .device_synchronize is NULL), acquires port->lock, clears dma->tx_terminating = false, and calls serial8250_tx_dma_complete(p) -> uart_xmit_unblock(&p->port). 2. If no new transfer has started yet (dma->tx_running == 0), Transfer #1's in-flight callback acquires port->lock, sees !serial8250_tx_dma_blocked(dma), and runs serial8250_tx_dma_complete(p) a second time even though dma->tx_running is 0. 3. If a writer has already started Transfer #2 (dma->tx_running == 1, dma->tx_flushed == false), Transfer #1's stale callback runs serial8250_tx_dma_complete(p) while Transfer #2 is still active in hardware, syncing tx_addr for CPU, advancing xmit_fifo by Transfer #2's tx_size, and submitting Transfer #3 concurrently with Transfer #2. In addition to implementing .device_synchronize in the affected DMAengine drivers, __dma_tx_complete() (and omap_8250_dma_tx_complete() for symmetry) should keep `if (!dma->tx_running) return;` and check `dmaengine_tx_status(dma->txchan, dma->tx_cookie, NULL) == DMA_COMPLETE` (matching dma_rx_complete()). We modeled the v7 TX and RX DMA cancellation state machines in TLA+ to verify both the counterexample traces above and the fix: https://github.com/ismell/linux-8250-dma-tla Thanks, Raul
