On Tue, Nov 22, 2022, Udipto Goswami wrote: > Hi Thinh, > > > On 11/22/22 7:00 AM, Thinh Nguyen wrote: > > Hi Udipto, > > > > On Fri, Nov 18, 2022, Udipto Goswami wrote: > > > Hi Thinh > > > > > > On 11/18/22 7:31 AM, Thinh Nguyen wrote: > > > > On Thu, Nov 17, 2022, Udipto Goswami wrote: > > > > > A dequeue for ep0 need to adjust the handling based on the > > > > > data stage and status stage. Currently if ep0 is in data/status > > > > > stage the handling isn't that different, driver will try giveback > > > > > as part of dequeue process which might potentially lead to the > > > > > controller accessing invalid trbs. > > > > > > > > > > Also for ep0 the requests aren't moved into the started_list, > > > > > which might potentially lead to the un-mapping of the request > > > > > buffers without sending endxfer. > > > > > > > > Maybe we need to track started_list for control endpoint? If the request > > > > isn't prepared yet or that the transfer had completed, then there's no > > > > need to issue End Tranfer command. > > > > > > > > But I believe sending End Transfer for inactive endpoint should be fine > > > > also. Then we maybe able to get away without checking the started list. > > > > If you're planning to do that, please test and note it somewhere. > > > > > > > > > > > > thanks for the suggestion, sure i'll do some more experiments and confirm > > > it. > > > > > > > Just curious, how do you hit/test this scenario? > > > > For other endpoint types, I can see possible scenarios where a dequeue > > may be needed, but I don't see one for control transfer. > > > > The host can cancel the control transfer, and the controller will see > > "setup_packet_pending" and handle accordingly. If there's a disconnect, > > that's also handled separately by the controller driver also. So, where > > does ep0_dequeue come into play here? > > > adding the reference to other thread [1] > > [1]: https://urldefense.com/v3/__https://www.spinics.net/lists/linux-usb/msg233862.html__;!!A4F2R9G_pg!Z6hsArtLfeqmaf08IqxTov5VyXdvLJuncb8wIXYHC5PW7Zk7WO6u_r8Ap1gR-TlrmzwmQEQ-cJElQ2ED_0deM8t49emcjw$ > > was trying to address a race condition in the ffs driver where ep_dequeue > was suggested, before freeing the request dequeue it. > > as per the current code, since ep0 req isn't moved to started list > therefore it will exit from this in ep_dequeue: > > list_for_each_entry(r, &dep->pending_list, list) { > if (r == req) { > dwc3_gadget_giveback(dep, req, -ECONNRESET); > goto out; > } > } > > but if the ep0 is in data/status phase technically it is still active. > We will unmap the buffer and giveback then the ep0 is in data/status stage. > > This could potentially happen right? > > The intent of a separate dequeue was to address that scenario when the > data/status phase isn't completed. > Hope this give some clarity. > I'm still unclear how it can lead to this condition. Can you list the sequence of events that can lead to this scenario. For functionfs_unbind() to occur, shouldn't there be a disconnect call, triggering soft-disconnect? When that happens, there are checks in dwc3 to ensure that any pending control transfer will be STALLed and given back. Any new control request will not be queued, and no active control transfer should happen at this point. Thanks, Thinh
