Re: [PATCH] ASoC: dmaengine: add runtime status checking in dmaengine_pcm_dma_complete

[Lars-Peter Clausen <lars@metafoo.de>]

On 06/09/2013 03:37 PM, Lars-Peter Clausen wrote:
> On 06/07/2013 04:34 PM, Mark Brown wrote:
>> On Fri, Jun 07, 2013 at 07:57:29PM +0800, Qiao Zhou wrote:
>>> the dmaengine_pcm_dma_complete callback is usually executed after
>>> the dma interrupt, which uses tasklet_schedule, workqueue, or other
>>> method for quick int handler return.
>>>
>>> in some corner case, where pcm stream is released unexpected, like
>>> media server is killed, the runtime parameter will be freed. if it
>>> happens between the t1 and t2 in below chart, then the callback
>>> will try to access members of paramters which is already freed,
>>> and kernel panics.
>>>
>>> to avoid this issue, add runtime checking before other handling in
>>> dmaengine_pcm_dma_complete. if pcm stream is already released,
>>> just ignore the current handling and return.
>>
>> This doesn't seem like a good or robust way of fixing this, if we're
>> tearing down the resources the DMA is using while the DMA is in progress
>> then in the worst case that might include the memory being DMAed and of
>> course there's races if you just check the pointer - the pointer can be
>> checked at the same time as it's being freed (or between the free and
>> the clear).
>>
>> I think we should be either halting the DMA or waiting for it to finish
>> here.
> 
> I haven't see the original patch, but the proper solution to this problem
> should be to add a check to snd_dmaengine_pcm_close() to see if the DMA is
> still running.

Ok, since this will never happen, I suppose the problem is rather that the DMA
callback is called after dma_terminate_all() has been called. Which sounds like
it is a bug in the dmaengine driver. And this will likely also be a problem for
other users of that dmaengine driver and not only the ASoC driver, so it should
be fixed in the dmaengine driver.

- Lars