On Fri, 30 Apr 2004, David Brownell wrote:

> Alan Stern wrote:
> > 
> > I interpreted "children" to mean just usbdev->children[], not ports.  If
> > you allow it to include the ports as well, you rule out the possibility of
> > manipulating different ports simultaneously.  I admit that doesn't come up
> > very often, but do you really want to prevent it entirely?
> 
> If finer grained locking turns out to be necessary, it could
> be added later.  Are you sure all hubs can support that sort
> of concurrent manipulation?  I'm not.

That's a good point.

> Plus, those ports are the ONLY way to access the child devices
> (in usbdev->children[]) ... for me, it makes no sense to lock
> the one and not the other.

It may be that we're making ->serialize do too much.  For example, my
recent change to devices.c uses it to prevent topology changes while
traversing the tree, but that function could easily be served by something
else, like a bus-specific topology semaphore.  This would specifically be
intended to protect the children[] arrays and nothing else.  (Your
flattened list, too, if that is adopted.)  Locking rule: Always lock
usbdev->serialize _before_ locking usbdev->bus->topology_sem.


> > Furthermore, you completely rule out the possibility of doing device
> > resets.  Since the caller of usb_reset_device() already owns the device's
> > lock, the routine _can't_ acquire the parent hub's lock -- that would mean
> > getting them in the wrong order.
> 
> Those locking issues do need care yes, but similar lock order
> problems have been solved before.  I'd start by failing if
> down_trylock() fails, and then see what issues come up ...

It's a tricky problem, and I haven't figured out all the ramifications 
yet...


> > I meant, what if we try to suspend a hub that has activity occurring on 
> > some of its ports?  A new connection, for example.
> 
> If HUB-A is suspending HUB-B, and a new connection comes on
> HUB-A, no problem ... it's going to wait a long time to
> debounce anyway, and the suspend takes hardly any time.
> 
> If the new connection comes in on HUB-B, then I'd sure hope
> that remote wakeup was turned on.  In which case there'd
> surely be a notification ... that's a wakeup event for hubs,
> nothing should be lost.

That's not what I had in mind.  Suppose we want to suspend hub B.  Then 
first we have to go through all of B's children and suspend them.  While 
that's happening, another child could be connected to B.  It would then be 
necessary to go back and suspend that child.  Not too bad, I suppose.
But consider this scenario:

        khubd                           some other thread
        -----                           -----------------
                                        Suspend all of B's children
                                        Suspend B:
                                          Acquire B->serialize
        Receive port-connect-
          change message from B
        Try to acquire B->serialize
                                          Tell A to suspend B's port
                                          Release B->serialize
        Process connect-change on B ???

You can see the problem.

Alan Stern



-------------------------------------------------------
This SF.Net email is sponsored by: Oracle 10g
Get certified on the hottest thing ever to hit the market... Oracle 10g. 
Take an Oracle 10g class now, and we'll give you the exam FREE. 
http://ads.osdn.com/?ad_id=3149&alloc_id=8166&op=click
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

Reply via email to