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

Frank Li <[email protected]>
Newsgroups dev.linux.lists.imx,dev.linux.lists.sashiko-reviews,org.kernel.vger.dmaengine
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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.