Hi Laurent, > -----Original Message----- > From: Laurent Pinchart [mailto:laurent.pinchart@xxxxxxxxxxxxxxxx] > Sent: Thursday, March 15, 2012 6:17 AM > To: Bhupesh SHARMA > Cc: linux-usb@xxxxxxxxxxxxxxx; linux-media@xxxxxxxxxxxxxxx; spear-devel > Subject: Re: [PATCH RESEND] usb: gadget/uvc: Remove non-required > locking from 'uvc_queue_next_buffer' routine > > Hi Bhupesh, > > Thank you for the patch. Thanks for your review comments.. > On Tuesday 13 March 2012 17:04:01 Bhupesh Sharma wrote: > > This patch removes the non-required spinlock acquire/release calls on > > 'queue_irqlock' from 'uvc_queue_next_buffer' routine. > > > > This routine is called from 'video->encode' function (which > translates to > > either 'uvc_video_encode_bulk' or 'uvc_video_encode_isoc') in > > 'uvc_video.c'. As, the 'video->encode' routines are called with > > 'queue_irqlock' already held, so acquiring a 'queue_irqlock' again in > > 'uvc_queue_next_buffer' routine causes a spin lock recursion. > > > > A sample kernel crash log is given below (as observed on using > 'g_webcam' > > with DWC designware 2.0 UDC): > > > > Kernel crash log: > > ----------------- > > [snip] > > I don't think you need to include the complete crash report in the > commit > message, the above description should be enough. > > > Signed-off-by: Bhupesh Sharma <bhupesh.sharma@xxxxxx> > > Acked-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> > > This should probably go in through the USB tree. Could you please > either send > a pull request or make sure the patch is picked up (after modifying the > commit > message if you agree with my comment) ? Sure. I will prune the commit message and then I will try to raise a pull request so that this patch goes through the USB tree. > > --- > > drivers/usb/gadget/uvc_queue.c | 4 +--- > > 1 files changed, 1 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/usb/gadget/uvc_queue.c > b/drivers/usb/gadget/uvc_queue.c > > index d776adb..104ae9c 100644 > > --- a/drivers/usb/gadget/uvc_queue.c > > +++ b/drivers/usb/gadget/uvc_queue.c > > @@ -543,11 +543,11 @@ done: > > return ret; > > } > > > > +/* called with &queue_irqlock held.. */ > > static struct uvc_buffer * > > uvc_queue_next_buffer(struct uvc_video_queue *queue, struct > uvc_buffer > > *buf) { > > struct uvc_buffer *nextbuf; > > - unsigned long flags; > > > > if ((queue->flags & UVC_QUEUE_DROP_INCOMPLETE) && > > buf->buf.length != buf->buf.bytesused) { > > @@ -556,14 +556,12 @@ uvc_queue_next_buffer(struct uvc_video_queue > *queue, > > struct uvc_buffer *buf) return buf; > > } > > > > - spin_lock_irqsave(&queue->irqlock, flags); > > list_del(&buf->queue); > > if (!list_empty(&queue->irqqueue)) > > nextbuf = list_first_entry(&queue->irqqueue, struct > uvc_buffer, > > queue); > > else > > nextbuf = NULL; > > - spin_unlock_irqrestore(&queue->irqlock, flags); > > > > buf->buf.sequence = queue->sequence++; > > do_gettimeofday(&buf->buf.timestamp); > > -- Regards, Bhupesh -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
