On Sun, 26 Apr 2020 19:12:41 +0200, Alexander Tsoy wrote: > > В Чт, 23/04/2020 в 21:24 +0300, Alexander Tsoy пишет: > > В Чт, 23/04/2020 в 19:35 +0200, Takashi Iwai пишет: > > > On Thu, 23 Apr 2020 19:29:08 +0200, > > > Takashi Iwai wrote: > > > > On Thu, 23 Apr 2020 18:57:34 +0200, > > > > Alexander Tsoy wrote: > > > > > And some further notes: > > > > > > > > > > - I removed locking from snd_usb_endpoint_next_packet_size() > > > > > and > > > > > this > > > > > seems completely fixed an issue with large URBs I reported > > > > > here: > > > > > > > > > > https://bugzilla.kernel.org/show_bug.cgi?id=199327#c28 > > > > > > > > > > So playing at 96 kHz, driver packs 48 frames per URB and no > > > > > more > > > > > audio > > > > > discontinuities. > > > > > > > > Hmm, that's weird. > > > > > > > > If removing the lock from snd_usb_endpoint_next_packet_size() > > > > really > > > > fixes the problem, it implies the lock contention. But as far as > > > > I > > > > see the code performed in this lock isn't conflicting so > > > > much. The > > > > URB processing shouldn't happen in parallel for the same EP. > > > > > > BTW, one potential racy code I found while looking at the code is > > > the > > > list management in queue_pending_output_urbs(). The fix patch is > > > below. > > > > OK, it seems like it was just a luck. I'm still getting clicking > > artifacts with and without your patch, with and without locking. Will > > investigate further. > > After more testing, it seems that with large URBs the transfer size is > too large for timer-based scheduling to work correctly in pulseaudio. > And looks like pulseaudio sometimes fail to adjust tsched watermark or > something like that. And it is not 100% reproducible. Aha, that's interesting. Basically that's natural that the timer scheduling doesn't work reliably for the device buffer management like USB-audio, but it was enabled on PA because it works in most of cases. Takashi
