Hi Laurent, On Wed, Jan 30, 2019 at 11:17:37AM +0200, Laurent Pinchart wrote: > Hi Sakari, > > Thank you for the patch. > > On Tue, Jan 29, 2019 at 11:49:44PM +0200, Sakari Ailus wrote: > > The UVC video driver converts the timestamp from hardware specific unit to > > one known by the kernel at the time when the buffer is dequeued. This is > > fine in general, but the streamoff operation consists of the following > > steps (among other things): > > > > 1. uvc_video_clock_cleanup --- the hardware clock sample array is > > released and the pointer to the array is set to NULL, > > > > 2. buffers in active state are returned to the user and > > > > 3. buf_finish callback is called on buffers that are prepared. buf_finish > > includes calling uvc_video_clock_update that accesses the hardware > > clock sample array. > > > > The above is serialised by a queue specific mutex. Address the problem by > > skipping the clock conversion if the hardware clock sample array is > > already released. > > > > Reported-by: Chiranjeevi Rapolu <chiranjeevi.rapolu@xxxxxxxxx> > > Tested-by: Chiranjeevi Rapolu <chiranjeevi.rapolu@xxxxxxxxx> > > Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx> > > The analysis looks good to me. > > Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> > > > --- > > Hi Laurent, > > > > This seems like something that's been out there for a while... I'll figure > > out soon which stable kernels should receive it, if any. > > Should I wait for the proper Fixes: and Cc:stable tags before queuing > this patch then ? Please do. I'll figure these out in a moment... > > > drivers/media/usb/uvc/uvc_video.c | 7 +++++++ > > 1 file changed, 7 insertions(+) > > > > diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c > > index 84525ff047450..a30c5e1893e72 100644 > > --- a/drivers/media/usb/uvc/uvc_video.c > > +++ b/drivers/media/usb/uvc/uvc_video.c > > @@ -676,6 +676,13 @@ void uvc_video_clock_update(struct uvc_streaming *stream, > > if (!uvc_hw_timestamps_param) > > return; > > > > + /* > > + * We may get called if there are buffers done but not dequeued by the > > + * user. Just bail out in that case. > > + */ > > + if (!clock->samples) > > + return; > > + > > spin_lock_irqsave(&clock->lock, flags); > > > > if (clock->count < clock->size) -- Regards, Sakari Ailus sakari.ailus@xxxxxxxxxxxxxxx
