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

[Mathias Nyman]

On 8/22/26 05:38, Alan Stern wrote:
> 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.

This sounds like a good idea.

Regarding theoretical race conditions:  If URBs for some odd and unknown
reason are submitted by several callers to the same endpoint (no idea why
or how this would happen) then, usb_hcd_submit_urb() could theoretically call
hcd->driver->urb_enqueue(hcd) for the second URB right when .urb_enqueue(hcd)
returned for the first URB, but _before_ increasing the atomic counter.

In this case hcd would incorrectly assume both first and second URB start a
new isoc stream. This would be avoided by hcd still checking list_empty(td_list)

Anyway, suggested solution still sounds like an improvement compared to what
we currently have.

> > 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.

xhci driver does this. If URB_ISO_ASAP is set then xhci driver always sets
the SIA "Start Isoch ASAP" flag for the transfer blocks.

The urb->start_frame value set by xhci driver might not be correct in this
case.
Thanks
Mathias