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