On Wed, Jul 15, 2015 at 09:13:47AM +0200, Lars-Peter Clausen wrote: > On 07/15/2015 03:37 AM, Shengjiu Wang wrote: > >On Tue, Jul 07, 2015 at 03:32:02PM +0200, Lars-Peter Clausen wrote: > >>On 07/07/2015 12:13 PM, Shengjiu Wang wrote: > >>>On Mon, Jul 06, 2015 at 02:27:01PM +0200, Lars-Peter Clausen wrote: > >>>>On 07/06/2015 11:01 AM, Shengjiu Wang wrote: > >>>>>On Fri, Jul 03, 2015 at 12:56:53PM +0200, Lars-Peter Clausen wrote: > >>>>>>On 07/03/2015 10:25 AM, Shengjiu Wang wrote: > >>>>>>>Hi alsa-devel > >>>>>>> > >>>>>>> There maybe a issue in ALSA when dma complete race with snd_pcm_release. > >>>>>>>The pcm release and dma complete are in different thread. There is occasion > >>>>>>>that dmaengine_pcm_dma_complete() is called too late, some memory has been > >>>>>>>freed, the prtd is null. Then there is kernel dump. > >>>>>>> > >>>>>>> Is there any solution for this issue? Thanks. > >>>>>> > >>>>>>We need to introduce a synchronization primitive that allows a > >>>>>>dmaengine client to synchronize to the execution of the complete > >>>>>>callbacks. > >>>>>> > >>>>>>terminate_all() unfortunately can't do this since terminate_all() > >>>>>>might be called from within one of the complete callbacks and so > >>>>>>would cause a deadlock if we'd wait for all complete callbacks to > >>>>>>finish before terminate_all() returns. > >>>>>> > >>>>>>So what is needed is a new function called dmaengine_sync() that > >>>>>>will wait until all scheduled complete callbacks have finished. A > >>>>>>call to this function needs to be put in snd_dmaengine_pcm_close() > >>>>>>before the prtd is closed. > >>>>>> > >>>>>>- Lars > >>>>> > >>>>>How to check " all scheduled complete callbacks have finished"? > >>>> > >>>>That will be up to the dmaengine driver. But it basically comes down > >>>>to two things: > >>>> > >>>>1) The driver needs to make sure that tasklet_schedule() is no > >>>>longer called after terminate_all() has finished. > >>>Some driver can't make sure this. The dma interrupt may come later after > >>>terminate_all. > >> > >>Most drivers can't make sure that the interrupt routine is not > >>executed after terminate_all() has been called, since both are fully > >>asynchronous. But what the driver needs to take care of is to > >>synchronize the two e.g. using a spin_lock() and make sure that if > >>terminate_all() has been called tasklet_schedule() is not called, > >>even if the isr is executed. Most drivers get this rigght. > >> > >>>>2) In the sync() callback call tasklet_kill() to make sure that it > >>>>has finished running > >>>> > >>>>> > >>>>>One concern is if add wait in snd_dmaengine_pcm_close(), which wil cause > >>>>>the snd_pcm_release is bound with dmaengine, when there is error in dma > >>>>>and no callback be called. Then the snd_pcm_release will not be released. > >>>> > >>>>The sync() function will only wait if there is a callback scheduled, > >>>>if there is non scheduled it will return immediately. > >>> > >>> > >>>Do you have other method to resolve this issue? or simple method? > >> > >>No, this is the way to go. You have two asynchronous processes and > >>you want to get deterministic execution ordering between the two you > >>need to add a synchronization primitive. > >> > >>- Lars > >> > >Thanks your advice. It is workable. But need to add new api in dmaengine.h > > > >I think about another method, can we add spin_lock in > >dmaengine_pcm_dma_complete() and snd_dmaengine_pcm_close()? and add a flag > >for pcm_close, if pcm closed, then just do nothing in dma complete. > >How do you think for this? > > Since you are freeing the memory that would store the flag that wont work. > > - Lars > It seems the snd_pcm_substream is not released, can we add flag in this struct?\ best regards wang shengjiu -- 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
