Am Montag, 13. Januar 2003 00:03 schrieb David Brownell:
> Oliver Neukum wrote:
> > Am Sonntag, 12. Januar 2003 18:41 schrieb David Brownell:
> >>If there's a CANCELED state it should just be a sub-state
> >>of SUBMITTED ... one where urb->status isn't -EINPROGRESS,
> >>but is -ECONNRESET (async unlink), -ENOENT (synchronous
> >>unlink) or -ESHUTDOWN (cleanup after host controller halt
> >>or power loss).
> >
> > No, that makes the cleanup pointless.
>
> Well then, what were you expecting to achieve?
Two things.
Firstly, I want to make sure that one member of struct urb has only
one function. Secondly I want to avoid a common race.
> I only noticed one issue get mentioned: detecting a COMPLETING
> state ... though I can't recall any problem having such a state
> (alone) would help to resolve, so maybe this simplification
> (only one "urb owned by HCD" state) just highlights pre-existing
> pointlessness? :)
Let me explain.
The first problem we avoid is with unloading. We want to be sure that
the completion handler is not running after return from usb_unlink_urb().
If we wish to make the guarantee, usb_unlink_urb() must not simply
fail if the completion handler is running. A synchronous unlink must wait.
The second problem we fix is with resubmission from the completion
handler. If we have a state for unlinked during completion, we can fail
resubmission. Thus we kill races with disconnect in rx paths.
> (We do know that if an URB is in that state then urb->hcpriv is
> null, BTW... which will not be true for urbs in SUBMITTED state.)
Yes, but that's a flaw, not a feature. It disallows reuse of that data
structure. We should not depend on it.
> > We should have absolute separation of error reporting and state.
> > Usbcore should never look at urb->status.
>
> Words like "absolute" are always danger signs to me.
>
> Particularly for things that are joined at the hips like
> that ... the only time any fault gets "reported" is during
> that COMPLETING state. (And not all faults are errors,
> of course.) So either we have that separation already,
> or you're saying something I'd likely disagree with.
I want to have a clear split between public and private here.
> Remember that HCDs _already_ have sub-states of SUBMITTED.
> Ones like "the controller is working on it" and "we detected
> this error" are only the most obvious ones; there are lots
> of states (and transitions) required by hardware or driver
> structures that are *by design* hidden from layers above.
As should be.
> (Except by side effect, like urb->status changing when the
> request starts heading back up to the device driver, or
> urb->actual_length growing over time.)
>
> >>That way the initial state is always IDLE, it always goes
> >>from there to SUBMITTED, and then becomes eligible to go
> >>through COMPLETING back to IDLE by (a) hardware-initiated
> >>completion, normal or fault; else (b) software-initiated
> >>completion, by unlinking.
> >
> > We want to make sure that unlinking works in state URB_COMPLETING
> > as well.
>
> What, and guarantee that the urb gets a SECOND completion?
No, see above.
> First for the normal/real completion, then again to report that
> it's somehow been unlinked again???? If it's COMPLETING, then
> we *know* that it's not linked to usbcore or hcd data structures,
> so in what sense could unlinking ever succeed?
>
> Nonsense. The policy is that one submit causes one completion,
> though its status may indicate cancelation (software unlink).
> Only confusion could come of changing that.
No intention of doing that. The policy is sound. Currently there's
a small window in usb_submit_urb() where it's violated. I'll fix that.
> > That has several advantages. It reliably unlinks URBs that are
> > resubmitted in the completion handler. And we can make sure that even the
> > completion handler is not running when usb_unlink_urb() returns. We beat
> > that pesky little unload race.
>
> That's not making sense to me.
>
> Current core+hcd code should handle unlinking urbs no matter when they
> get submitted. If it's not, I've yet to hear any bug reports.
>
> Likewise, "hcd.c" certainly guarantees that for _successful synchronous_
> unlinks, the completion handler returned. Any non-"hcd" drivers need
> to provide that guarantee some other way.
Correct. But we want it always.
> >>- when unlinking urbs, they must be in SUBMITTED
> >
> > No, we cannot do that, even in principle. The only
> > way we could make that an invariant is by calling usb_unlink_urb()
> > with a spinlock held, which is impossible, as it may sleep.
>
> Again, you're not making sense to me.
>
> Exactly why would it be anything other than The Right Thing To Do
> to report appropriate status codes when trying to unlink URBs that
> can't be unlinked ... in this case, because they're not linked?
Misunderstanding.
We can unlink only linked urbs. But usb_unlink_urb() however must handle
an URB in any state. On the other hand we should treat submitting an already
submitted URB as a kernel bug and report it. (Just as an example)
> >>- usb_alloc_urb() returns IDLE urbs, and those are
> >> the only kind the various init_urb calls take.
> >
> > Nope, fails for resubmission in completion.
>
> OK, "IDLE or COMPLETING" then. Unless a less trivial urb
> state machine is used, like:
>
> IDLE -> INITTED -> SUBMITTED -> COMPLETING -> DONE
>
> so that unlinking either accelerates transition to DONE
> or (new feature as-yet-undiscussed here) prevents going to
> SUBMITTED in the first place. That feature would suffice
> to let urbs be used for some basic task synchronization;
> right now they can't, additional data structures are needed.
Interesting. How would that be used?
How's the transition from IDLE to INITTED done?
Why is DONE different from IDLE?
How is resubmission in completion handled?
Regards
Oliver
-------------------------------------------------------
This SF.NET email is sponsored by: FREE SSL Guide from Thawte
are you planning your Web Server Security? Click here to get a FREE
Thawte SSL guide and find the answers to all your SSL security issues.
http://ads.sourceforge.net/cgi-bin/redirect.pl?thaw0026en
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
