DMA Engine development
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Koichiro Den" <den@valinux.co.jp>
Cc: Frank.Li@kernel.org, dmaengine@vger.kernel.org, vkoul@kernel.org
Subject: Re: [PATCH v5 19/24] dmaengine: dw-edma: Drain LL entries for STOP and PAUSE
Date: Wed, 12 Aug 2026 16:29:15 +0000	[thread overview]
Message-ID: <20260812162915.6FB5C1F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260812155721.2807506-20-den@valinux.co.jp>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] `dw_edma_wait_termination()` loops infinitely when the hardware is undergoing teardown (e.g., driver unbind) because interrupts and rechecks are suppressed and the stop request is never completed.
--

--- Patch [19]: [PATCH v5 19/24] dmaengine: dw-edma: Drain LL entries for STOP and PAUSE ---
commit 1e80fd5741b43db059b6d11175c2e35c684806f3
Author: Koichiro Den <den@valinux.co.jp>

dmaengine: dw-edma: Drain LL entries for STOP and PAUSE

An EDMA_REQ_STOP or EDMA_REQ_PAUSE can arrive while hardware still owns
published LL entries. Keep the request pending until hardware consumes
them instead of discarding the ring state immediately.

> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index eb0fc73a4f475..1cc9fa4866b9d 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c

[ ... ]

