From mboxrd@z Thu Jan 1 00:00:00 1970 From: Lars-Peter Clausen Subject: Re: Issue in alsa when dma complete race with pcm release Date: Wed, 15 Jul 2015 10:59:22 +0200 Message-ID: <55A620EA.9050502@metafoo.de> References: <20150703082530.GA21580@shlinux2> <55966A75.90200@metafoo.de> <20150706090113.GA4995@shlinux2> <559A7415.8000006@metafoo.de> <20150707101302.GA22212@shlinux2> <559BD4D2.6080502@metafoo.de> <20150715013722.GA3005@shlinux2> <55A6082B.5020207@metafoo.de> <20150715070028.GB3005@shlinux2> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from smtp-out-020.synserver.de (smtp-out-020.synserver.de [212.40.185.20]) by alsa0.perex.cz (Postfix) with ESMTP id 825CD26172C for ; Wed, 15 Jul 2015 10:59:26 +0200 (CEST) In-Reply-To: <20150715070028.GB3005@shlinux2> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: alsa-devel-bounces@alsa-project.org To: Shengjiu Wang Cc: dmaengine , alsa-devel@alsa-project.org List-Id: alsa-devel@alsa-project.org 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.