Re: [PATCH] usb: xhci: Fix isochronous scheduling regression

[Alan Stern <stern@rowland.harvard.edu>]

On Fri, Aug 21, 2026 at 06:03:50PM +0200, Michal Pecio wrote:
> On Fri, 21 Aug 2026 10:44:07 -0400, Alan Stern wrote:
> > Maybe it's time to correct the hcd_periodic_completion_in_progress() 
> > implementation.
> > 
> > For instance, we could add an atomic giveback_count field to the 
> > usb_host_endpoint struct.  The HCD would increment the field (while 
> > still holding its private lock) before doing a giveback, and 
> > __usb_hcd_giveback_urb() would decrement the field after calling the 
> > completion handler.
> > 
> > What do you think?
> 
> I would go as far as incrementing it on successful usb_submit_urb() and
> completely doing away with those list_empty(td_list) checks in HCDs.
> 
> I wrote an xhci-only (less compilation and module reloading) prototype
> which relies on hijacking completions of isoc URBs for counting, it
> worked, results identical as with the standard solution in a few test
> runs with snd-usb-audio.
> 
> Theoretical race condition: it seems we can't prevent new submissions
> after completion releases its lock and class driver considers the pipe
> idle, but before the counter is decremented to zero. That would be
> another case of "scheduling to the past" unexpectedly.
> 
> Seems low probability, but this type of bug hasn't existed so far, we
> generally have the opposite problem.
> 
> Or does it exist in non-BH HCDs?

It does.  It's impossible for a non-BH HCD to reacquire its private lock 
exactly when the completion handler returns, so somewhere around that 
time the class driver and the HCD will inevitably have differing 
opinions about whether the iso queue is empty.

> Maybe the documented API guarantee just isn't feasible?

I'm not averse to your proposed solution.  As for that race, just update 
the API documentation to take it into account.  Seriously -- it's an 
unavoidable defect and that's all we can do about it.

Possible alternative: Make the URB_ISO_ASAP flag take precedence over 
the "queue is non-empty" condition.

> Entirely out of the box alternative: new URB flag. And really, if
> only xhci-hcd existed, it wouldn't be hard to even implement explicit
> start_frame requests or hints as MOTU wished for, which would make
> the whole business of starting synchronized endpoints cleaner.

Yes.  Something like an URB_USE_START_FRAME flag.  It wouldn't help much 
for starting a single endpoint, but it would allow drivers to 
synchronize new iso streams with existing ones more easily.  (Although 
there would be a little uncertainty in cases where the schedules for the 
two streams don't have the same phase.)

Alan Stern