On Tue, 26 Mar 2019 08:49:33 +0100,
<twischer@de.adit-jv.com> wrote:

From: Timo Wischer <twischer@de.adit-jv.com>

snd_pcm_link() can be called by the user as long as the device is not
yet started. Therefore currently a driver which wants to iterate over
the linked substreams has to do this at the start trigger. But the start
trigger should not block for a long time. Therefore there is no callback
which can be used to iterate over the linked substreams without delaying
the start trigger.
This patch introduces a new callback function which will be called after
the linked substream list was updated by snd_pcm_link(). This callback
function is allowed to block for a longer time without interfering the
synchronized start up of linked substreams.

Signed-off-by: Timo Wischer <twischer@de.adit-jv.com>

Well, the idea appears interesting, but I'm afraid that the
implementation is still racy.  The place you're calling the new
callback isn't protected, hence the stream can be triggered while
calling it.  That is, even during operating your loopback link_changed
callback, another thread is able to start the stream.

Hi Takashi,

As far as I got you mean the following scenario:

• snd_pcm_link() is called for a HW sound card
• loopback_snd_timer_link_changed()
• loopback_snd_timer_open()
• spin_lock_irqsave(&dpcm->cable->lock, flags);
• snd_pcm_start() called for aloop sound card
• loopback_trigger()
• spin_lock(&cable->lock) -> has to wait till loopback_snd_timer_open() calls spin_unlock_irqrestore()

So far snd_pcm_start() has to wait for loopback_snd_timer_open().

• loopback_snd_timer_open() will continue with
• dpcm->cable->snd_timer.instance = NULL;
• spin_unlock_irqrestore()
• loopback_trigger() can enter the lock
• loopback_snd_timer_start() will fail with -EINVAL due to (loopback_trigger == NULL)

At least this will not result into memory corruption due to race or any other wired behavior.
But my expectation is that snd_pcm_link(hw, aloop) or snd_pcm_link(aloop, hw) is only called by the application calling snd_pcm_start(aloop)
because the same aloop device cannot be opened by multiple applications at the same time.

Do you see an use case where one application would call snd_pcm_start() in parallel with snd_pcm_link() (somehow configuring the device)?
May be we should add an additional synchronization mechanism in pcm_native.c to avoid call of snd_pcm_link() in parallel with snd_pcm_start().

thanks,

Takashi