The Linux Kernel Mailing List
 help / color / mirror / Atom feed
From: Michal Pecio <michal.pecio@gmail.com>
To: Mathias Nyman <mathias.nyman@linux.intel.com>
Cc: Mathias Nyman <mathias.nyman@intel.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] usb: xhci: Fix isochronous scheduling regression
Date: Fri, 21 Aug 2026 16:46:45 +0200	[thread overview]
Message-ID: <20260821164645.047c9020.michal.pecio@gmail.com> (raw)
In-Reply-To: <6804ed57-8405-4310-a64f-20c400bffde0@linux.intel.com>

On Fri, 21 Aug 2026 15:05:53 +0300, Mathias Nyman wrote:
> On 8/21/26 12:47, Michal Pecio wrote:
> > Fix this by using the best criteria we can muster, taken from ehci-hcd.
> > This brings false negative rate back to zero and false positive rate to
> > less than ever before in xhci-hcd. Traditional logic was equivalent to:
> > 
> > 	if (list_empty(&ep_ring->td_list) ||
> > 	    GET_EP_CTX_STATE(ep_ctx) != EP_STATE_RUNNING)
> > 		// consider this URB a new stream
> > 
> > While free of false negatives, it had easily avoidable false positives:
> > * no check for completion in progress when the list is empty
> > * the ep_ctx check doesn't make up for it at all, but it adds a race -
> >    EP state can remain "stopped" for a while after the first submission  
> 
> I would still prefer:
> if (list_empty(&ep_ring->td_list) && GET_EP_CTX_STATE(ep_ctx) != EP_STATE_RUNNING)
> to detect the start of a new isoch stream.
> 
> It has zero false positives mid stream, and fixes the "stopped" state
> race case.
> 
> I can't figure out when the false negatives it introduces is an issue.
> 
> If you can show me a usecase where a driver or specification is
> designed to let an isoch endpoint intentionally run dry, leaving it
> in running/idle, and then continue queuing URBs expecting ASAP
> scheduling, then we can change it.

Well, snd-usb-audio does. In snd_complete_urb(), in the
usb_pipeout(ep->pipe) case and ep->lowlatency_playback, which is
the default (controlled by 'lowlatency' module parameter).

It adds the completed URB to a queue of "ready" URBs and then tries
to submit as many URBs from that queue as possible, which may be
zero if there is no data to send. Then pending URB count is checked
for zero, which triggers xrun recovery in ALSA core.

I haven't dug into ALSA, but the observable outcome is:

snd_usb_endpoint_stop() is called on both IN and OUT, which unlinks
IN URBs and does nothing to OUT (zero URBs left).

snd_usb_endpoint_start() is called on both endpoints to submit URBs.

See dynamic debug below. Notably, "Setting usb interface" does appear,
but only at the end, when I killed jackd. And it seems suboptimal to
demand that drivers spend time on control requests in such cases.

