* Issue in alsa when dma complete race with pcm release @ 2015-07-03 8:25 Shengjiu Wang 2015-07-03 10:56 ` Lars-Peter Clausen 0 siblings, 1 reply; 10+ messages in thread From: Shengjiu Wang @ 2015-07-03 8:25 UTC (permalink / raw) To: alsa-devel 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. Best regards wang shengjiu ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Issue in alsa when dma complete race with pcm release 2015-07-03 8:25 Issue in alsa when dma complete race with pcm release Shengjiu Wang @ 2015-07-03 10:56 ` Lars-Peter Clausen 2015-07-06 9:01 ` Shengjiu Wang 0 siblings, 1 reply; 10+ messages in thread From: Lars-Peter Clausen @ 2015-07-03 10:56 UTC (permalink / raw) To: Shengjiu Wang, alsa-devel; +Cc: dmaengine 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Issue in alsa when dma complete race with pcm release 2015-07-03 10:56 ` Lars-Peter Clausen @ 2015-07-06 9:01 ` Shengjiu Wang 2015-07-06 12:27 ` Lars-Peter Clausen 0 siblings, 1 reply; 10+ messages in thread From: Shengjiu Wang @ 2015-07-06 9:01 UTC (permalink / raw) To: Lars-Peter Clausen; +Cc: dmaengine, alsa-devel 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"? 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. Best regards wang shengjiu ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Issue in alsa when dma complete race with pcm release 2015-07-06 9:01 ` Shengjiu Wang @ 2015-07-06 12:27 ` Lars-Peter Clausen 2015-07-07 10:13 ` Shengjiu Wang 0 siblings, 1 reply; 10+ messages in thread From: Lars-Peter Clausen @ 2015-07-06 12:27 UTC (permalink / raw) To: Shengjiu Wang; +Cc: dmaengine, alsa-devel 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. 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. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Issue in alsa when dma complete race with pcm release 2015-07-06 12:27 ` Lars-Peter Clausen @ 2015-07-07 10:13 ` Shengjiu Wang 2015-07-07 13:32 ` Lars-Peter Clausen 0 siblings, 1 reply; 10+ messages in thread From: Shengjiu Wang @ 2015-07-07 10:13 UTC (permalink / raw) To: Lars-Peter Clausen; +Cc: dmaengine, alsa-devel 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. > 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? Best regards wang shengjiu ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Issue in alsa when dma complete race with pcm release 2015-07-07 10:13 ` Shengjiu Wang @ 2015-07-07 13:32 ` Lars-Peter Clausen 2015-07-15 1:37 ` Shengjiu Wang 0 siblings, 1 reply; 10+ messages in thread From: Lars-Peter Clausen @ 2015-07-07 13:32 UTC (permalink / raw) To: Shengjiu Wang; +Cc: dmaengine, alsa-devel 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Issue in alsa when dma complete race with pcm release 2015-07-07 13:32 ` Lars-Peter Clausen @ 2015-07-15 1:37 ` Shengjiu Wang 2015-07-15 7:13 ` Lars-Peter Clausen 0 siblings, 1 reply; 10+ messages in thread From: Shengjiu Wang @ 2015-07-15 1:37 UTC (permalink / raw) To: Lars-Peter Clausen; +Cc: dmaengine, alsa-devel 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? best regards wang shengjiu ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Issue in alsa when dma complete race with pcm release 2015-07-15 1:37 ` Shengjiu Wang @ 2015-07-15 7:13 ` Lars-Peter Clausen 2015-07-15 7:00 ` Shengjiu Wang 0 siblings, 1 reply; 10+ messages in thread From: Lars-Peter Clausen @ 2015-07-15 7:13 UTC (permalink / raw) To: Shengjiu Wang; +Cc: dmaengine, alsa-devel 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Issue in alsa when dma complete race with pcm release 2015-07-15 7:13 ` Lars-Peter Clausen @ 2015-07-15 7:00 ` Shengjiu Wang 2015-07-15 8:59 ` Lars-Peter Clausen 0 siblings, 1 reply; 10+ messages in thread From: Shengjiu Wang @ 2015-07-15 7:00 UTC (permalink / raw) To: Lars-Peter Clausen; +Cc: dmaengine, alsa-devel 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Issue in alsa when dma complete race with pcm release 2015-07-15 7:00 ` Shengjiu Wang @ 2015-07-15 8:59 ` Lars-Peter Clausen 0 siblings, 0 replies; 10+ messages in thread From: Lars-Peter Clausen @ 2015-07-15 8:59 UTC (permalink / raw) To: Shengjiu Wang; +Cc: dmaengine, alsa-devel On 07/15/2015 09:00 AM, Shengjiu Wang wrote: > 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?\ No, there is no guarantee that the substream hasn't been freed either. ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2015-07-15 8:59 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-07-03 8:25 Issue in alsa when dma complete race with pcm release Shengjiu Wang 2015-07-03 10:56 ` Lars-Peter Clausen 2015-07-06 9:01 ` Shengjiu Wang 2015-07-06 12:27 ` Lars-Peter Clausen 2015-07-07 10:13 ` Shengjiu Wang 2015-07-07 13:32 ` Lars-Peter Clausen 2015-07-15 1:37 ` Shengjiu Wang 2015-07-15 7:13 ` Lars-Peter Clausen 2015-07-15 7:00 ` Shengjiu Wang 2015-07-15 8:59 ` Lars-Peter Clausen
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.