Hi, On 01/16/2017 04:59 PM, Felipe Balbi wrote: > Hi, > > Lu Baolu <baolu.lu@xxxxxxxxxxxxxxx> writes: >> Hi, >> >> On 12/29/2016 07:01 PM, Felipe Balbi wrote: >>> These three new tracers will help us tie TRBs into URBs by *also* >>> looking into URB lifetime. >> I didn't see anything related to TRBs in the patch. >> Anything I missed? > yes. Ordering of events. Seriously, you ought to compile my xhci-cleanup > patches and run with tracers enabled. Here's what I'm talking about: > > before $subject, we would only see TRBs being queued to HW and > completed, etc. After $subject, we see URBs being queued by class > drivers, which in turn triggers xHCI driver to prepare TRBs which will > be queued to HW. Once a completion IRQ fires, we see TRBs being > "dequeued" from HW and URBs being given back. > > IOW, after $subject we know that a particular TRB (or a set of them) > were queued because an URB was queued. We know the size of the URB, a > pointer to it, etc. We can actually match urb->transfer_length to > sum_of_trb_sizes(). Yes. Get it now. Thanks for this comment. >>> Signed-off-by: Felipe Balbi <felipe.balbi@xxxxxxxxxxxxxxx> >>> --- >>> drivers/usb/host/xhci-ring.c | 1 + >>> drivers/usb/host/xhci-trace.h | 70 +++++++++++++++++++++++++++++++++++++++++++ >>> drivers/usb/host/xhci.c | 5 ++++ >>> 3 files changed, 76 insertions(+) >>> >>> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c >>> index 0ee7d358b812..1431e0651e78 100644 >>> --- a/drivers/usb/host/xhci-ring.c >>> +++ b/drivers/usb/host/xhci-ring.c >>> @@ -627,6 +627,7 @@ static void xhci_giveback_urb_in_irq(struct xhci_hcd *xhci, >>> usb_hcd_unlink_urb_from_ep(hcd, urb); >>> spin_unlock(&xhci->lock); >>> usb_hcd_giveback_urb(hcd, urb, status); >>> + trace_xhci_urb_giveback(urb); >> There is another trace point in xhci_urb_dequeue(). >> >> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c >> index b49588f..ee46877 100644 >> --- a/drivers/usb/host/xhci.c >> +++ b/drivers/usb/host/xhci.c >> @@ -1535,6 +1535,7 @@ int xhci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status) >> >> usb_hcd_unlink_urb_from_ep(hcd, urb); >> spin_unlock_irqrestore(&xhci->lock, flags); >> + trace_xhci_urb_giveback(urb); > yes, so? I want to know that the URB *was* indeed given back. > Got it. There is no need to trace a giveback which ties no real trbs. I have no further questions about this patch. Thanks. Best regards, Lu Baolu -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
