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.

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

Reply via email to