On Wed, 19 Mar 2003, Matthew Dharm wrote:

> I'm not certain this is a problem...
> 
> I know blocking is legal in the SCSI error-handling context.  Other I/O
> will continue during this time.
> 
> I have to think about this some more....
> 
> Matt
> 
> On Thu, Mar 20, 2003 at 12:48:02AM +0100, Oliver Neukum wrote:
> > Am Mittwoch, 19. März 2003 23:46 schrieb Greg KH:
> > > On Fri, Mar 14, 2003 at 07:56:27AM -0800, David Brownell wrote:
> > > > Oliver Neukum wrote:
> > > > >am I right in assuming that any disconnects/probes _must_ be done
> > > > > through driver core? Or is calling usb_device_remove() directly still
> > > > > legal?
> > > >
> > > > I'm not even sure why usb_device_{probe,remove}() are even exported.
> > >
> > > Someone needs to fix up the usb-storage driver for these functions to be
> > > made private.
> > 
> > I just thought about this. It's much less trivial than I thought.
> > The patch I made for cleaning up reset is crap.
> > We cannot ignore it either. Today I saw a mouse with memory stick
> > reader. If this is HID+storage, multiinterface devices are no longer
> > theory.
> > 
> > The problem is somewhat involved and I might even be wrong.
> > Upon device reset the interfaces whose drivers do not make a reset
> > must be notified that they cannot use the device while we reset it.
> > Currently this is done via disconnect()ing all interfaces.
> > This is potentially lethal. This is done as part of the block io error
> > handling path. This means that all memory allocations must be made
> > GFP_NOIO or GFP_ATOMIC. Now please correct me if I am wrong,
> > but the hotplugging subsystem does exactly that, doesn't it?
> > 
> >     Regards
> >             Oliver


Oliver is right, this is a problem.

usb_device_{probe,remove} could be made private to the core if the core 
would handle device resets correctly.  Read this comment from 
usb_device_reset() in core/hub.c:

/*
 * WARNING - If a driver calls usb_reset_device, you should simulate a
 * disconnect() and probe() for other interfaces you doesn't claim. This
 * is left up to the driver writer right now. This insures other drivers
 * have a chance to re-setup their interface.
 *
 * Take a look at proc_resetdevice in devio.c for some sample code to
 * do this.
 */

That's bad design.

What's needed is some way to notify a driver that someone else is about to 
reset the device.  This has to be done for every driver bound to an 
interface on that device, excepting (perhaps) the one requesting the reset 
since it is already aware of what's going on.  Ideally the drivers should 
be given a chance to put things into a quiescent state, and only after 
everyone is ready should the reset take place.  Then the drivers should be 
notified once the reset is complete.

Of course, there are potential error paths that must be handled: the 
device might get unplugged during this procedure, or its descriptors might 
change somehow (which would require disconnecting all the interfaces and 
starting from scratch).

Yes it's awkward, but it seems unavoidable given that it is only possible 
to reset an entire USB device.

Requiring every driver which might be bound to an interface that is part
of a storage device to avoid GFP_KERNEL feels too draconian to me.  Maybe
it would be better simply to remove the usb_reset_device() call from
usb-storage, or to implement it differently.  For example, usb-storage's
function for handling an emulated SCSI bus reset (which is where
usb_reset_device() gets called) might mark the device as bad, fail the
reset call, fail all following SCSI calls, disconnect itself (telling SCSI
that the device is gone), and then reset the device and be re-probed.  
I'm not really recommending this, but at least it would mean any other
drivers bound to that same device wouldn't be forced to avoid ever waiting
on I/O.

Alan Stern



-------------------------------------------------------
This SF.net email is sponsored by: Tablet PC.  
Does your code think in ink? You could win a Tablet PC. 
Get a free Tablet PC hat just for playing. What are you waiting for? 
http://ads.sourceforge.net/cgi-bin/redirect.pl?micr5043en
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

Reply via email to