RE: [PATCH RESEND] usb: gadget/uvc: Remove non-required locking from 'uvc_queue_next_buffer' routine

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux