>> Using snd_pcm_stream_lock_irqsave(be_substream, flags); will prevent
>> multiple triggers indeed, but the state management is handled by
>> dpcm_lock, so I think we have to use dpcm_lock/mutex in all BE transitions.
>>
>> if ((be->dpcm[stream].state != SND_SOC_DPCM_STATE_PREPARE) &&
>> (be->dpcm[stream].state != SND_SOC_DPCM_STATE_STOP) &&
>> (be->dpcm[stream].state != SND_SOC_DPCM_STATE_PAUSED))
>>
>> if ((be->dpcm[stream].state != SND_SOC_DPCM_STATE_PREPARE) &&
>> (be->dpcm[stream].state != SND_SOC_DPCM_STATE_STOP) &&
>> (be->dpcm[stream].state != SND_SOC_DPCM_STATE_PAUSED))
>
> The stream lock can be put around those appropriate places, I suppose?
I doubled checked the code a bit more, and all functions using
be->dpcm[stream].state and be->dpcm[stream].users are protected by the
card->mutex.
The exceptions are dpcm_be_dai_trigger() and dpcm_show_state() so we
probably don't need to worry too much about these fields.
I am more nervous about that the dpcm_lock was supposed to protect. It
was added in "ASoC: dpcm: prevent snd_soc_dpcm use after free" to solve
a race condition, according to the commit log between
void dpcm_be_disconnect(
...
list_del(&dpcm->list_be);
list_del(&dpcm->list_fe);
kfree(dpcm);
...
and
for_each_dpcm_fe()
for_each_dpcm_be*()
That would suggest that every one of those loops should be protected,
but that's not the case at all. In some cases the spinlock is taken
*inside* of the loops.
I think this is what explains the problem reported by Gyeongtaek Lee in
https://lore.kernel.org/alsa-devel/002f01d7b4f5$c030f4a0$4092dde0$@samsung.com/
the for_each_dpcm_be() loop in dpcm_be_dai_trigger() is NOT protected.
But if we add a spin-lock in there, the atomicity remains a problem.
I think the only solution is to follow the example of the PCM case,
where the type of lock depends on the FE types, with the assumption that
there are no mixed atomic/non-atomic FE configurations.
