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

On Fri, 7 Aug 2026 03:07:08 +0300, Mathias Nyman wrote:
> On 8/7/26 01:01, Michal Pecio wrote:
> > On Thu, 6 Aug 2026 16:25:15 +0300, Mathias Nyman wrote:  
> >> On 8/6/26 11:17, Michal Pecio wrote:  
> >>> Besides initialization, it's made -1 if and only if xHCI endpoint
> >>> state isn't Running at the time of submission. This effectively
> >>> means that to start a new "isoch data flow" aka "stream", class
> >>> driver must unlink the last remaining URB of the previous flow. If
> >>> the driver doesn't unlink (or tries too late), the endpoint goes
> >>> Running-Idle.  
> >>
> >> If class driver doesn't unlink remaining urbs then list won't be
> >> empty and you have the exact same situation.  
> > 
> > Obviously not, it can simply stop resubmitting and wait. Or there
> > may be no URBs left to unlink at all, when recovering from ring
> > underrun.
> > 
> > Then HW endpoint state will still be Running, not Stopped, and new
> > URBs will be scheduled into the past, doomed to complete with
> > -EXDEV. 
> 
> Isn't this exactly what we want?

This was discussed all the way back in Bugzilla - usb_submit_urb()
kerneldoc spells out what we want and Alan was clear that it reflects
what HCDs have been doing for 20 years:

 * If the driver is unable to keep up and the queue empties out, the
 * behavior for new submissions is governed by the URB_ISO_ASAP flag.
 * If the flag is set, or if the queue is idle, then the URB is always
 * assigned to the first available (and not yet expired) slot in the
 * endpoint's schedule.

"If the queue is idle" means all completions have already returned.
Class driver is aware of this and expected to consider the submission
a new stream. If it wishes to continue the stream that underrun, its
last chance is to submit from completion, when the queue is "active":

 * If the flag is not set and the queue is active then the URB is
 * always assigned to the next slot in the schedule following the end
 * of the endpoint's previous URB, even if that slot is in the past.

These rules were straightforward with synchronous giveback in IRQ.
To deal with BH, the following was done for ehci-hcd:

c7ccde6eac6d USB: see if URB comes from a completion handler
46c73d1d3ebc USB: EHCI: handle isochronous underruns with tasklets

Dylan Robinson correctly noted that this doesn't cover submissions
from other code while the completion is still waiting in the BH queue.
But that's a USB subsystem bug affecting ehci-hcd too, and IDK if any
in-tree driver cares. (It might be that no one knows that they care).

> Lets take the underrun case. Audio playback as an relatable example.

It was my impression while testing this that snd-usb-audio handles
playback underrun during synchronized duplex operation without cycling
through altsetting zero or unlinking playback URBs (none are left), so
it could possibly be affected.

It's also possible that the only effect would be wasting a few ms
to resubmit URBs until it catches up with MFINDEX. But then, another
concern of users was to recover from glitches as fast as possible.

Regards,
Michal