On Wed, 2 May 2018, Felipe Balbi wrote:
> >> Look at the result. With this change we don't need to take a lock,
> >> allocate memory, search for endpoint index, search for endpoint
> >> state. All of those are needed for proper operation of the function, but
> >> if the controller has already died, there's no point in going any
> >> further.
> >
> > But we might miss the fact that host died, and go even further, adding URB
> > to list,
> > writing TRBs to ringbuffers etc.
> >
> > In code we save one line,
> > goto: free_priv
>
> We're saving a lot more than that, actually. All of the following ends
> up being skipped. All of these are unnecessary work when xHC has already
> died:
>
> 8<------------------------------------------------------------------------
>
> slot_id = urb->dev->slot_id;
> ep_index = xhci_get_endpoint_index(&urb->ep->desc);
> ep_state = &xhci->devs[slot_id]->eps[ep_index].ep_state;
>
> if (!HCD_HW_ACCESSIBLE(hcd)) {
> if (!in_interrupt())
> xhci_dbg(xhci, "urb submitted during PCI suspend\n");
> return -ESHUTDOWN;
> }
>
> if (usb_endpoint_xfer_isoc(&urb->ep->desc))
> num_tds = urb->number_of_packets;
> else if (usb_endpoint_is_bulk_out(&urb->ep->desc) &&
> urb->transfer_buffer_length > 0 &&
> urb->transfer_flags & URB_ZERO_PACKET &&
> !(urb->transfer_buffer_length % usb_endpoint_maxp(&urb->ep->desc)))
> num_tds = 2;
> else
> num_tds = 1;
>
> urb_priv = kzalloc(sizeof(struct urb_priv) +
> num_tds * sizeof(struct xhci_td), mem_flags);
> if (!urb_priv)
> return -ENOMEM;
>
> urb_priv->num_tds = num_tds;
> urb_priv->num_tds_done = 0;
> urb->hcpriv = urb_priv;
>
> trace_xhci_urb_enqueue(urb);
>
> if (usb_endpoint_xfer_control(&urb->ep->desc)) {
> /* Check to see if the max packet size for the default control
> * endpoint changed during FS device enumeration
> */
> if (urb->dev->speed == USB_SPEED_FULL) {
> ret = xhci_check_maxpacket(xhci, slot_id,
> ep_index, urb);
> if (ret < 0) {
> xhci_urb_free_priv(urb_priv);
> urb->hcpriv = NULL;
> return ret;
> }
> }
> }
>
> spin_lock_irqsave(&xhci->lock, flags);
>
> 8<------------------------------------------------------------------------
Regardless, how often does it happen that an xHCI host controller dies?
A few times a day for every xHCI controller in the world seems like a
gross overestimate.
But even if it isn't, who cares if we end up executing the code above a
few extra times per day? It's the "almost never used" path. :-)
Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
