From mboxrd@z Thu Jan 1 00:00:00 1970 From: Laurent Pinchart Date: Wed, 20 May 2015 08:28:10 +0000 Subject: Re: [Bug Report] SOUND: Kernel panic occurs when playback Message-Id: <477370794.VLkyzhOzzL@avalon> List-Id: References: <55530BE5.2080402@jinso.co.jp> In-Reply-To: <55530BE5.2080402@jinso.co.jp> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: linux-sh@vger.kernel.org Hi Morimoto-san, On Friday 15 May 2015 08:32:39 Kuninori Morimoto wrote: > Hi Laurent > > Can I ask you about DMAEngine and spin_lock ? > > > > > We are testing Linux upstream version-4.1-rc2 on Koelsch and Lager. > > > > We found a bug(kernel panic) on SOUND driver when Playback a sound > > > > file. > > > > > > > > The kernel panic logs is showed as below: > > > > "root@linaro-nano:~# aplay audio/Track\ 10.wav > > > > Playing WAVE 'audio/Track 10.wav' : Signed 16 bit Little Endian, > > > > Rate 44100 Hz, Stereo > > > > Unable to handle kernel NULL pointer dereference at virtual address > > > > 00000048 > > > > pgd = edfac000 > > > > [00000048] *pgdn0f0831, *pte000000, *ppte000000 > > > > Internal error: Oops: 17 [#1] SMP ARM > > > > CPU: 0 PID: 2009 Comm: aplay Not tainted 4.1.0-rc2-dirty #4 > > > > Hardware name: Generic R8A7790 (Flattened Device Tree) > > > > ..." > > In my debug, this issue happens under DMAEngine, > and it depends on spin_lock (I think we talked this topic before...) > but I don't know which one is wrong, DMAEngine or Sound ? > > Now, sound driver is calling DMAEngine API under spin_lock_irqsave > > spin_lock_irqsave(xxxx); > ... > dmaengine_prep_dma_cyclic(); > ... > spin_unlock_irqrestore(); > > dmaengine_prep_dma_cyclic() does many things, > and it is using spin_unlock_irq(). > It re-enabled sound interrupt before calling spin_unlock_irqrestore() > > My question is which one is correct solution ?? > - sound driver shouldn't call DMAEngine API under spin_lock_irqsave() ? > - DMAEngine driver should use spin_lock_irqsave() instead of > spin_lock_irq() ? I'm sorry to have missed this e-mail. I've now replied to your patch series fixing the problem. I believe the best solution would be to refactor the DMA engine API to make reuse of descriptors possible. That way drivers could allocate descriptors at initialization time in non-atomic context, and then use them in atomic contexts (including interrupt handlers) without requiring reallocation. That's a longer term problem, and we need a fix now for the sound bug, so switching to spin_lock_irqsave() seems fine to me. -- Regards, Laurent Pinchart