| From f6a196477184b99a31d16366a8e826558aa11f6d Mon Sep 17 00:00:00 2001 |
| From: Vincent Whitchurch <vincent.whitchurch@axis.com> |
| Date: Mon, 18 Nov 2019 10:25:47 +0100 |
| Subject: serial: pl011: Fix DMA ->flush_buffer() |
| |
| From: Vincent Whitchurch <vincent.whitchurch@axis.com> |
| |
| commit f6a196477184b99a31d16366a8e826558aa11f6d upstream. |
| |
| PL011's ->flush_buffer() implementation releases and reacquires the port |
| lock. Due to a race condition here, data can end up being added to the |
| circular buffer but neither being discarded nor being sent out. This |
| leads to, for example, tcdrain(2) waiting indefinitely. |
| |
| Process A Process B |
| |
| uart_flush_buffer() |
| - acquire lock |
| - circ_clear |
| - pl011_flush_buffer() |
| -- release lock |
| -- dmaengine_terminate_all() |
| |
| uart_write() |
| - acquire lock |
| - add chars to circ buffer |
| - start_tx() |
| -- start DMA |
| - release lock |
| |
| -- acquire lock |
| -- turn off DMA |
| -- release lock |
| |
| // Data in circ buffer but DMA is off |
| |
| According to the comment in the code, the releasing of the lock around |
| dmaengine_terminate_all() is to avoid a deadlock with the DMA engine |
| callback. However, since the time this code was written, the DMA engine |
| API documentation seems to have been clarified to say that |
| dmaengine_terminate_all() (in the identically implemented but |
| differently named dmaengine_terminate_async() variant) does not wait for |
| any running complete callback to be completed and can even be called |
| from a complete callback. So there is no possibility of deadlock if the |
| DMA engine driver implements this API correctly. |
| |
| So we should be able to just remove this release and reacquire of the |
| lock to prevent the aforementioned race condition. |
| |
| Signed-off-by: Vincent Whitchurch <[email protected]> |
| Cc: stable <[email protected]> |
| Link: https://lore.kernel.org/r/[email protected] |
| Signed-off-by: Greg Kroah-Hartman <[email protected]> |
| |
| --- |
| drivers/tty/serial/amba-pl011.c | 6 ++---- |
| 1 file changed, 2 insertions(+), 4 deletions(-) |
| |
| --- a/drivers/tty/serial/amba-pl011.c |
| +++ b/drivers/tty/serial/amba-pl011.c |
| @@ -684,10 +684,8 @@ __acquires(&uap->port.lock) |
| if (!uap->using_tx_dma) |
| return; |
| |
| - /* Avoid deadlock with the DMA engine callback */ |
| - spin_unlock(&uap->port.lock); |
| - dmaengine_terminate_all(uap->dmatx.chan); |
| - spin_lock(&uap->port.lock); |
| + dmaengine_terminate_async(uap->dmatx.chan); |
| + |
| if (uap->dmatx.queued) { |
| dma_unmap_sg(uap->dmatx.chan->device->dev, &uap->dmatx.sg, 1, |
| DMA_TO_DEVICE); |