[146029.539992] xhci_hcd 0000:06:00.0: Stopped on Transfer TRB for slot 1 ep 2
[146029.540031] xhci_hcd 0000:06:00.0: Underrun event on slot 1 ep 1
[146029.542857] usb 6-2: Starting data EP 0x81 (running 0)
[146029.542949] usb 6-2: 12 URBs submitted for EP 0x81
[146029.542958] usb 6-2: 2:2 Start Capture PCM
[146029.542962] usb 6-2: Starting data EP 0x1 (running 0)
[146029.542991] usb 6-2: 2 URBs submitted for EP 0x1
[146029.542996] usb 6-2: 1:3 Start Playback PCM
[146029.543799] usb 6-2: notify_xrun ep 01 from line 579	<-- my patch
[146029.543816] usb 6-2: Stopping data EP 0x81 (running 1)
[146029.543861] usb 6-2: 2:2 Stop Capture PCM
[146029.543863] usb 6-2: Stopping data EP 0x1 (running 1)
[146029.543865] usb 6-2: 1:3 Stop Playback PCM
[146029.543882] xhci_hcd 0000:06:00.0: Stopped on Transfer TRB for slot 1 ep 2
[146029.543886] xhci_hcd 0000:06:00.0: Underrun event on slot 1 ep 1
[146029.545849] usb 6-2: Starting data EP 0x81 (running 0)
[146029.545962] usb 6-2: 12 URBs submitted for EP 0x81
[146029.545969] usb 6-2: 2:2 Start Capture PCM
[146029.545972] usb 6-2: Starting data EP 0x1 (running 0)
[146029.545998] usb 6-2: 2 URBs submitted for EP 0x1
[146029.546001] usb 6-2: 1:3 Start Playback PCM
[146029.546793] usb 6-2: notify_xrun ep 01 from line 579
[146029.546814] usb 6-2: Stopping data EP 0x81 (running 1)
[146029.546876] usb 6-2: 2:2 Stop Capture PCM
[146029.546881] usb 6-2: Stopping data EP 0x1 (running 1)
[146029.546885] usb 6-2: 1:3 Stop Playback PCM
[146029.546903] xhci_hcd 0000:06:00.0: Underrun event on slot 1 ep 1
[146029.546909] xhci_hcd 0000:06:00.0: Stopped on Transfer TRB for slot 1 ep 2
[146029.557837] usb 6-2: Closing EP 0x81 (count 1)
[146029.557845] usb 6-2: Setting usb interface 2:0 for EP 0x81
[146029.558636] usb 6-2: EP 0x81 closed
[146029.558664] usb 6-2: Closing EP 0x1 (count 1)
[146029.558666] usb 6-2: Setting usb interface 1:0 for EP 0x1
[146029.559505] usb 6-2: EP 0x1 closed

0000:06:00.0 is ASM1042, USB 3.0, no CFC. Hence no Missed Service
before the Underrun. OUT URBs seem to complete successfully, but
ALSA apparently doesn't feed playback data until the IN endpoint
starts moving, which is scheduled to happen later, so OUT underruns
again and the whole mess repeats.

On CFC, all OUT URBs complete 1 uframe after submisison with MSE,
so it's pretty much guaranteed to happen before any IN is scheduled.

Up to v7.2, such xrun is only a brief audible glitch. On usb-next
it loops like that for seconds to minutes before recovering by
some lucky miracle (IDK why that happens, but sometimes it panics
and begins unlinking OUT URBs before the last one completes).

Hence the idea to not rock the boat too much and stay with zero
false negatives and only the unavoidable false postives which have
been here in HCD_BH drivers for ages.

The whole thing is madness. It nobody has issues with wasting 4 bytes
per endpoint, I think it could be fixed by counting all pending URBs
from submit to complete. Changes to ehci/xhci-hcd will be minimal. Or
add a flag which overrides this guesswork. But too late for v7.3.

> At least we should add a debug message so audio/video developers can
> find the reason for the glitches:
> 
> if (list_empty(&ep_ring->td_list) &&
>      !hcd_periodic_completion_in_progress(xhci_to_hcd(xhci), urb->ep)) {
> 		if (GET_EP_CTX_STATE(ep_ctx) == EP_STATE_RUNNING)
>  			xhci_dbg(xhci, "Starting new isoc stream on running endpoint at uframe %d, killing sync..
> 		xep->next_uframe = -1;
> }

That seems harmless, although the same information can be obtained by
checking urb->start_frame after submission.

Fun fact, I found only two in-tree drivers which do anything meaningful
with it. Particularly, if snd-usb-audio submits IN and OUT across frame
boundary, it can only recover by xruning and trying again, meh.

Regards,
Michal

  reply	other threads:[~2026-08-21 14:46 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-21  9:47 [PATCH] usb: xhci: Fix isochronous scheduling regression Michal Pecio
2026-08-21 12:05 ` Mathias Nyman
2026-08-21 14:46   ` Michal Pecio [this message]
2026-08-21 14:44 ` Alan Stern
2026-08-21 16:03   ` Michal Pecio
2026-08-22  2:38     ` Alan Stern

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260821164645.047c9020.michal.pecio@gmail.com \
    --to=michal.pecio@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=mathias.nyman@intel.com \
    --cc=mathias.nyman@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox