• Lukas Wunner's avatar
    dmaengine: bcm2835: Fix interrupt race on RT · 63ca7858
    Lukas Wunner authored
    commit f7da7782 upstream.
    
    If IRQ handlers are threaded (either because CONFIG_PREEMPT_RT_BASE is
    enabled or "threadirqs" was passed on the command line) and if system
    load is sufficiently high that wakeup latency of IRQ threads degrades,
    SPI DMA transactions on the BCM2835 occasionally break like this:
    
    ks8851 spi0.0: SPI transfer timed out
    bcm2835-dma 3f007000.dma: DMA transfer could not be terminated
    ks8851 spi0.0 eth2: ks8851_rdfifo: spi_sync() failed
    
    The root cause is an assumption made by the DMA driver which is
    documented in a code comment in bcm2835_dma_terminate_all():
    
    /*
     * Stop DMA activity: we assume the callback will not be called
     * after bcm_dma_abort() returns (even if it does, it will see
     * c->desc is NULL and exit.)
     */
    
    That assumption falls apart if the IRQ handler bcm2835_dma_callback() is
    threaded: A client may terminate a descriptor and issue a new one
    before the IRQ handler had a chance to run. In fact the IRQ handler may
    miss an *arbitrary* number of descriptors. The result is the following
    race condition:
    
    1. A descriptor finishes, its interrupt is deferred to the IRQ thread.
    2. A client calls dma_terminate_async() which sets channel->desc = NULL.
    3. The client issues a new descriptor. Because channel->desc is NULL,
       bcm2835_dma_issue_pending() immediately starts the descriptor.
    4. Finally the IRQ thread runs and writes BCM2835_DMA_INT to the CS
       register to acknowledge the interrupt. This clears the ACTIVE flag,
       so the newly issued descriptor is paused in the middle of the
       transaction. Because channel->desc is not NULL, the IRQ thread
       finalizes the descriptor and tries to start the next one.
    
    I see two possible solutions: The first is to call synchronize_irq()
    in bcm2835_dma_issue_pending() to wait until the IRQ thread has
    finished before issuing a new descriptor. The downside of this approach
    is unnecessary latency if clients desire rapidly terminating and
    re-issuing descriptors and don't have any use for an IRQ callback.
    (The SPI TX DMA channel is a case in point.)
    
    A better alternative is to make the IRQ thread recognize that it has
    missed descriptors and avoid finalizing the newly issued descriptor.
    So first of all, set the ACTIVE flag when acknowledging the interrupt.
    This keeps a newly issued descriptor running.
    
    If the descriptor was finished, the channel remains idle despite the
    ACTIVE flag being set. However the ACTIVE flag can then no longer be
    used to check whether the channel is idle, so instead check whether
    the register containing the current control block address is zero
    and finalize the current descriptor only if so.
    
    That way, there is no impact on latency and throughput if the client
    doesn't care for the interrupt: Only minimal additional overhead is
    introduced for non-cyclic descriptors as one further MMIO read is
    necessary per interrupt to check for idleness of the channel. Cyclic
    descriptors are sped up slightly by removing one MMIO write per
    interrupt.
    
    Fixes: 96286b57 ("dmaengine: Add support for BCM2835")
    Signed-off-by: default avatarLukas Wunner <lukas@wunner.de>
    Cc: stable@vger.kernel.org # v3.14+
    Cc: Frank Pavlic <f.pavlic@kunbus.de>
    Cc: Martin Sperl <kernel@martin.sperl.org>
    Cc: Florian Meier <florian.meier@koalo.de>
    Cc: Clive Messer <clive.m.messer@gmail.com>
    Cc: Matthias Reichl <hias@horus.com>
    Tested-by: default avatarStefan Wahren <stefan.wahren@i2se.com>
    Acked-by: default avatarFlorian Kauer <florian.kauer@koalo.de>
    Signed-off-by: default avatarVinod Koul <vkoul@kernel.org>
    Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
    63ca7858
bcm2835-dma.c 28.6 KB