Alan Stern responding to David Brownell:

But did any kernel ever guarantee those semantics?  I don't think so.
Certainly 2.6 never did, and neither OHCI nor EHCI on 2.4 does...

I agree, they didn't. However, that lack didn't prevent driver authors from _assuming_ that they did.

Sure, but I can't think of that as anything other than a set of long-standing driver bugs.


  static inline void urb_begone(struct urb *urb)
  ...

Whether this is smaller than my patch is a question of interpretation: Yours is a much bigger change for the device driver! It also doesn't

But those are less fragile than the combination of usbcore and the hcds. Changing the semantics of unlinking could have lots of un-intended consequences at lower levels; IMO they should get even simpler, not more complicated.

On the other hand, if we just agreed not to call those semantics
by the name "unlinking", and to provide them through some simple
function like usb_urb_begone(), then I see simple ways out.

Sure many drivers would need "obviously correct" one-line changes,
to call that routine.  But that'd be unlikely to break any of
the hard-to-fix lower level code.


address the problems of failing resubmission and races between submit and unlink.

That second problem is orthogonal; even drivers that use asynchronous unlinking have to solve it.

I don't think that usbcore should be involved in such policies.
It certainly can't know what the driver intended, so it can't
guess how to patch up a given event sequence for the relevant
version of "correct".


          I think the difficulty is that historically the guarantees made
by the synchronous unlink call were not spelled out clearly enough, so a
lot of people misunderstood them.  I did at first.

Sure, this is part of why the 2.6 kernel has much more kerneldoc!


I think of it as an API maturity issue; the issue wasn't unique
to synchronous unlinking.  The URB lifecycle model had lots of
odd quirks with respect to urbs-in-flight, most of which weren't
implemented consistently.  They're now gone, and the remaining
behaviors tend to be easily testable.



But as far as I can see, there is _no_ statement in the kernel source
about what struct usb_operations->disable() is supposed to do. The
comments in front of one implementation in hcd.c (hcd_endpoint_disable)
merely state that all endpoint state is gone from the hardware. Since UHCI doesn't store any endpoint state in the hardware to begin with, that doesn't say anything. _Nothing_ is mentioned anywhere about waiting until all the software/driver state is gone as well.

If usbfs were waiting for urb completions, that UHCI-specific behavior wouldn't matter -- there'd be no oopsing. If that UHCI-specific behavior were changed, there'd also be none.

... the thing is, I think that we probably added an "expectation"
that HCDs wait for the URBs to complete, by some changes several
months after the disable() method was first defined.  That's
where the UHCI-specific problem came up.

It's hard to see "fire and forget" working well if urbs can complete
after disconnect() is invoked -- the "forget" part wouldn't work.



why not have usb_unlink_urb (sync) do this?


If the urb was resubmitted by the completion handler, urb_begone() as
written would loop endlessly.

At the risk of sounding like a broken record, device drivers that don't have synchronization on resubmit paths are buggy, and they need to add it.

Of course, auditing for resubmit bugs could easily be
part of converting to call some urb_begone().

- Dave





-------------------------------------------------------
This SF.net email is sponsored by: IBM Linux Tutorials.
Become an expert in LINUX or just sharpen your skills.  Sign up for IBM's
Free Linux Tutorials.  Learn everything from the bash shell to sys admin.
Click now! http://ads.osdn.com/?ad_id=1278&alloc_id=3371&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