Re: [PATCH v8 2/2] dmaengine: fsl-edma: Support dynamic scatter/gather chaining

Frank Li <[email protected]> Tue, 4 Aug 2026 17:06:42 -0500
Newsgroups org.kernel.vger.dmaengine,dev.linux.lists.imx,dev.linux.lists.sashiko-reviews
Message-ID <anJicljn3M1ARmkQ@SMW015318>
On Mon, Aug 03, 2026 at 04:28:20PM +0000, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] O(N^2) algorithmic complexity inside a spinlock during scatter-gather descriptor submission can cause severe IRQ latency.
> --
>
> --- Patch [2]: [PATCH v8 2/2] dmaengine: fsl-edma: Support dynamic scatter/gather chaining ---
> commit f4efa8f669813b67983a5bf98e40939bbc5e00b4
> Author: Benoît Monin <[email protected]>
>
> dmaengine: fsl-edma: Support dynamic scatter/gather chaining
>
> This commit implements dynamic linking of scatter/gather transfers to enable
> chaining multiple DMA descriptors without stopping the channel. It adds
> fsl_edma_link_sg() to dynamically link the last TCD of a previously issued
> descriptor to the first TCD of a new descriptor.
>
> > diff --git a/drivers/dma/fsl-edma-common.c b/drivers/dma/fsl-edma-common.c
> > index c5f5951c988be..189eb9d1269ea 100644
> > --- a/drivers/dma/fsl-edma-common.c
> > +++ b/drivers/dma/fsl-edma-common.c
> [ ... ]
> > @@ -931,14 +958,93 @@ void fsl_edma_xfer_desc(struct fsl_edma_chan *fsl_chan)
> [ ... ]
> > +static void fsl_edma_link_sg(struct fsl_edma_chan *fsl_chan, struct fsl_edma_desc *fsl_desc)
> > +{
> [ ... ]
> > +	if (!(last_csr & EDMA_TCD_CSR_D_REQ) ||
> > +	    last_csr & EDMA_TCD_CSR_E_LINK ||
> > +	    list_count_nodes(&fsl_chan->vchan.desc_issued) >= FIELD_MAX(EDMA_TCD_CSR_LINKCH))
> > +		return;
>
> [Severity: High]
> Could this list_count_nodes() call cause severe IRQ latency spikes?
>
> list_count_nodes() unconditionally iterates over the entire list to compute
> its length. If the list contains hundreds or thousands of nodes, it will
> traverse all of them, well past the limit of 31 checked here.

Nice work, only left this issue by found AI.

>
> [ ... ]
> > @@ -949,7 +1055,12 @@ void fsl_edma_issue_pending(struct dma_chan *chan)
> [ ... ]
> > -	if (vchan_issue_pending(&fsl_chan->vchan) && !fsl_chan->edesc)
> > +	list_for_each_entry_safe(vdesc, tmp, &fsl_chan->vchan.desc_submitted, node) {
> > +		fsl_edma_link_sg(fsl_chan, to_fsl_edma_desc(vdesc));

You check return value, if > 31, use list_splice to whole submitted queue
to pending queue.

but this is small optimzation.

Frank

> > +		list_move_tail(&vdesc->node, &fsl_chan->vchan.desc_issued);
> > +	}
>
> Since fsl_edma_link_sg() is called here in a loop over desc_submitted, while
> holding the vchan.lock spinlock with interrupts disabled, submitting a large
> batch of N descriptors (e.g., from network or ALSA drivers) could result in
> O(N^2) list node traversals.
>
> Could this heavily stall the CPU and potentially cause soft lockups?
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2