> @@ -1598,19 +1646,27 @@ static void dw_edma_wait_termination(struct dma_chan *dchan)
>  	/*
>  	 * A STOP may be deferred to a later interrupt while the channel is still
>  	 * running. Wait until that handler completes the termination.
> +	 *
> +	 * dmaengine_synchronize() cannot report an error. Returning while
> +	 * EDMA_REQ_STOP remains pending would let callers release transfer
> +	 * resources even though hardware access cannot be ruled out. Warn
> +	 * periodically, but keep waiting.
>  	 */
> -	while (time_before(jiffies, timeout)) {
> +	for (;;) {
>  		scoped_guard(spinlock_irqsave, &chan->vc.lock)
>  			stopping = chan->request == EDMA_REQ_STOP;
>  
>  		if (!stopping)
>  			return;

[Severity: High]
Could this infinite loop cause a soft lockup during device teardown?

If a client calls dmaengine_terminate_sync() concurrently with or just
before the DMA controller is unbound, dw_edma_remove() sets
chan->dw->teardown = true.

When teardown is set, background workers and IRQ handlers exit early without
clearing the channel status:

drivers/dma/dw-edma/dw-edma-core.c:dw_edma_queue_irq_work() {
        if (unlikely(READ_ONCE(chan->dw->teardown)))
                return;
        ...
}

Since these handlers are the only paths that clear EDMA_REQ_STOP,
the stop request is never cleared and this loop might never exit.

> +
> +		if (time_after_eq(jiffies, timeout)) {
> +			dev_warn(chan->dw->chip->dev,
> +				 "timeout waiting for channel termination; still waiting\n");
> +			timeout = jiffies + msecs_to_jiffies(5000);
> +		}
> +
>  		fsleep(1000);
>  	}
> -
> -	dev_warn(chan->dw->chip->dev,
> -		 "timeout waiting for channel termination\n");
>  }
>  
>  static void dw_edma_device_synchronize(struct dma_chan *dchan)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260812155721.2807506-1-den@valinux.co.jp?part=19

  reply	other threads:[~2026-08-12 16:29 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-12 15:56 [PATCH v5 00/24] dmaengine: dw-edma: Support dynamic LL appends Koichiro Den
2026-08-12 15:56 ` [PATCH v5 01/24] dmaengine: dw-edma: Add dw_edma_core_ll_cur_idx() to get current LL entry index Koichiro Den
2026-08-12 18:50   ` Frank Li
2026-08-12 15:56 ` [PATCH v5 02/24] dmaengine: dw-edma: Add dw_edma_core_ll_clear() to clear LL control-word Koichiro Den
2026-08-12 15:57 ` [PATCH v5 03/24] dmaengine: dw-edma: Factor out linked-list transfer start Koichiro Den
2026-08-12 15:57 ` [PATCH v5 04/24] dmaengine: dw-edma: Make DMA link list work as a circular buffer Koichiro Den
2026-08-12 16:14   ` sashiko-bot
2026-08-13  8:31     ` Koichiro Den
2026-08-12 15:57 ` [PATCH v5 05/24] dmaengine: dw-edma: Move callback result helper before LL helpers Koichiro Den
2026-08-12 15:57 ` [PATCH v5 06/24] dmaengine: dw-edma: Dispatch DONE interrupts by channel request Koichiro Den
2026-08-12 15:57 ` [PATCH v5 07/24] dmaengine: dw-edma: Centralize LL doorbell decisions Koichiro Den
2026-08-12 15:57 ` [PATCH v5 08/24] dmaengine: dw-edma: Prepare LL progress event handling Koichiro Den
2026-08-12 15:57 ` [PATCH v5 09/24] dmaengine: dw-edma: Prepare deferred IRQ reporting for LL events Koichiro Den
2026-08-12 15:57 ` [PATCH v5 10/24] dmaengine: dw-edma: Prepare LL kicks for event serialization Koichiro Den
2026-08-12 15:57 ` [PATCH v5 11/24] dmaengine: dw-edma: Serialize LL event capture with channel kicks Koichiro Den
2026-08-12 15:57 ` [PATCH v5 12/24] dmaengine: dw-edma: Keep channels stopped while ABORT is pending Koichiro Den
2026-08-12 15:57 ` [PATCH v5 13/24] dmaengine: dw-edma: Reclaim issued descriptors from IRQ-paired LL progress Koichiro Den
2026-08-12 20:54   ` Frank Li
2026-08-12 15:57 ` [PATCH v5 14/24] dmaengine: dw-edma: Add LL interrupt placement policy Koichiro Den
2026-08-12 15:57 ` [PATCH v5 15/24] dmaengine: dw-edma: Recheck stopped LL channels before restart Koichiro Den
2026-08-12 16:33   ` sashiko-bot
2026-08-12 15:57 ` [PATCH v5 16/24] dmaengine: dw-edma: Use HDMA watermarks as progress events Koichiro Den
2026-08-12 15:57 ` [PATCH v5 17/24] dmaengine: dw-edma: Recover stopped channels from tx_status() Koichiro Den
2026-08-12 15:57 ` [PATCH v5 18/24] dmaengine: dw-edma: Make the LL ring reset a full channel resync Koichiro Den
2026-08-12 15:57 ` [PATCH v5 19/24] dmaengine: dw-edma: Drain LL entries for STOP and PAUSE Koichiro Den
2026-08-12 16:29   ` sashiko-bot [this message]
2026-08-12 15:57 ` [PATCH v5 20/24] dmaengine: dw-edma: Dynamically append requests while running Koichiro Den
2026-08-12 15:57 ` [PATCH v5 21/24] dmaengine: dw-edma: Add engine reset and enable operations Koichiro Den
2026-08-12 15:57 ` [PATCH v5 22/24] dmaengine: dw-edma: Add engine recovery infrastructure Koichiro Den
2026-08-12 15:57 ` [PATCH v5 23/24] dmaengine: dw-edma: Detect and recover a stalled eDMA engine Koichiro Den
2026-08-12 15:57 ` [PATCH v5 24/24] dmaengine: dw-edma: Add trace support Koichiro Den
2026-08-12 21:05 ` [PATCH v5 00/24] dmaengine: dw-edma: Support dynamic LL appends Frank Li
2026-08-13  1:04   ` Koichiro Den

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260812162915.6FB5C1F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=Frank.Li@kernel.org \
    --cc=den@valinux.co.jp \
    --cc=dmaengine@vger.kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    --cc=vkoul@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox