lkml.org 
[lkml]   [2022]   [Aug]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 13/33] dmaengine: at_hdmac: Fix concurrency problems by removing atc_complete_all()
    Date
    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/
    ---
    drivers/dma/at_hdmac.c | 49 ++++--------------------------------------
    1 file changed, 4 insertions(+), 45 deletions(-)

    diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c
    index aeb241832c52..10424a6fcbbf 100644
    --- a/drivers/dma/at_hdmac.c
    +++ b/drivers/dma/at_hdmac.c
    @@ -920,42 +920,6 @@ atc_chain_complete(struct at_dma_chan *atchan, struct at_desc *desc)
    dma_run_dependencies(txd);
    }

    -/**
    - * 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->dma_chan), "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
    @@ -963,25 +927,20 @@ static void atc_complete_all(struct at_dma_chan *atchan)
    static void atc_advance_work(struct at_dma_chan *atchan)
    {
    unsigned long flags;
    - int ret;

    dev_vdbg(chan2dev(&atchan->dma_chan), "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);
    }

    --
    2.25.1
    \
     
     \ /
      Last update: 2022-08-20 15:00    [W:2.675 / U:0.212 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site