Re: [RFT PATCHv3 1/3] xhci: fix frame id calculation and checks for isoc URBs

On Thu, 13 Aug 2026 16:19:13 +0300, Mathias Nyman wrote:
> On 8/13/26 01:56, Michal Pecio wrote:
> > This does make a difference with snd-usb-audio. If I run
> > 
> > jackd -d alsa -d hw:... -p 24 -n 2
> > 
> > Ring Underrun and Missed Service are reported every now and then,
> > sometimes repeatedly, and it doesn't take long to enter this loop:
> > 
> > 1. playback Ring Underrun (xHCI EP state is Running)
> > 2. capture URB unlink (xHCI EP state is Stopped)
> > 3. multiple capture URBs scheduled to the same start_frame before
> >     EP state becomes Running  
> 
> agree, and this needs to be fixed with something like:
> 
> -       if (GET_EP_CTX_STATE(ep_ctx) != EP_STATE_RUNNING)
> +       if (list_empty(&ep_ring->td_list) &&
> +           GET_EP_CTX_STATE(ep_ctx) != EP_STATE_RUNNING)

Sure, it fixes 3, but not the "4, 5, goto 1" sequence below.

> > 4. new playback URB is scheduled into a distant past due Running EP
> > 5. 2 uframes later OUT endpoint reports Missed Service for all
> >    those misscheduled TDs
> > 6. goto 1

Example:

AFAICT, notify_xrun means userspace didn't submit new playback
samples before the last OUT URB completed. No OUT URBs are left,
endpoint state is Running-Idle, the driver only unlinks IN URBs.

[300299.231743] usb 10-1: notify_xrun 01 from 577
[300299.231749] usb 10-1: Stopping data EP 0x81 (running 1)
[300299.231815] usb 10-1: 2:2 Stop Capture PCM
[300299.231817] usb 10-1: Stopping data EP 0x1 (running 1)
[300299.231819] usb 10-1: 1:3 Stop Playback PCM

Some time later, IN is restarted and scheduled 0x11 uframes ahead.
On second submission we see the "ep state not yet running" race.

[300299.233588] usb 10-1: Starting data EP 0x81 (running 0)
[300299.233616] xhci_hcd 0000:08:00.0: xdebug ep 81 uframe 0f47: scheduling new stream for 0f58
[300299.233628] xhci_hcd 0000:08:00.0: xdebug ep 81 uframe 0f47: pending URBs but not running
[300299.233705] usb 10-1: 12 URBs submitted for EP 0x81
[300299.233710] usb 10-1: 2:2 Start Capture PCM

Then OUT is restarted and mis-scheduled to a random uframe 1018,
far away from IN URBs, because xHCI endpoint state is Running.

[300299.233713] usb 10-1: Starting data EP 0x1 (running 0)
[300299.233722] xhci_hcd 0000:08:00.0: xdebug ep 01 uframe 0f48: running without pending URBs
[300299.233726] xhci_hcd 0000:08:00.0: xdebug ep 01 uframe 0f48: mis-scheduling new stream for 1018
[300299.233739] usb 10-1: 2 URBs submitted for EP 0x1
[300299.233741] usb 10-1: 1:3 Start Playback PCM

And this smells like regression, because the original condition was:

if (list_empty || EP_STATE != RUNNING)
	start new stream

so the driver would never *fail* to start a new stream when no URBs
are pending at all, it could only attempt to start a new stream while
URBs are pending, due to the pointless EP_STATE test or BH giveback.

> > Changing the condition from "running endpoint" back to "empty list"
> > breaks this pathological cycle. Experimental patch below.
> >   
> 
> But this is different from incorrectly assuming a new stream started
> mid stream just because list_empty(&ring->td_list) is true and the
> urb completion workqueue isn't at this instance processing a work
> item belonging to this endpoint.

Yes, this behavior is inconsistent with documentiation and disliked
by one out-of-tree driver developer (probably rightly so), but it's
been this way for over a decade, both in xhci-hcd and ehci-hcd.

And it doesn't affect drivers that resubmit from completion, which is
apparently good enough that people stopped complaining when Alan added
the "or completion is running now on this CPU" hack.

If somebody is very bothered by this, it seems it could be fixed, for
example by counting submitted URBs and having the BH worker decrement
the count after each finished completion.

In fact, I wrote a hack which does this for xhci-hcd (by replacing
the complete callback with a wrapper that performs counting). So
far I found no case where this check makes a difference, but finding
one could become an argument for fixing this in core for real.

Regards,
Michal