Hi Martin, Thank you very much for the information and commentary!
On May 30, 2016 at 9:28:52 AM, Martin Pieuchot wrote: > Hello Phil, > > On 26/05/16(Thu) 14:55, Phil Vachon wrote: > > Hi all, > > > > -- snip -- > > You should get in touch with Grant Czajkowski, he did something similar > during the GSoC 2015, see: > > https://www.google-melange.com/gsoc/project/details/google/gsoc2015/coolguy1253/5649050225344512 > Damn, now I wish I had looked harder before I embarked on all this. I will reach out to Grant and see how we could maybe work together to get this over the finish line to go into an upcoming release. It's very helpful to have! I'm guessing Grant's contact info can be found in the mailing list archives, so I'll have a look. > > > > Currently broken things: > > - The entire synchronous interface (poll-related changes) > > That's fine as long as libusb is update accordingly. But if it doesn't > make sense to keep two set of ioctl(2) then, simply get rid of the old > ones ;) > Loud and clear. I'll see if we can keep the current interface as intact as possible (based on your comment below), but if there are 'appendices' that remain, they will be removed. > > - Cancelling pending transfers > > That's the hard part of the problem. Grant's work includes a working > solution, but modifying the HC driver for that should be avoided. > This is one thing I'm not 100% understanding -- my plan (though Grant has already gone through this) was to expose the pipe's abort() method through a similar usbi_* function. I presumed this should be callable safely at splusb(), since this is how a pipe goes about cleaning itself up? There was a change to uhci(4) that seems to insert a USB task for every transfer that is to be cancelled, is this the HC driver change you're referring to? I am guessing that my presumption that abort() is safely callable at splusb() is wrong? The other thing that I was trying to avoid in cancellation was having complexities around identifying pending operations. Originally I hacked something together using the address of the source/target userspace buffer for identifying the operation. But the same buffer could be reused for multiple transactions so this does not make sense. The other idea I had was to provide a transaction 'handle' returned by the USB_ASYNC_SUBMIT ioctl(2) that would then be used to later reference the operation. Both approaches have lots of downsides, but since libusb is already doing bookkeeping on the programmer's behalf, the latter approach might not be too onerous. > > - Isochronous transfers > > That's not important in the first place. > I didn't think so, but I was thinking of providing a kernel-managed queue for isochronous transfers. Then userspace would populate the queue with transfers to be submitted after the next isochronous transfer completes. Something similar could be done for interrupt endpoints too, which would allow for the timing 'requirements' to be met more readily for such things. > > - Kqueue (maybe? I haven't tested it thoroughly yet) > > Not sure if we should keep support it. If libusb's handle_event doesn't > need kqueue(2) support after your modifications then it can go. > It _could_ be used I suppose, since the libusb user could register to receive notification of all new fds, and use kqueue(2) rather than poll(2) to receive notification of completed operations. I was planning to support it, based on that. > > - Locking is somewhat heavy-handed in places > > Please removing your locking code. All ioctl(2) are serialized by the > KERNEL_LOCK(). The only problem is if your code can sleep, in which > case the mutex you're using does not help. > Perfect, will do. I guess I was worried about concurrent modification of the queue of pending ops, but if all ioctl(2) are serialized as such, then there's no sense in keeping any locking around. This was one thing that I'm admittedly not comfortable with in OpenBSD, and need to learn more about. > > - There's a hard cap of 16 asynchronous operations in-flight per > > endpoint at this time. This could be readily made configurable. > > Nice. > > > My current test cases are a trivial "smoke test" of the API, as well as > > using libusb and the libimobiledevice/usbmux and ipheth-pair tools. Of > > course, this requires a modified libusb, which can be found at: > > https://github.com/pvachon/libusb - see the openbsd-6.0 branch > > Nice, I'd really suggest you to look at Grant's work, hopefully with him > because his work also solve some other issues, like timeout handling > without deprecating the existing interface. > > > and some light modifications to usbmuxd: > > https://github.com/pvachon/usbmuxd > > Nice, I'd suggest you to submit a port for usbmuxd :) > > > Otherwise, upstream libimobiledevice, libusbmux, etc. can all be used > > as-is. > > I'll make ports for all of these, seems there are none that exist as of today. Makes sense, since they didn't work before.
