On Tue, Jun 05, 2012 at 08:01:23PM +0200, Peter Stuge wrote: > Hi Sarah, > > Sarah Sharp wrote: > > > 2) Change usbdevfs to use an sg like interface for the BULK_CONTINUATION > > > flag, which also means changing the userspace API since userspace now > > > also needs a way to signal that the first urb is part of a multi urb > > > transfer > > > > Or just change libusb to not break up large transfers, and get rid of > > the BULK_CONTINUATION flag all together. libusb should submit > > the whole transfer to usbfs with an iovec. > > Do you know since which kernel version this is possible? The iovec interface is still being designed, see the continuation of this thread between Hans and I. The USB core has had support for native scatter gather under xHCI since 2.6.31, the first kernel version that had xHCI/USB 3.0 support. > > usbfs can then: > > > > a) verify each iovec entry is not too long > > b) verify the total length of the iovec isn't too big > > c) translate each iovec entry into one sglist vector > > d) submit one URB with a populated sglist to the USB core > > Lots of verification. Will the error paths for those verifications be > any different from what is being done today? Put another way: Given a > 4Mbyte transfer with the way libusb works now, and putting that in an > iovec instead, how would the ioctl() fallout differ? > > If there would be an error on submit when using an iovec, does that > error instead happen on reap when submitting lots of individual URBs? Let's look at a couple error cases. 1a. Short packet with BULK_CONTINUATION 1b. Short packet with iovec 2a. Buffer too large with BULK_CONTINUATION 2b. iovec entry too large with iovec 3a. Total usbfs buffer allocations too big with BULK_CONTINUATION 3b. Total usbfs buffer allocations too big with iovec 1a. libusb breaks up a large transfer into several URBs, and submits them all (except for the first one) with the BULK_CONTINUATION flag. Somewhere along the way, there is a short packet on one of the URBs. usbfs will attempt to cancel the rest of the URBs that had BULK_CONTINUATION set. If the host controller driver works as we previously assumed by halting the queue on a short packet, no more buffers should be transferred. libusb will get an -EREMOTEIO status for the URB in the part of the transfer that generated the short packet, and an -ECONNRESET for the rest of the URBs. 1b. libusb submits a large transfer as one iovec to usbfs. One buffer in that iovec generates a short packet. At this point, the xHCI driver will not transfer the rest of the iovec buffers. The one URB will be given back to usbfs with the length of the data transferred up to the short packet, and the URB will have a status of -EREMOTEIO. I think 1a and 1b are identical. Both get the short packet error on reap of the URB (although there's only one URB in 1b's case), and both only transfer data up to the short packet. 2a. libusb breaks up a large transfer into several URBs. At some point, one of the URBs has a buffer that is too big to map into DMA'able memory, and the submission of that particular URB will fail. I'm not sure what libusb will do in this case. It will obviously want to cancel the previous URBs, but some of the data may or may not be transferred already. It all depends on how fast the host controller is at dealing with submitted URBs. I think this behavior is less than ideal, and really not what userspace drivers want. The end result is that the submission of one URB with BULK_CONTINUATION set will fail with a -E2BIG status. The previously submitted URBs may have a successful status on reap, or they may have a -ECONNRESET status. It's a crap shoot which status userspace sees. 2b. libusb submits one iovec. usbfs iterates over that iovec, mapping each entry into a DMA'able buffer. When one entry is too big, it will undo all the mappings for the previous entries. Then it will return an -E2BIG error on submission. The behavior userspace sees in 2a vs 2b differs, but in this case, I think you really want the behavior in 2b. I think the 2a behavior is just a bug. Why would you want to allow a transfer to partially complete when you can't submit the whole thing? This logic also applies to 3a and 3b. At some point during the URB submission process, we will hit the usbfs upper limit for total mapped buffer size, and we will see the same behavior as in 2a or 2b. Again, I think you really want the whole submission to fail, rather than allowing part of the submission to complete when you can't submit the whole transfer. > > the job is half done, although arguably the userspace half is the > > harder half, since it requires getting libusb developers on board. > > I'm the libusb maintainer since a little less than two years. I've > been involved in the project for about eight, and was offered to take > over as maintainer already in 2007, but declined at that time. > > I've worked some 1000-1500 unpaid hours on a long list of action > items since the previous libusb release 1.0.8, including lots of > bugs, the odd new feature, project infrastructure, and last but not > least on getting contributions for Windows support in libusb > included, which finally lead to the 1.0.9 release in late April. Thank you for your time. libusb is a pretty important part of the USB ecosystem, and I'm very grateful that you are maintaining it. > There were a few other active contributors in the libusb community > during this time, one of which is Hans, he got involved when we met > at FOSDEM in 2010 and he told me about his work on USB redirection > for SPICE, and about the things he needed from libusb in terms of new > functionality. (He's also the package maintainer in Fedora.) > > A fork of libusb has been announced, called libusbx, with the quite libusbx is not a very good name, IMO. I actually removed the libusbx mailing list from this email thread because I thought it was simply a spelling error! Maybe it should have been called libusb2.0... > outspoken purpose to replace the libusb project, and shoehorn me away > as maintainer, because I am very damaging to the project. (I want > commits to be reviewed before inclusion in libusb.git, and when > noone else does review I can't always do it timely enough to keep > everyone happy. My bigger mistake was to not manage the quite > problematic Windows development effort better, which made the 1.0.9 > release to take nearly two years to finish.) Hmm, sounds like you really needed a co-maintainer that shared your views on reviewing code before pushing it, so that they could take on some of your review burden. Too bad they decided to fork rather than working with you, but that's water under the bridge. > The fork effort is spearheaded by the developer of the libusb Windows > support, who doesn't get along with me, as well as by Hans along with > at least two colleagues of his at Red Hat as the other driving > development force. > > The code difference is quite minimal between the two repositories, > since libusbx.git was forked from libusb.git very recently. Hmm, any chance of ever merging libusbx back into libusb? Or were there contenious changes that you didn't want to merge into libusb that made it into libusbx? > To get back on topic, I think that libusb giving iovecs to the kernel > is absolutely the right thing, and as a bonus it will remove some > complexity from libusb, which is always a good thing! :) > > I'm on board since a long time already. Great! > Between myself, Hans, and Alan (who has also contributed to libusb) > I don't think moving to use iovec is a big problem. The only thing > that is important for the change is that libusb shouldn't suddenly > start behaving very differently as a consequence, but I'm not really > worried. I hope you can the error case questions can be answered > though, that's where the hairy details are. Sure. Let me know if you can think of any other error cases we need to examine. Sarah Sharp -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
