On Wed, 28 May 2003, David Brownell wrote:

> Alan Stern wrote:
> >        (1) We don't want connect processing
> > (registering interfaces in the driver model and probing device drivers,
> > maybe other things too) and/or disconnect processing (detaching drivers
> > and deregistering interfaces, etc.) to occur simultaneously in different
> > threads. 
> 
> Why not?  It'd certainly make for a more robust system, when
> that task hits problems with some device or driver:  only the
> problematic device/driver should lock up.  The serialization in
> sysfs suffices ... it allows simultaneous "processing", with
> only some critical sections needing serialization.

I should have been more precise; (1) referred to connect/disconnect for a
single device, not multiple threads handling different devices.  I don't
think we're in any disagreement here.  Imagine the chaos if one thread was
probing drivers for a device at the same time another thread was handling
the device's disconnection while simultaneously a third thread was trying
to change the active configuration.

> The relevant usb code is monolithic, and splitting out the parts
> will necessarily expose the things that can/can't be concurrent.
> 
>   - Enumerating through the end of "address zero" stage,
>     which puts the device into configuration 0.  Clearly,
>     only one "address zero" (STATE_DEFAULT) device per bus,
>     but any number in later states.

Yes.  There's actually two parts to this.  The first part involves talking
to the hub upstream of the device: recognizing that a device is connected,
resetting the port, and determining the connection speed.  The second part
involves talking to the device: setting its address, maybe also
downloading its device and configuration descriptors.  The first part is
mostly in hub.c while the second is mostly in usb.c, so they are already
somewhat separated.  But neither part can be allowed to run on more than
one device at a time on the same bus (or in two different threads talking
to the same device).

>   - Picking a configuration.  If there's only one, the
>     kernel can clearly choose it.  If there's only one
>     that meets the hub's power budget, likewise.
> 
>     Otherwise the kernel should leave this to some task
>     in userspace.  So there's natural concurrency there,
>     between that task and (today's) khubd, as well as
>     between busses.

The details of this are not so important to my proposal because it doesn't 
involve any USB communication, merely examining host-side data 
structures.

>   - Actually setting that configuration.  Update sysfs etc.
>     (Including "set config 0", which is a kind of reset.)
> 
>     This is the time consuming and error prone stuff.  The
>     binding is a side effect of adding interfaces to sysfs,
>     which can only exist if config value != 0.  It needs
>     to be serialized within scope of a given device, since
>     some drivers use multiple interfaces, but no more.

Agreed.

> Device reset is a partial re-enmeration.  I suspect some
> of the issues there would go better if it were more of
> a full re-enumeration (you said something similar).  And
> again, that's normally done in some other thread (say,
> scsi-eh).

It overlaps the first of your tasks above: everything up to the ADDRESS
state.  There's no need to pick a configuration; we simply re-use the one
previously in effect.  The interfaces, sysfs stuff, drivers, and so on
persist throughout the process with no change.  Again, these stages have
to be serialized only within the scope of a single device, no more.  But 
device resets occur so infrequently that we don't lose anything by making 
them globally serial.

> > Now consider configuration changes.  They shouldn't be a big problem
> > either.  They should simply repeat the same processing that
> 
> Right, a code-shuffling patch could be developed very quickly.

This stuff is newer to me than to you; make that "reasonably quickly" :-)

> > usb_disconnect() does now to remove drivers and interfaces, issue the
> > set-configuration request, and then probe the new interfaces.  In fact,
> > there's no reason why the last two steps here shouldn't be carried out by
> > the same code that handles new device connections, and no reason why the 
> > first step shouldn't be carried out by the same code that handles device 
> > disconnects.
> 
> That is, the same _modified_ code.

Of course.

>  Right now there's
> one great big usb_new_device() routine that mixes up
> two very different stages
> 
>   - set address, fetch device and config descriptors
> 
>   - set configuration, update sysfs
> 
> The "update sysfs" should be part of usb_set_configuration(),
> clearly ... including setting to config 0, which should be
> similar to "disconnect device" logic except that the device,
> and ep0, state remains (but no sysfs interface state).

usb_set_configuration() involves two parts.  First shut down the current
config (skip if the old config is 0).  Then install the new one (skip if
the new config is 0).  The second part should be shared with the updated
version of usb_new_device(), the first part with the updated version of
usb_disconnect().

> And "echo $CONFIG_VALUE > bConfigurationValue" sysfs hooks
> will certainly need usb_set_configuration() to do all the
> sysfs stuff that it currently doesn't do.

Absolutely.

> >>And <linux/workqueue.h> can be used to make keventd do things
> >>in parallel, so that misbehavior through one port can't break
> >>the others.  Dedicated thread not necessary.
> > 
> > 
> > That's true.  But on the other hand, if the actions being requested are
> > things that khubd is doing anyway, it hardly seems necessary to go to a
> > different thread.  Also, khubd is _already_ a big single point of failure.  
> 
> That was the notion:  get rid of it.  A lot of the problems
> with the current enumeration logic come from how it's all
> so monolithic.  And khubd is a part of that.

What do you propose?  Put most of the usb_hub_port_connect_change() code 
into a work queue that can run independently of khubd?

> > This is a bit confusing...  Hub status interrupt data only indicates that
> > a port has undergone a status change.  It doesn't say what the cause of
> > the change was; for that khubd has to make a port status request.  So
> > there doesn't seem to be any way to carry out this suggestion.
> 
> Wrong detail... :)  We're still flagging NOTATTACHED pretty late,
> so we get more false error reports than we need.  The instant
> we know a port is gone, the hub driver can mark every child of
> that port (other hubs too!) NOTATTACHED even if it has to take
> its own sweet time to disconnect all the drivers from some
> child of that port.

Well, yes.  That could be done right now, independently of all the other 
changes discussed here.


Your reply did not address the main point of my previous post, which was 
the implications of having to serialize the operations for each individual 
device.  There are lots of steps involved here:

        Reset the port, Set the device address, Download the descriptors,
        Select a configuration, Set the configuration, Set the interfaces,
        Create sysfs entries for the interfaces, Delete sysfs entries, ...

plus plenty of little ones I left out.  What's the best way to arrange
that these things only happen one at a time for each device (one at a time
per bus for some of them)?  How do we do that and still permit a driver to
request a configuration change, a device reset, or a re-enumeration?

Alan Stern



-------------------------------------------------------
This SF.net email is sponsored by: eBay
Get office equipment for less on eBay!
http://adfarm.mediaplex.com/ad/ck/711-11697-6916-5
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

Reply via email to