On Tue, 19 Feb 2019 guido@xxxxxxxxxxxxxxxxxx wrote: > Hi Alan, > > My last proposal "udc: net2280: Fix overrun of OUT messages" is still > under investigation. > > During the random stress tests I found a new rare problem: > > When a request must be dequeued with net2280_dequeue() e.g. due > to a device clear action and the same request is finished by the > function scan_dma_completions(): > > https://elixir.bootlin.com/linux/v5.0-rc7/source/drivers/usb/gadget/udc/net2280.c#L1269 > > then the following search loop does not find the request and the function > returns the error -EINVAL without restoring the state ep->stopped = stopped! > > https://elixir.bootlin.com/linux/v5.0-rc7/source/drivers/usb/gadget/udc/net2280.c#L1269 > > Thus the endpoint keeps blocked and does not receive any data. > When we insert ep->stopped = stopped here: > > if (&req->req != _req) { > ep->stopped = stopped; // <<<<<---- > spin_unlock_irqrestore(&ep->dev->lock, flags); > dev_err(&ep->dev->pdev->dev, "%s: Request mismatch\n", > __func__); > return -EINVAL; > } > > then the driver continues as expected, although the driver issues the > false error message "Request mismatch". > > What do you think? It's labor-intensive to fix the error message here, > or shall we leave it as it is, and only insert the "ep->stopped = stopped;" Yes, this is a bug. I think the dev_err() should be changed to ep_dbg(). It's not a real error, after all -- it's just a race. Do that and insert the "ep->stopped = stopped;" line. Also, consider changing the -EINVAL return code here to -EIDRM. This is analogous to what usb_unlink_urb() does on the host side. Alan Stern
