On Thu, 29 Jul 2004, David Brownell wrote:
> Oh, those! Right now they're not in the trees because of problems
> they turned up (as I understand things).
My understanding was that they didn't get applied because Greg was too
busy preparing for OLS. It doesn't matter now.
> What I had to do:
>
> - Get USB_SUSPEND to work, with locktree(), tested;
> in use on x86 + ARM before those other patches;
>
> - Try merging against those patches; result didn't
> seem ever quite right and never got tested. (This
> is where most of those doc/comment updates
> came from: sorting some issues out.)
I'm not surprised there were problems; those patches and the locking
changes interfere considerably.
> - Go back to normal Linus tree _without_ those locking
> changes; tested, it still works.
>
> So I guess my coordination proposal is that instead of me trying
> to coordinate with your locking patches again, it should go the other
> way around this time. (And anyway, you had seen locktree a few
> weeks before you did your patches!)
Okay. When the locking stuff has been merged I'll redo my earlier
patches.
> > I realize that these delays would be minimal, but I don't see any reason
> > for imposing them at all. And I don't understand what you mean by "that
> > kind of signaling". Making usb_reset_device() use locktree won't protect
> > any downstream devices; instead it will prevent someone from resetting the
> > device while an upstream hub is locked. Likewise for khubd.
>
> Right, the reason that upstream port got locked was because it needed
> to use non-data signaling downstream of that port, which would interfere
> with other such uses -- including a RESET.
Aha! The upstream "port" wasn't locked; the entire hub was! So even if
non-data signalling is taking place on a port/branch that is disjoint from
your device you still have to suffer the locking delay.
> And certainly trying to reset a
> hub should protect against anyone thinking the downstream devices
> will just continue as usual!
In other words, what about the case where the port/branch _is_ the one
containing your device, right? Okay, let's consider some subcases.
Suppose an upstream hub is locked for some reason while another thread
wants to work on a downstream device, either to reset it or to do event
processing (if the downstream device is another hub):
1. The upstream hub has a connect change event. Khubd will
immediately mark all devices below the port as NOTATTACHED.
It doesn't matter what type of locking is used on the
downstream device; pretty much everything will fail.
Certainly reset or event processing will.
2. The upstream hub is being suspended. Your recursive suspend
code will lock the downstream device when it gets there. So
it doesn't matter what sort of locking is used by the other
thread working on the device; both locktree and regular locking
will have to wait until the device is suspended. (Locktree
will have to wait until _everything_ below the upstream hub
is suspended.) Once the downstream device is suspended both
reset and event processing will fail.
3. The upstream hub is being resumed. Under these circumstances
it's unlikely that anyone would want to do anything to the
downstream device since it must still be suspended, but never
mind that. Resume processing will recursively lock devices
below the upstream hub as they are resumed. The downstream
device will get locked in its turn, so again it doesn't matter
whether the other thread uses locktree or regular locking.
4. The upstream hub is being reset. Part of the code I added
(probably in one of the patches that Greg hasn't applied yet)
changed usb_reset_device() so that when a hub is reset all of
its children are disconnected first. Once the downstream
device is gone, the question of whether to use locktree or
regular locking is moot.
5. The upstream hub is processing an overcurrent-change notification
for the port leading to the downstream device. All khubd does
when this happens is to clear the overcurrent-change feature.
There's no need for the other thread to wait for that to
complete, as locktree would make it do.
In short, I don't see any situations in which using locktree() for hub
event processing or device resets will provide any advantage at all. But
there _are_ situations (2 and 5) in which it will provide useless delays.
> > They _do_ get disconnected later on. Or rather, the devices that have
> > already been through usb_new_device() get disconnected. And it's not all
> > that much later; it's as soon as khubd can wake up and process the
> > virtual-disconnect notification.
>
> I'd have to look again ... but assuming you're right, then that'd be a
> fine thing to put in comments: non-obvious.
It may be more obvious than you think when you look at the code -- or
maybe not; I don't have a copy on hand to check against. Adding some
comments will be no trouble.
> > Really? Exactly how did it misbehave? Numerous SCSI errors showing up in
> > your system log?
>
> See the attachment. READ CAPACITY failing every second or so.
> Presumably a usb-storage suspend that called SCSI suspend would
> behave right ... assuming there's a SCSI suspend to talk to! :)
You got some strange errors there. It looks like the SCSI disk driver
thinks the medium was unloaded, while some process is repeatedly trying to
open the device file...
> I don't object to teaching drivers how to suspend()/resume() ... at
> least so long as someone else is doing the work! But I would object
> to leaving the PM core broken, long-term.
Me too.
> Of course, sometimes teaching how to suspend may be tricky.
> Like Wake-on-LAN from network adapters. We may want to
> change the driver suspend() to provide a "don't enable remote
> wakeup" indicator of some kind. It may be simpler to fix the PM
> core (I actually have an old Pat Mochel patch that does much
> of that) than address all those types of issue at once!
I foresee more problems. For example, whose responsibility should it
ultimately be to suspend a subtree of nodes in the USB tree -- the driver
core's or usbcore's? The driver core doesn't have the knowledge to apply
the proper locking. And what should happen when there are non-USB devices
located below a USB node (a SCSI disk for example), who's responsible for
locking them?
Alan Stern
-------------------------------------------------------
This SF.Net email is sponsored by OSTG. Have you noticed the changes on
Linux.com, ITManagersJournal and NewsForge in the past few weeks? Now,
one more big change to announce. We are now OSTG- Open Source Technology
Group. Come see the changes on the new OSTG site. www.ostg.com
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
