Hi Laurent,
On Fri, Jul 11, 2014 at 3:27 PM, Laurent Pinchart
<laurent.pinchart@xxxxxxxxxxxxxxxx> wrote:
>> >> AFAIK, descriptors are cleaned up automatically after use, and the only
>> >> function that takes a dma_async_tx_descriptor is dmaengine_submit().
>> >> But indeed, if you don't use them, that doesn't happen.
>> >> So calling dmaengine_terminate_all() seems appropriate to fix this.
>> >>
>> >> But, Documentation/dmaengine.txt says:
>> >> desc = dmaengine_prep_slave_sg(chan, sgl, nr_sg, direction,
>> >> flags);
>> >>
>> >> Once a descriptor has been obtained, the callback information can be
>> >> added and the descriptor must then be submitted. Some DMA engine
>> >> drivers may hold a spinlock between a successful preparation and
>> >> submission so it is important that these two operations are closely
>> >> paired.
>> >>
>> >> W.r.t. the spinlock, is it safe to call dmaengine_terminate_all() for a
>> >> prepared but not submitted transfer?
>> >> Is there another/better way?
>> >
>> > Basically, I have no idea. I'm pretty sure some drivers will support it,
>> > others won't. Reading the code won't help much, as there's no available
>> > information regarding what the expected behaviour is. Welcome to the
>> > wonderful world of the undocumented DMA engine API :-)
>>
>> I did dive a bit into the code...
>>
>> 1. The spinlock comment seems to apply to INTEL_IOATDMA only.
>> This driver doesn't implement dma_device.device_control(), so
>> dmaengine_terminate_all() is a no-op there, and there doesn't seem to be
>> a way to release a descriptor without submitting it first.
>>
>> 2. While I thought dmaengine_terminate_all() would release all descriptors
>> on shdma, as it calls shdma_chan_ld_cleanup(), it only releases the
>> descriptors that are at least in submitted state.
>> Hence after a while, you get
>>
>> sh-dma-engine e6700020.dma-controller: No free link descriptor
>> available
>>
>> Interestingly, this contradicts with the comment in
>> shdma_free_chan_resources():
>>
>> /* Prepared and not submitted descriptors can still be on the queue
>> */
>> if (!list_empty(&schan->ld_queue))
>> shdma_chan_ld_cleanup(schan, true);
>>
>> As dmaengine_submit() will not start the DMA operation, but merely add it
>> to the pending queue (starting needs a call to dma_async_issue_pending()),
>> it seems the right solution is to continue submitting the request for which
>> preparation succeeded, and then aborting everything using
>> dmaengine_terminate_all().
>>
>> Note that this also means that if submitting the RX request failed, you
>> should still submit the TX request, as it has been prepared.
>>
>> Alternatively, you can interleave prep and submit for both channels, which
>> makes the error recovery code less convoluted.
>
> How about fixing the DMA API to allow cleaning up a prepared request without
> submitting it ?
That's another option. But that would require updating all drivers.
Note that only ioat, iop-adma, mv_xor, and ppc4xx do not implement
.device_control() and/or DMA_TERMINATE_ALL.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
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
