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 15/24] dmaengine: dw-edma: Recheck stopped LL channels before restart
Date: Wed, 12 Aug 2026 16:33:17 +0000	[thread overview]
Message-ID: <20260812163318.1D6CD1F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260812155721.2807506-16-den@valinux.co.jp>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Silent hardware stop during channel termination causes a 5-second timeout and descriptor leak.
--

--- Patch [15]: [PATCH v5 15/24] dmaengine: dw-edma: Recheck stopped LL channels before restart ---
commit aec4dbaf9567f7c6e772cd5d67fe89362acafbbf
Author: Koichiro Den <den@valinux.co.jp>

dmaengine: dw-edma: Recheck stopped LL channels before restart

This commit introduces a mechanism to recheck stopped LL channels before
restarting them to handle races between DMA_LLP samples and updates. It also
gates eDMA kicks with a one-shot credit and serializes the stopped-state
check against IRQ capture.

> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index 13c45549e08be..8325e296cb2a4 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c
[ ... ]
>  /* Must be called with vc.lock held. */
>  static void
>  dw_edma_set_request(struct dw_edma_chan *chan, enum dw_edma_request request)
>  {
> +	if (!chan->non_ll && chan->request != request)
> +		dw_edma_ll_recheck_cancel(chan);
>  	chan->request = request;
>  }

[Severity: High]
This isn't a bug introduced by this patch, but does cancelling the recheck
work when setting EDMA_REQ_STOP remove the mechanism that detects a silent
hardware stop?

If the client calls dmaengine_terminate_all(), it sets
chan->request = EDMA_REQ_STOP and cancels the recheck work here. If the
hardware stops silently without an interrupt, dw_edma_wait_termination()
will spin for 5 seconds waiting for the interrupt to clear EDMA_REQ_STOP,
leading to a timeout and leaking the pending descriptors.

[ ... ]
> -/* Must be called with vc.lock held. */
> -static void dw_edma_core_ch_maybe_doorbell(struct dw_edma_chan *chan)
> +/*
> + * Must be called with vc.lock held. Return true when published work is still
> + * running and may need one later stop recheck.
> + */
> +static bool dw_edma_core_ch_maybe_doorbell(struct dw_edma_chan *chan)
>  {
>  	if (chan->non_ll || chan->request != EDMA_REQ_NONE ||
> -	    chan->status != EDMA_ST_BUSY || !dw_edma_ll_pending(chan))
> -		return;
> +	    chan->status != EDMA_ST_BUSY || !dw_edma_ll_pending(chan) ||
> +	    dw_edma_abort_is_pending(chan))
> +		return false;

[Severity: High]
This is a pre-existing issue, but because this early return checks
chan->request != EDMA_REQ_NONE, doesn't this explicitly prevent
dw_edma_ll_reconcile_and_refill() from running when chan->request ==
EDMA_REQ_STOP?

Combined with the cancelled recheck work above, this means there is no
mechanism to clear EDMA_REQ_STOP if the hardware stops silently during
termination.

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

  reply	other threads:[~2026-08-12 16:33 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 [this message]
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
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=20260812163318.1D6CD1F000E9@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