Re: pre/post_reset methods for skel

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