Messages in this thread | | | Date | Tue, 03 Mar 2015 08:23:10 +0100 | From | Lars-Peter Clausen <> | Subject | Re: [PATCH] dmaengine: virt-dma: fix completion list manipulation |
| |
On 03/02/2015 11:03 PM, Robert Jarzmik wrote: > Lars-Peter Clausen <lars@metafoo.de> writes: > >> On 03/02/2015 10:19 PM, Robert Jarzmik wrote: >>> diff --git a/drivers/dma/virt-dma.h b/drivers/dma/virt-dma.h >>> index 3772032..2a3da22 100644 >>> --- a/drivers/dma/virt-dma.h >>> +++ b/drivers/dma/virt-dma.h >>> @@ -91,7 +91,7 @@ static inline void vchan_cookie_complete(struct virt_dma_desc *vd) >>> dma_cookie_complete(&vd->tx); >>> dev_vdbg(vc->chan.device->dev, "txd %p[%x]: marked complete\n", >>> vd, cookie); >>> - list_add_tail(&vd->node, &vc->desc_completed); >>> + list_move_tail(&vd->node, &vc->desc_completed); >> >> That will break all drivers which handle this currently correctly and remove the >> descriptor from any list before calling vchan_cookie_complete. > Ah, well well I don't agree. > > First, let's split the drivers which remove the descriptors and these which > don't : > > These which remove the descriptor: > dma-jz4740.c > fsl-edma.c > > These which don't remove the descriptor: > amba-pl08x.c > edma.c > img-mdc-dma.c > k3dma.c > moxart-dma.c > omap-dma.c > qcom_bam_dma.c > s3c24xx-dma.c > sa11x0-dma.c > sun6i-dma.c
All of those remove the descriptor from the list when they start the transfer.
> > That settles the correctness I think, the correct behavior is to not remove the > descriptor and let it be done by vchan_cookie_complete(). > > Now for the remaining 2 drivers, we'll have : > - list_del(&vd->node) => vd becomes a singleton > - list_move_tail(&vd->node, &...desc_completed) > => list_del(&vd->node) : nothing changes, it's a nop > => list_add_tail(&vd->node, &...desc_completed) > > And the behavior remains correct after the patch, only one "list_del()" is done > twice for nothing. Where do you see any breakage ?
Calling list_del() on a list item that is not on a list causes undefined behavior, which can result in memory corruption, segfaults, etc...
| |