On Tue, 8 May 2007, Oliver Neukum wrote:
> Am Dienstag, 8. Mai 2007 18:59 schrieb Alan Stern:
> > But first, I think we should be able to get rid of the skel_open_lock
> > mutex plus similar mutexes in other USB drivers. The problem they address
> > is the race between open() and disconnect() -- or more accurately, open()
> > and unregister_dev(). It should be fixed once and for all in usbcore.
> > And without relying on the BKL.
> >
> > How do you like this approach?
>
> 1. An rwsem is likely overkill.
Why do you say that? Think about what we need to accomplish:
There must be no calls, either new or outstanding, to the
driver's open() method after usb_unregister_dev() returns.
This means:
All calls to usb_open() in file.c which started before
usb_unregister_dev() was called, must complete before
usb_unregister_dev() returns.
This could be done using SRCU, but an rwsem is simpler and easier to
understand. It could be done using a mutex, but then you couldn't have
multiple simultaneous open() calls.
Simply changing the spinlock into an rwsem seems to be by far the best
solution, IMO.
> 2. I would prefer to have exclusion between open and reset, too.
Why? I can understand wanting exclusion between read/write and reset.
But there's no obvious reason to make open and reset exclusive.
Oh yes, there's one other thing you need to know. I haven't documented it
yet, and so far it exists only as part of a patch in Greg's input queue:
http://marc.info/?l=linux-usb-devel&m=117580359813254&w=2
so it's understandable that you're not aware of it. :-)
In certain error paths, suspend() can be paired with
post_reset() instead of with resume(). post_reset() will
get a second argument to indicate such events, which I
call reset-resumes.
This will happen with devices that have USB_QUIRK_RESET_RESUME set,
meaning that they have to be reset after being suspended (like that audio
device from a few months ago). It will also happen with the USB-persist
facility, in cases where power to the host controller was lost during
suspend.
There's actually quite a lot of overlap between suspend() and pre_reset()
as well as between resume() and post_reset(). In fact, I would expect in
many drivers the suspend() and pre_reset() method pointers could point to
the same subroutine, which would need only to quiesce the driver.
Likewise, resume() and post_reset() need to activate the driver. However
post_reset() may need to do a little more, because it may have to restore
any device state that got lost during the reset.
Alan Stern
-------------------------------------------------------------------------
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/
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
