From mboxrd@z Thu Jan 1 00:00:00 1970 From: Qiao Zhou Subject: Re: async between dmaengine_pcm_dma_complete and snd_pcm_release Date: Thu, 10 Oct 2013 09:08:53 +0800 Message-ID: <5255FE25.1020906@marvell.com> References: <525505C2.4070201@marvell.com> <5255119D.9020303@metafoo.de> <52551416.9020004@metafoo.de> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from mx0a-0016f401.pphosted.com (mx0a-0016f401.pphosted.com [67.231.148.174]) by alsa0.perex.cz (Postfix) with ESMTP id D6105261661 for ; Thu, 10 Oct 2013 03:09:11 +0200 (CEST) In-Reply-To: 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: Takashi Iwai Cc: "alsa-devel@alsa-project.org" , Lars-Peter Clausen , "lgirdwood@gmail.com" , Mark Brown , "zhangfei.gao@gmail.com" , "trinity.qiao.zhou@gmail.com" , Chao Xie List-Id: alsa-devel@alsa-project.org On 10/09/2013 06:53 PM, Takashi Iwai wrote: > At Wed, 09 Oct 2013 10:30:14 +0200, > Lars-Peter Clausen wrote: >> >> On 10/09/2013 10:19 AM, Lars-Peter Clausen wrote: >>> On 10/09/2013 09:29 AM, Qiao Zhou wrote: >>>> Hi Mark, Liam, Jaroslav, Takashi >>>> >>>> I met an issue in which kernel panic appears in dmaengine_pcm_dma_complete >>>> function on a quad-core system. The dmaengine_pcm_dma_complete is running >>>> core0, while snd_pcm_release has already been executed on core1, due to in >>>> low memory stress oom killer kills the audio thread to release some memory. >>>> >>>> snd_pcm_release frees the runtime parameters, and runtime is used in >>>> dmaengine_pcm_dma_complete, which is a callback from tasklet in dmaengine. >>>> In current audio driver, we can't promise that dmaengine_pcm_dma_complete is >>>> not executed after snd_pcm_release on multi cores. Maybe we should add some >>>> protection. Do you have any suggestion? >>>> >>>> I have tried to apply below workaround, which can fix the panic, but I'm not >>>> confident it's proper. Need your comment and better suggestion. >>> >>> I think this is a general problem with your dmaengine driver, nothing audio >>> specific. If the callback is able to run after dmaengine_terminate_all() has >>> returned successfully there is a bug in the dmaengine driver. You need to >>> make sure that none of the callbacks is called after terminate_all() has >>> finished and you probably also have to make sure that the tasklet has >>> completed, if it is running at the same time as the call to >>> dmaengine_terminate_all(). >> >> On the other hand that last part could get tricky as the >> dmaengine_terminate_all() might be call from within the callback. > > Basically you'd need a sync for the termination (in this case > something like tasklet_kill()) to be called at hw_free and prepare > where you can do schedule. dmaengine_terminate_all() can't sync for > termination by itself, as it's called in the trigger PCM callback > inside a spinlock. Yes, I tried tasklet_kill() in terminate_all() but it didn't help due to inside a spinlock. I'll try to implement in a better place as you suggest. thanks. > > > Takashi > >> >>> >>> - Lars >>> >>> >>>> >>>> >>>> From d568a88e8f66ee21d44324bdfb48d2a3106cf0d1 Mon Sep 17 00:00:00 2001 >>>> From: Qiao Zhou >>>> Date: Wed, 9 Oct 2013 15:24:29 +0800 >>>> Subject: [PATCH] ASoC: soc-dmaengine: add mutex to protect runtime param >>>> >>>> under SMP arch, the dmaengine_pcm_dma_complete callback has the >>>> chance to run on one cpu while at the same time, the substream is >>>> released on another cpu. thus it may access param which is already >>>> freed. we need to add mutes to protect such access, and check PCM >>>> availability before using it. >>>> >>>> Signed-off-by: Qiao Zhou >>>> --- >>>> sound/soc/soc-dmaengine-pcm.c | 11 ++++++++++- >>>> 1 files changed, 10 insertions(+), 1 deletions(-) >>>> >>>> diff --git a/sound/soc/soc-dmaengine-pcm.c b/sound/soc/soc-dmaengine-pcm.c >>>> index 111b7d9..5917029 100644 >>>> --- a/sound/soc/soc-dmaengine-pcm.c >>>> +++ b/sound/soc/soc-dmaengine-pcm.c >>>> @@ -125,13 +125,22 @@ EXPORT_SYMBOL_GPL(snd_hwparams_to_dma_slave_config); >>>> static void dmaengine_pcm_dma_complete(void *arg) >>>> { >>>> struct snd_pcm_substream *substream = arg; >>>> - struct dmaengine_pcm_runtime_data *prtd = substream_to_prtd(substream); >>>> + struct dmaengine_pcm_runtime_data *prtd; >>>> + >>>> + mutex_lock(&substream->pcm->open_mutex); >>>> + if (!substream || !substream->runtime) { >>>> + mutex_unlock(&substream->pcm->open_mutex); >>>> + return; >>>> + } >>>> + >>>> + prtd = substream_to_prtd(substream); >>>> >>>> prtd->pos += snd_pcm_lib_period_bytes(substream); >>>> if (prtd->pos >= snd_pcm_lib_buffer_bytes(substream)) >>>> prtd->pos = 0; >>>> >>>> snd_pcm_period_elapsed(substream); >>>> + mutex_unlock(&substream->pcm->open_mutex); >>>> } >>>> >>>> static int dmaengine_pcm_prepare_and_submit(struct snd_pcm_substream >>>> *substream) >>> >>> _______________________________________________ >>> Alsa-devel mailing list >>> Alsa-devel@alsa-project.org >>> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel >>> >> -- Best Regards Qiao