[alsa-devel] Questions on locking, snd-usb-caiaq

[Takashi Iwai]

At Sun, 20 Sep 2009 13:07:05 +0100 (BST),
Mark Hills wrote:
> 
> I fixed some race issues in snd-usb-caiaq, but have some questions on the 
> interface between the driver and ALSA.
> 
> First fix is a lock in pointer(), which caused snd_pcm_update_hw_ptr_pos() 
> to report an out of range buffer position (even though it was sucessfully 
> corrected to zero).
> 
> Second is a lock as part of trigger(). On testing, this appears to fix an 
> issue where white noise could sometimes be transferred by the driver.

Both look fine to me.

> According to the documentation, trigger() and pointer() are called with 
> local interrupts disabled. This means they should really be non-irqsave 
> locks?

They can be non-irqsave one, but not necessarily to be.

> (http://www.alsa-project.org/~tiwai/writing-an-alsa-driver/ch05s08.html)
> 
> Even for callbacks documented as atomic, if read_completed() and 
> write_completed() can be called at any time interrupts are enabled (eg. on 
> another CPU) it seems that additional locking my be required. Candidates 
> are:
> 
>    snd_usb_caiaq_substream_open
>    snd_usb_caiaq_substream_close
>    snd_usb_caiaq_pcm_hw_free
>    snd_usb_caiaq_pcm_prepare
> 
> Are there any guarantees on these functions which mean the lock is not 
> required?

They are all protected via PCM substream lock, thus they aren't racy.

> To lock in prepare() would (I think) require demoting a 
> spin_lock_irqsave() to a spin_lock() to send initialisation commands. If 
> so, what is the best design for this?

The best is to sync the deactivation.  Right now, activation /
deactivation are done in asynchronous way in caiaq driver.
At the beginning of prepare callback, you should be sure that all URBs
are killed, then proceed the rest preparation job.


Takashi