dmaengine: at_hdmac: Fix concurrency problems by removing atc_complete_all()

commit c6babed879fbe82796a601bf097649e07382db46 upstream.

atc_complete_all() had concurrency bugs, thus remove it:
1/ atc_complete_all() in its entirety was buggy, as when the atchan->queue
list (the one that contains descriptors that are not yet issued to the
hardware) contained descriptors, it fired just the first from the
atchan->queue, but moved all the desc from atchan->queue to
atchan->active_list and considered them all as fired. This could result in
calling the completion of a descriptor that was not yet issued to the
hardware.
2/ when in tasklet at atc_advance_work() time, atchan->active_list was
queried without holding the lock of the chan. This can result in
atchan->active_list concurrency problems between the tasklet and
issue_pending().

Fixes: dc78baa2b90b ("dmaengine: at_hdmac: new driver for the Atmel AHB DMA Controller")
Reported-by: Peter Rosin <peda@axentia.se>
Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
Cc: stable@vger.kernel.org
Link: https://lore.kernel.org/lkml/13c6c9a2-6db5-c3bf-349b-4c127ad3496a@axentia.se/
Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>
Link: https://lore.kernel.org/r/20221025090306.297886-1-tudor.ambarus@microchip.com
Link: https://lore.kernel.org/r/20221025090306.297886-8-tudor.ambarus@microchip.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c
index c30cce9..6dc273b 100644 (file)
--- a/drivers/dma/at_hdmac.c
+++ b/drivers/dma/at_hdmac.c
@@ -486,67 +486,26 @@ atc_chain_complete(struct at_dma_chan *atchan, struct at_desc *desc)
 }
 
 /**
- * atc_complete_all - finish work for all transactions
- * @atchan: channel to complete transactions for
- *
- * Eventually submit queued descriptors if any
- *
- * Assume channel is idle while calling this function
- * Called with atchan->lock held and bh disabled
- */
-static void atc_complete_all(struct at_dma_chan *atchan)
-{
-       struct at_desc *desc, *_desc;
-       LIST_HEAD(list);
-       unsigned long flags;
-
-       dev_vdbg(chan2dev(&atchan->chan_common), "complete all\n");
-
-       spin_lock_irqsave(&atchan->lock, flags);
-
-       /*
-        * Submit queued descriptors ASAP, i.e. before we go through
-        * the completed ones.
-        */
-       if (!list_empty(&atchan->queue))
-               atc_dostart(atchan, atc_first_queued(atchan));
-       /* empty active_list now it is completed */
-       list_splice_init(&atchan->active_list, &list);
-       /* empty queue list by moving descriptors (if any) to active_list */
-       list_splice_init(&atchan->queue, &atchan->active_list);
-
-       spin_unlock_irqrestore(&atchan->lock, flags);
-
-       list_for_each_entry_safe(desc, _desc, &list, desc_node)
-               atc_chain_complete(atchan, desc);
-}
-
-/**
  * atc_advance_work - at the end of a transaction, move forward
  * @atchan: channel where the transaction ended
  */
 static void atc_advance_work(struct at_dma_chan *atchan)
 {
        unsigned long flags;
-       int ret;
 
        dev_vdbg(chan2dev(&atchan->chan_common), "advance_work\n");
 
        spin_lock_irqsave(&atchan->lock, flags);
-       ret = atc_chan_is_enabled(atchan);
+       if (atc_chan_is_enabled(atchan) || list_empty(&atchan->active_list))
+               return spin_unlock_irqrestore(&atchan->lock, flags);
        spin_unlock_irqrestore(&atchan->lock, flags);
-       if (ret)
-               return;
-
-       if (list_empty(&atchan->active_list) ||
-           list_is_singular(&atchan->active_list))
-               return atc_complete_all(atchan);
 
        atc_chain_complete(atchan, atc_first_active(atchan));
 
        /* advance work */
        spin_lock_irqsave(&atchan->lock, flags);
-       atc_dostart(atchan, atc_first_active(atchan));
+       if (!list_empty(&atchan->active_list))
+               atc_dostart(atchan, atc_first_active(atchan));
        spin_unlock_irqrestore(&atchan->lock, flags);
 }