Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> writes:
> On Wed, 22 Aug 2012, Michael Grzeschik wrote:
>
>> On Thu, Jul 19, 2012 at 12:20:11AM +0200, Michael Grzeschik wrote:
>> > The lockdep hunter mentions a non consistent usage of spin_lock and
>> > spin_lock_irqsafe in the composite_disconnect and usb_function_activate
>> > function:
>> >
>> > [ 15.700897] =================================
>> > [ 15.705255] [ INFO: inconsistent lock state ]
>> > [ 15.709617] 3.5.0-rc5+ #413 Not tainted
>> > [ 15.713453] ---------------------------------
>> > [ 15.717812] inconsistent {HARDIRQ-ON-W} -> {IN-HARDIRQ-W} usage.
>> > [ 15.723822] uvc-gadget/116 [HC1[1]:SC0[0]:HE0:SE1] takes:
>> > [ 15.729222] (&(&cdev->lock)->rlock){?.+...}, at: [<7f0049e8>] composite_disconnect+0x2c/0x74 [g_webcam]
>> > [ 15.738797] {HARDIRQ-ON-W} state was registered at:
>> > [ 15.743677] [<8006de3c>] mark_lock+0x148/0x688
>> > [ 15.748325] [<8006ecb0>] __lock_acquire+0x934/0x1b74
>> > [ 15.753481] [<8007047c>] lock_acquire+0x98/0x138
>> > [ 15.758288] [<804c776c>] _raw_spin_lock+0x4c/0x84
>> > [ 15.763188] [<7f006ae4>] usb_function_activate+0x28/0x94 [g_webcam]
>> > [ 15.769652] [<7f00820c>] usb_ep_autoconfig_reset+0x78/0x98 [g_webcam]
>> > [ 15.776287] [<7f0082a4>] uvc_v4l2_open+0x78/0x94 [g_webcam]
>
>> > --- a/drivers/usb/gadget/composite.c
>> > +++ b/drivers/usb/gadget/composite.c
>> > @@ -300,9 +300,10 @@ int usb_function_deactivate(struct usb_function *function)
>> > int usb_function_activate(struct usb_function *function)
>> > {
>> > struct usb_composite_dev *cdev = function->config->cdev;
>> > + unsigned long flags;
>> > int status = 0;
>> >
>> > - spin_lock(&cdev->lock);
>> > + spin_lock_irqsave(&cdev->lock, flags);
>> >
>> > if (WARN_ON(cdev->deactivations == 0))
>> > status = -EINVAL;
>> > @@ -312,7 +313,7 @@ int usb_function_activate(struct usb_function *function)
>> > status = usb_gadget_connect(cdev->gadget);
>> > }
>> >
>> > - spin_unlock(&cdev->lock);
>> > + spin_unlock_irqrestore(&cdev->lock, flags);
>> > return status;
>> > }
>
> Wouldn't it be better to fix the uvc-gadget driver to avoid calling
> these functions in interrupt context? Or have I misunderstood the flow
> of control?
It goes the other way around. f_uvc calls usb_function_activate
outside of interrupt context. Ie. it is called from uvc_v4l2_open()
which is an open fop.
In either case though, I feel like it's hardly a critical path, so
there's not much sense to worry about performance, and the only drawback
of irqsave is performance.
--
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o
..o | Computer Science, Michał “mina86” Nazarewicz (o o)
ooo +----<email/xmpp: mpn@xxxxxxxxxx>--------------ooO--(_)--Ooo--Attachment:
pgpxjevYNSVVQ.pgp
Description: PGP signature
