All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mathias Nyman <mathias.nyman@linux.intel.com>
To: Phil Elwell <phil@raspberrypi.com>,
	Mathias Nyman <mathias.nyman@intel.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Jonathan Bell <jonathan@raspberrypi.com>,
	linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] xhci: guard accesses to ep_state in xhci_endpoint_reset()
Date: Wed, 1 Sep 2021 12:21:08 +0300	[thread overview]
Message-ID: <3830571c-566c-ef13-bc08-60206a634253@linux.intel.com> (raw)
In-Reply-To: <20210831160259.2392459-1-phil@raspberrypi.com>

On 31.8.2021 19.02, Phil Elwell wrote:
> From: Jonathan Bell <jonathan@raspberrypi.com>
> 
> See https://github.com/raspberrypi/linux/issues/3981

Thanks, so in a nutshell the issue looks something like:

[827586.220071] xhci_hcd 0000:01:00.0: WARN Cannot submit Set TR Deq Ptr
[827586.220087] xhci_hcd 0000:01:00.0: A Set TR Deq Ptr command is pending.
[827723.160680] INFO: task usb-storage:93 blocked for more than 122 seconds.

The blocked task is probably because xhci driver failed to give back the
URB after failing to submit a "Set TR Deq Ptr" command. This part should
be fixed in:
https://lore.kernel.org/r/20210820123503.2605901-4-mathias.nyman@linux.intel.com
which is currently in usb-next, and should be in 5.15-rc1 and future 5.12+ stable.

> 
> Two read-modify-write cycles on ep->ep_state are not guarded by
> xhci->lock. Fix these.
> 

This is probably one cause for the "Warn Cannot submit Set TR Deq Ptr A Set TR
Deq Ptr command is pending" message.
Another possibility is that with UAS and streams we have several transfer rings
per endpoint, meaning that if two TDs on separate stream rings on the same 
endpoint both stall, or are cancelled we could see this message.

The SET_DEQ_PENDING flag in ep->ep_state should probably be per ring, not per
endpoint. Then we also need a "rings_with_pending_set_deq" counter per endpoint
to keep track when all set_tr_deq commands complete, and we can restart the endpoint  

Anyway, my patch linked above together with this patch should make these errors
a lot more harmless.

Let me know if you can trigger the issue with both these patches applied.

I'll add your patch to the queue as well.

Thanks
-Mathias

  reply	other threads:[~2021-09-01  9:19 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-31 16:02 [PATCH] xhci: guard accesses to ep_state in xhci_endpoint_reset() Phil Elwell
2021-09-01  9:21 ` Mathias Nyman [this message]
2021-09-01 13:15   ` Phil Elwell
2021-09-01 18:51     ` Jonathan Bell

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=3830571c-566c-ef13-bc08-60206a634253@linux.intel.com \
    --to=mathias.nyman@linux.intel.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=jonathan@raspberrypi.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=mathias.nyman@intel.com \
    --cc=phil@raspberrypi.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.