Re: [PATCH v9 6/8] serial: 8250: drain canceled TX DMA before reusing its buffer

[Andy Shevchenko]

On Mon, Nov 02, 2026 at 04:14:33PM -0700, James Hilliard wrote:
> uart_flush_buffer() resets the transmit kfifo before calling the driver.
> serial8250_tx_dma_flush() terminates DMA asynchronously and clears
> tx_running, allowing a new write to replace tx_size while a detached
> callback has not yet run. That callback can then consume bytes
> from the new transmission. Reusing the reset kfifo can also overwrite the
> source of a transfer that has not stopped yet.
> 
> Reserve the flushed FIFO storage through serial core before returning
> from the flush callback. Keep tx_running set until a worker synchronizes
> the terminated channel, suppressing old callbacks and new submissions
> during that interval. Return the mapping to CPU ownership before releasing
> the reservation and waking writers. Normal transfers continue to use the
> FIFO directly.
> 
> Reuse the locked completion processing after retirement, including OMAP
> delayed register restoration and transmitter-empty handling. Take a runtime
> PM reference around worker UART accesses and drain the worker before
> releasing the channels. Shutdown releases the FIFO reservation after
> stopping DMA and unmapping its source. Serial core keeps writes and wakeups
> gated until startup or resume makes the port ready again.
> 
> Track a flushed FIFO separately from the transfer length. If termination
> fails, retain the reservation until normal completion and account the
> completed bytes without advancing the reset FIFO. After successful
> cancellation, process a zero-length completion once callbacks have drained.
> 
> The race was found by inspecting cancellation and restart while preparing
> H616 UART DMA support.
> 
> Retain the tx_running check in both completion callbacks. Releasing the
> reservation relies on the DMA provider draining detached callbacks when
> dmaengine_synchronize() returns; provider callback-lifetime fixes supply
> that guarantee for controllers which previously lacked it.

...

> -	if (!dma->tx_running)
> +	if (!dma->tx_running || serial8250_tx_dma_blocked(dma))
>  		return;

> -	if (!dma->tx_running)
> +	if (!dma->tx_running || serial8250_tx_dma_blocked(dma))
>  		return;

...

>  	guard(uart_port_lock_irqsave)(&p->port);
>  
> -	if (dma->shutting_down)
> +	if (serial8250_tx_dma_blocked(dma) || !dma->tx_running)
>  		return;

Try to make these checks consistent. It's always better to choose the order and
be aware of the potential side-effects of the bit-wise operations.

...

> -	if (dma->shutting_down || dma->tx_running)
> +	if (serial8250_tx_dma_blocked(dma) || dma->tx_running)
>  		return 0;
>  	if (uart_tx_stopped(&p->port) || kfifo_is_empty(&tport->xmit_fifo)) {

-- 
With Best Regards,
Andy Shevchenko