Lars-Peter Clausen <lars@xxxxxxxxxx> writes: >>> 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. Ah, I didn't see that. Isn't the descriptor lost if a terminate_all(), relying on vchan_get_all_descriptors() and vchan_dma_desc_free_list() is used ? >> >> 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... Ah yes, you must be thinking about the "poisoning" after the __list_del() call, I forgot about that. Do you think amending all these drivers and changing their list_del() into list_del_init() would at least prevent the "undefined behavior" ? I still think that their use of virt-dma is incorrect, ie. that at one point in time a virtual descriptor has to be on exactly one list of virt-dma (excepting transient critical sections). Cheers. -- Robert -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
