On Mon, 2 Jul 2007, Alan Cox wrote:
> > spin_lock_irqsave(&some_lock, flags);
> > ...
> > spin_unlock(&some_lock);
> > usb_hcd_giveback_urb(hcd, urb);
> > local_irq_restore(flags);
> > or is there a better approach?
> Why not just bite the bullet and change the callback convention. The
> lock verification code should catch the cases that matter and which are
> overlooked on a code scan. You could also change the name of the
> callback to be sure it breaks anything out of tree that isn't fixed.
Plus the code above code will break as soon as someone removes the
preempt_disable() from spin_lock_irqsave() and preempt_enable() from
spin_lock_irqrestore() -- which would in fact be a good optimization to do
(local interrupts are disabled, so there is no need to disable
preemption).
But this has to wait until all the places in kernel which do this kind of
incorrect pairing are fixed. I'd ceratinly vote for not adding any more of
them.
spin_lock_irqsave(&some_lock, flags);
...
preempt_disable();
spin_unlock(&some_lock);
usb_hcd_giveback_urb(hcd, urb);
local_irq_restore(flags);
preempt_enable();
Would make it working in both cases. Not a nice thing, indeed :)
--
Jiri Kosina
-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
