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
