On Tue, 21 Mar 2023 05:02:58 +0100, Takashi Sakamoto wrote:
Hi,
On Mon, Mar 20, 2023 at 03:28:38PM +0100, Takashi Iwai wrote:
The recent support of low latency playback in USB-audio driver made the snd_usb_queue_pending_output_urbs() function to be called via PCM ack ops. In the new code path, the function is performed alread in
's/alread/already/' or slang.
the PCM stream lock. The problem is that, when an XRUN is detected, the function calls snd_pcm_xrun() to notify, but snd_pcm_xrun() is
's/the function/the function/'
Corrected at applying.
supposed to be called only outside the stream lock. As a result, it leads to a deadlock of PCM stream locking.
For avoiding such a recursive locking, this patch adds an additional check to the code paths in PCM core that call the ack callback; now it checks the error code from the callback, and if it's -EPIPE, the XRUN is handled in the PCM core side gracefully. Along with it, the USB-audio driver code is changed to follow that, i.e. -EPIPE is returned instead of the explicit snd_pcm_xrun() call when the function is performed already in the stream lock.
Practically, the implementation of 'pcm_hw' in alsa-lib never evaluates the return value (see 'snd_pcm_hw_mmap_commit()' and the others). I guess that it is inconvenient for the low-latency mode of USB Audio device class driver for the case of failure.
My additional concern is PCM indirect layer, since typically the layer is used by drivers with SNDRV_PCM_INFO_SYNC_APPLPTR. But as long as I read, the change does not matter to them.
I find rather that's an extra bonus :) It allows the existing code to give a more proper error handling. For example, the indirect PCM helper returned -EINVAL, but this can be switched to -EPIPE for stopping the stream.
I'm going to submit the patch together with the documentation updates.
thanks,
Takashi