[alsa-devel] [question about DPCM] dpcm_be_disconnect race condition

[Mark Brown]

On Wed, Aug 27, 2014 at 04:15:00PM +0800, Qiao Zhou wrote:
> Hi Liam, Mark
> 
> I'm developing audio with DPCM on one phone platform, and met one kernel
> panic issue related with dpcm_be_connect. Could you please help to suggest?

Adding Liam.

> Taking audio playback for example:
> 1. User space writes all the data into audio buffer. When the blocking write
> returns, it closes audio path(which updates dapm and triggers
> soc_dpcm_runtime_update).
> 2. When user space closes audio path, kernel audio driver(DMA) is still
> transferring data from audio buffer to audio peripheral. So DMA interrupt
> will come, and the interrupt handler runs on different cpu comparing to
> which the task to close audio path runs on.
> 3. Due to system schedule, below case may happen.
> 
>            cpu0                                      cpu1
> 
> soc_dapm_mixer_update_power ->        |       DMA interrupt ->
> soc_dpcm_runtime_update ->            |       snd_pcm_period_elapsed ->
> dpcm_be_disconnect(old) ->            |       snd_pcm_update_hw_ptr0 ->
>  __________________________           |       xrun ->
> |                         |           |       snd_pcm_stop ->
> |list_del(&dpcm->list_be) |           |       dpcm_fe_dai_trigger ->
> |list_del(&dpcm->list_fe) |           |       dpcm_be_dai_trigger ->
> |kfree(dpcm);             |           |       list_for_each_entry(dpcm..
> |_________________________|           |
> 
> On CPU1, list_for_each_entry(dpcm, &fe->dpcm[stream].be_clients, list_be)
> will get dpcm and corresponding be, but there is no protection against the
> dpcm_be_disconnect on cpu0. Related dpcm for the BE may be invalid/freed
> when cpu1 tries to access it.

> Should we require user space to make sure audio transfer ends before closing
> audio path to avoid such issue? I'm not sure if I missed anything or used it
> wrongly.

Well, it should do that in general if it wants to make sure that the
audio actually gets played but the kernel should protect against this.
We should be holding a lock whenever we update the lists.

> Thanks in advance!
> -- 
> 
> Best Regards
> Qiao
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://mailman.alsa-project.org/pipermail/alsa-devel/attachments/20140827/ec2c68b4/attachment.sig>