Hi, On Fri, Mar 17, 2023 at 07:51:27PM +0000, John Keeping wrote: > snd_usb_queue_pending_output_urbs() may be called from > snd_pcm_ops::ack() which means the PCM stream is locked. > > For the normal case where the call back into the PCM core is via > prepare_output_urb() the "_under_stream_lock" variant of > snd_pcm_period_elapsed() is called, but when an error occurs and the > stream is stopped as XRUN then snd_pcm_xrun() tries to recursively lock > the stream which results in deadlock. > > Follow the example of snd_pcm_period_elapsed() by adding > snd_pcm_xrun_under_stream_lock() and use this when the PCM substream > lock is already held. > > Signed-off-by: John Keeping <john@xxxxxxxxxxxx> > --- > include/sound/pcm.h | 1 + > sound/core/pcm_native.c | 28 ++++++++++++++++++++++++---- > sound/usb/endpoint.c | 18 +++++++++++------- > 3 files changed, 36 insertions(+), 11 deletions(-) The name of added kernel API implies me that you refer to existent 'snd_pcm_period_elapsed_under_stream_lock()' which I added to Linux v5.14. In my opinion, unlike the version of period elapsed API, the version of XRUN API seems not to be necessarily required to ALSA PCM core, since PCM device drivers can implement .pointer callback in the part of PCM operation. When the callback returns SNDRV_PCM_POS_XRUN, ALSA PCM application get occurence of XRUN as a result of any operation relevant to hwptr movement (e.g. SNDRV_PCM_IOCTL_HWSYNC). Therefore I think it possible to fix the issue without the proposed kernel API. I can assume some scenario: 1. Failure at tasklet for URB completion It is softIRQ context. The stream lock is not acquired. It doesn't matter to call current XRUN API. 2. Failure at PCM operation called by ALSA PCM application It is process context. The stream lock is acquired before calling driver code. When detecting any type of failure, driver code stores the state. Then .pointer callback should return SNDRV_PCM_IOCTL_HWSYNC refering to the state. Of course, I'm not a developer for USB audio devices. I'm just a developer for the other type of packet-oriented drivers (IEC 61883-1/6 packet streaming engine for audio and music unit in IEEE 1394 bus). So I do not get every part of USB driver. However, from my experience for the packet-oriented drivers, I have the above concern about adding the new XRUN API. I apologize if miss-hitting the point for your issue. Regards Takashi Sakamoto
