[alsa-devel] [PATCH 09/10] ALSA: pcm: Add snd_pcm_ops for snd_pcm_link()

[Timo Wischer]

On 3/26/19 09:35, Takashi Iwai wrote:
> On Tue, 26 Mar 2019 08:49:33 +0100,
> <twischer at de.adit-jv.com> wrote:
>> From: Timo Wischer <twischer at 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 at 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
      o loopback_snd_timer_link_changed()
      o loopback_snd_timer_open()
      o spin_lock_irqsave(&dpcm->cable->lock, flags);
  * snd_pcm_start() called for aloop sound card
      o loopback_trigger()
      o 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
      o dpcm->cable->snd_timer.instance = NULL;
      o spin_unlock_irqrestore()
  * loopback_trigger() can enter the lock
      o 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