]> git.proxmox.com Git - mirror_ubuntu-bionic-kernel.git/commitdiff
serial: pl011: Fix DMA ->flush_buffer()
authorVincent Whitchurch <vincent.whitchurch@axis.com>
Mon, 18 Nov 2019 09:25:47 +0000 (10:25 +0100)
committerMarcelo Henrique Cerri <marcelo.cerri@canonical.com>
Fri, 17 Jan 2020 17:22:00 +0000 (14:22 -0300)
BugLink: https://bugs.launchpad.net/bugs/1857158
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 <vincent.whitchurch@axis.com>
Cc: stable <stable@vger.kernel.org>
Link: https://lore.kernel.org/r/20191118092547.32135-1-vincent.whitchurch@axis.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Kamal Mostafa <kamal@canonical.com>
Signed-off-by: Kleber Sacilotto de Souza <kleber.souza@canonical.com>
drivers/tty/serial/amba-pl011.c

index 320a759facdb44ba63d4942f90936462c04efcb2..a5232c1a65a1002b080947c686b0994d7d700367 100644 (file)
@@ -815,10 +815,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);