David,
Here is some comments on the 2.6.4 ohci driver relative to my original
proposed patch:
- The state which I called ED_DESCHEDULED you represent with
"ED_UNLINK && not on the ed_rm_list"
- The state which I called ED_UNLINK you represent with
"ED_UNLINK && on the ed_rm_list"
This looks fine to me (seems like a good idea to keep the number of
explicit states down).
Stylistically, it's a bit strange for this state to be called
ED_UNLINK (why would you have to call start_ed_unlink() when you're
already in ED_UNLINK state?) but that's obviously a matter of taste
more than anything else and has no functional effect.
Some more differences:
- The 2.6.4 version of OHCI no longer causes an ed_deschedule()
to transition the ED directly to ED_IDLE. That's very good since
it plugs the biggest hole I was concerned about.
- Your start_ed_unlink() doesn't disable the bulk/control list-scanning
like my patch did. Your version seems fine provided an ED never
transitions to ED_IDLE without the TD-queue first going empty
(in which case ed_deschedule() already takes care of
disabling the scanning of the appropriate list).
- The new wmb() in finish_unlinks():
ed->hwNextED = 0;
+ wmb ();
+ ed->hwINFO &= ~(ED_SKIP | ED_DEQUEUE);
strikes me as superfluous. We know for a fact that the
HC doesn't see the ED at this point anymore, so the
order in which the stores are completed doesn't matter
to the HC (and when it gets reactivated, ed_schedule() already has
the necessary wmb()).
Now the one piece that I'm still unsure of is that in the 2.6.4
version of the OHCI driver, ohci_endpoint_disable() doesn't call
start_ed_unlink() when in state ED_UNLINK but not on the ed_rm_list.
That's OK _provided_ the TD queue will eventually drain to empty. I
think the only way this could be violated is if there were a continual
stream of new submissions while someone is trying to do an
ohci_endpoint_disable(). Not sure if that can happen.
In summary, the patches in 2.6.4 look very good to me. I'm quite
certain it'll work at least as well (and probably better) than my
proposed patch. I guess I would have appreciated if you had ack'd my
contribution in tracking down some of the races, but I'm sure that was
just an oversight.
Thanks for your patience & keep up the good work!
--david
-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of
GenToo technologies. Learn everything from fundamentals to system
administration.http://ads.osdn.com/?ad_id=1470&alloc_id=3638&op=click
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
