Hi Laurent, Thanks for the input. On Tue, Apr 19, 2022 at 11:46:37PM +0300, Laurent Pinchart wrote: > > This indeed fixes an issue, so I think we can merge the patch, but I > also believe we need further improvements on top (of course if you would > like to improve the implementation in a v4, I won't complain :-)) It looks like Greg has already accepted the change and it's in linux-next. We can discuss here how to better handle these -EXDEV errors for future improvements, as it seems like it's been an issue in the past as well: https://www.mail-archive.com/linux-usb@xxxxxxxxxxxxxxx/msg105615.html > > As replied in v2 (sorry for the late reply), it seems that this error > can occur under normal conditions. This means we shouldn't cancel the > queue, at least when the error is intermitent (if all URBs fail that's > another story). My impression was that canceling the queue was still necessary as we may be in progress for the current frame. Perhaps we don't need to flush all the frames from the queue, but at a minimum we need to reset the buf_used value. > > > We likely need to differentiate between -EXDEV and other errors in > uvc_video_complete(), as I'd like to be conservative and cancel the > queue for unknown errors. We also need to improve the queue cancellation > implementation so that userspace gets an error when queuing further > buffers. We already feedback to userspace the error, via the state of vb2_buffer_done(). When userspace dequeues the buffer it can check if v4l2_buffer.flags has V4L2_BUF_FLAG_ERROR to see if things failed, then decide what to do like re-queue that frame. However, this appears to not always occur since I believe the pump thread is independent of the uvc_video_complete() callback. As a result, the complete callback of the failed URB may be associated with a buffer that was already released back to the userspace client. In this case, I don't know if there's anything to be done, since a new buffer and subsequent URBs might already be queued up. You suggested an error on a subsequent buffer queue, but I don't know how helpful that'd be at this point, perhaps in the scenario that all URBs are failing? > > -- > Regards, > > Laurent Pinchart Appreciate the feedback, Dan
