On Wed, Jan 16, 2013 at 12:22:59PM -0500, Alan Stern wrote: > On Wed, 16 Jan 2013, Martin Mokrejs wrote: > > > A corresponding diff of dmesg output is attached. Note that the first kmemleak in there > > happened just without any prior fiddling with a USB drive. For about two days I haven't > > connected a drive. However, usb-storage might be in a wrong shape since several days > > when it happened for the first time. Did you want me to reboot? ;-) I did not. > > Sarah, I looked through the xhci-hcd driver. There does appear to be a > leak in xhci-ring.c:handle_tx_event(). Thanks for catching that. > The routine looks like this: > > /* Leave the TD around for the reset endpoint function > * to use(but only if it's not a control endpoint, > * since we already queued the Set TR dequeue pointer > * command for stalled control endpoints). > */ > if (usb_endpoint_xfer_control(&urb->ep->desc) || > (trb_comp_code != COMP_STALL && > trb_comp_code != COMP_BABBLE)) > xhci_urb_free_priv(xhci, urb_priv); > > ... > > /* EHCI, UHCI, and OHCI always unconditionally set the > * urb->status of an isochronous endpoint to 0. > */ > if (usb_pipetype(urb->pipe) == PIPE_ISOCHRONOUS) > status = 0; > usb_hcd_giveback_urb(bus_to_hcd(urb->dev->bus), urb, status); > > If the condition on the first "if" statement fails, urb_priv won't be > deallocated. It needs something like > > if (...) > xhci_urb_free_priv(xhci, urb_priv); > + else > + kfree(urb_priv); Ok, so you're proposing freeing the urb_priv, but leaving the TD allocated for the Set TR dequeue functions to use? Yes, that looks like the right solution, feel free to submit a patch. > Martin, can you tell if adding these two lines fixes the problem? > > Also, the comment is wrong. urb->status is not set to 0 > unconditionally for isochonrous endpoints in [eou]hcd-hcd. If any of > the iso packets gets a nonzero status then urb->status is set to one of > those values, not to 0. The code needs to be fixed then. How do you pick which iso packet status to return? E.g. if one frame has a babble and another has a short packet, which error code do you return? Whichever one occurred first? Sarah Sharp -- 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
