[PATCH 3.2 53/95] xhci: Manually give back cancelled URB if we can't queue it for cancel
From: Ben Hutchings
Date: Sun Jul 16 2017 - 10:59:24 EST
3.2.91-rc1 review patch. If anyone has any objections, please let me know.
------------------
From: Mathias Nyman <mathias.nyman@xxxxxxxxxxxxxxx>
commit d3519b9d9606991a1305596348b6d690bfa3eb27 upstream.
xhci needs to take care of four scenarios when asked to cancel a URB.
1 URB is not queued or already given back.
usb_hcd_check_unlink_urb() will return an error, we pass the error on
2 We fail to find xhci internal structures from urb private data such as
virtual device and endpoint ring.
Give back URB immediately, can't do anything about internal structures.
3 URB private data has valid pointers to xhci internal data, but host is
not responding.
give back URB immedately and remove the URB from the endpoint lists.
4 Everyting is working
add URB to cancel list, queue a command to stop the endpoint, after
which the URB can be turned to no-op or skipped, removed from lists,
and given back.
We failed to give back the urb in case 2 where the correct device and
endpoint pointers could not be retrieved from URB private data.
This caused a hang on Dell Inspiron 5558/0VNM2T at resume from suspend
as urb was never returned.
[ 245.270505] INFO: task rtsx_usb_ms_1:254 blocked for more than 120 seconds.
[ 245.272244] Tainted: G W 4.11.0-rc3-ARCH #2
[ 245.273983] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
[ 245.275737] rtsx_usb_ms_1 D 0 254 2 0x00000000
[ 245.277524] Call Trace:
[ 245.279278] __schedule+0x2d3/0x8a0
[ 245.281077] schedule+0x3d/0x90
[ 245.281961] usb_kill_urb.part.3+0x6c/0xa0 [usbcore]
[ 245.282861] ? wake_atomic_t_function+0x60/0x60
[ 245.283760] usb_kill_urb+0x21/0x30 [usbcore]
[ 245.284649] usb_start_wait_urb+0xe5/0x170 [usbcore]
[ 245.285541] ? try_to_del_timer_sync+0x53/0x80
[ 245.286434] usb_bulk_msg+0xbd/0x160 [usbcore]
[ 245.287326] rtsx_usb_send_cmd+0x63/0x90 [rtsx_usb]
Reported-by: diego.viola@xxxxxxxxx
Tested-by: diego.viola@xxxxxxxxx
Signed-off-by: Mathias Nyman <mathias.nyman@xxxxxxxxxxxxxxx>
Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
[bwh: Backported to 3.2:
- Move debug logging along with endpoint lookup
- Adjust context]
Signed-off-by: Ben Hutchings <ben@xxxxxxxxxxxxxxx>
---
drivers/usb/host/xhci.c | 43 +++++++++++++++++++++++++------------------
1 file changed, 25 insertions(+), 18 deletions(-)
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -1522,19 +1522,38 @@ int xhci_urb_dequeue(struct usb_hcd *hcd
unsigned int ep_index;
struct xhci_ring *ep_ring;
struct xhci_virt_ep *ep;
+ struct xhci_virt_device *vdev;
xhci = hcd_to_xhci(hcd);
spin_lock_irqsave(&xhci->lock, flags);
/* Make sure the URB hasn't completed or been unlinked already */
ret = usb_hcd_check_unlink_urb(hcd, urb, status);
- if (ret || !urb->hcpriv)
+ if (ret)
goto done;
+
+ /* give back URB now if we can't queue it for cancel */
+ vdev = xhci->devs[urb->dev->slot_id];
+ urb_priv = urb->hcpriv;
+ if (!vdev || !urb_priv)
+ goto err_giveback;
+
+ xhci_dbg(xhci, "Cancel URB %p\n", urb);
+ xhci_dbg(xhci, "Event ring:\n");
+ xhci_debug_ring(xhci, xhci->event_ring);
+ ep_index = xhci_get_endpoint_index(&urb->ep->desc);
+ ep = &vdev->eps[ep_index];
+ ep_ring = xhci_urb_to_transfer_ring(xhci, urb);
+ if (!ep || !ep_ring)
+ goto err_giveback;
+
+ xhci_dbg(xhci, "Endpoint ring:\n");
+ xhci_debug_ring(xhci, ep_ring);
+
temp = xhci_readl(xhci, &xhci->op_regs->status);
if (temp == 0xffffffff || (xhci->xhc_state & XHCI_STATE_HALTED)) {
xhci_dbg(xhci, "HW died, freeing TD.\n");
- urb_priv = urb->hcpriv;
for (i = urb_priv->td_cnt;
- i < urb_priv->length && xhci->devs[urb->dev->slot_id];
+ i < urb_priv->length;
i++) {
td = urb_priv->td[i];
if (!list_empty(&td->td_list))
@@ -1542,30 +1561,9 @@ int xhci_urb_dequeue(struct usb_hcd *hcd
if (!list_empty(&td->cancelled_td_list))
list_del_init(&td->cancelled_td_list);
}
-
- usb_hcd_unlink_urb_from_ep(hcd, urb);
- spin_unlock_irqrestore(&xhci->lock, flags);
- usb_hcd_giveback_urb(hcd, urb, -ESHUTDOWN);
- xhci_urb_free_priv(xhci, urb_priv);
- return ret;
+ goto err_giveback;
}
- xhci_dbg(xhci, "Cancel URB %p\n", urb);
- xhci_dbg(xhci, "Event ring:\n");
- xhci_debug_ring(xhci, xhci->event_ring);
- ep_index = xhci_get_endpoint_index(&urb->ep->desc);
- ep = &xhci->devs[urb->dev->slot_id]->eps[ep_index];
- ep_ring = xhci_urb_to_transfer_ring(xhci, urb);
- if (!ep_ring) {
- ret = -EINVAL;
- goto done;
- }
-
- xhci_dbg(xhci, "Endpoint ring:\n");
- xhci_debug_ring(xhci, ep_ring);
-
- urb_priv = urb->hcpriv;
-
for (i = urb_priv->td_cnt; i < urb_priv->length; i++) {
td = urb_priv->td[i];
list_add_tail(&td->cancelled_td_list, &ep->cancelled_td_list);
@@ -1586,6 +1584,14 @@ int xhci_urb_dequeue(struct usb_hcd *hcd
done:
spin_unlock_irqrestore(&xhci->lock, flags);
return ret;
+
+err_giveback:
+ if (urb_priv)
+ xhci_urb_free_priv(xhci, urb_priv);
+ usb_hcd_unlink_urb_from_ep(hcd, urb);
+ spin_unlock_irqrestore(&xhci->lock, flags);
+ usb_hcd_giveback_urb(hcd, urb, -ESHUTDOWN);
+ return ret;
}
/* Drop an endpoint from a new bandwidth configuration for this device.