Hi Paul, On 3/13/2018 6:26 AM, Paul Zimmerman wrote: > Hi Minas, > > While looking at the QH queuing in the DWC2 driver, I think I've found > some places where the QH struct may not be freed. Normally, the sequence is: > > dwc2_hcd_qh_unlink(); > < other stuff > > dwc2_hcd_qh_free(); > > or else: > > dwc2_hcd_qh_unlink(); > < other stuff > > < link the QH to some other list > > > For non-periodic EPs, dwc2_hcd_qh_unlink() does > list_del_init(&qh->qh_list_entry), or for periodic EPs it calls > dwc2_deschedule_periodic() which in turn does the list_del_init(). > This means the QH is removed from whatever list it was on. > > So after the call to dwc2_hcd_qh_unlink(), the QH either needs to be freed > by calling dwc2_hcd_qh_free(), or it needs to be re-linked to another list, > otherwise the QH would be "lost" and could never be freed. > > The places where I think a problem can happen are in dwc2_hcd_qh_deactivate(), > dwc_hcd_urb_dequeue(), and dwc_hcd_complete_xfer_ddma(). In most if not all > of these places, interrupts are disabled, which means that dwc2_hcd_qh_free() > cannot be called, since it can sleep. So maybe the freeing was omitted because > it was hard to do in these places? > > What do you think, am I reading the code correctly and this could be a real > problem, or am I crazy? :) > Thank you for your input. I'm planing to review all host code in 1 or 2 months. Yes, in host code there are lot of stuff should be updated/corrected. Currently we want to complete all fixes/improvements on gadget side. We still have lot of prepared patches in our queue which need to be included in mainline Kernel. BTW, besides fixes/improvements there are few new features like Service Interval support on device side which currently under development and debugging. Thanks, Minas -- 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
