From a61f12b532e9dcf20e7a4a4a4daf3d8efdc9ff8f Mon Sep 17 00:00:00 2001 From: Mathias Nyman Date: Tue, 28 Mar 2017 15:55:30 +0300 Subject: [PATCH] xhci: Manually give back cancelled URB if we can't queue it for cancel 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@gmail.com Tested-by: diego.viola@gmail.com Signed-off-by: Mathias Nyman Signed-off-by: Greg Kroah-Hartman [bwh: Backported to 3.2: - Move debug logging along with endpoint lookup - Adjust context] Signed-off-by: Ben Hutchings --- drivers/usb/host/xhci.c | 56 +++++++++++++++++++++++------------------ 1 file changed, 31 insertions(+), 25 deletions(-) diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index a834373411f2..547a03949dee 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -1522,19 +1522,38 @@ int xhci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status) 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, struct urb *urb, int status) 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, struct urb *urb, int status) 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. -- 2.39.2