-
Notifications
You must be signed in to change notification settings - Fork 51
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
BSOD when transferring more than 64MB in one bulk transfer #51
Comments
Found during the investigation of: #45 @sonatique mentioned this: @dontech : I just test with the simple loopback test I provided on December 7 and I can no longer see any "data corruption", that is great, thanks a lot! However: I then tried to increase buffer size even more, just for fun (since you claimed we could go up to 4GBytes ;-) ) I have no idea where this comes from. Did you try on your side with 64 MBytes or more? |
@dontech Thanks a lot for opening this. |
I found out that the crash appears to come from the core USBD drivers. It would seem that they do not like if the user tries to submit requests that are much bigger than the maximum transfer size that they advertise. Probably a bug in USBD. We can easily mitigate this by never requesting more than maximum transfer size allowed. USBD never transfers more than this anyway, so there is no performance impact. |
Thanks a lot @dontech ! Is there a way to retrieve this advertised maximum transfer size, where is it advertised? Or is it defined somewhere in a header or something? Thanks. |
This originates inside the driver from USBD_INTERFACE_INFORMATION, which is something USBD returns for each endpoint. |
This should be fixed on this branch: https://github.com/mcuee/libusb-win32/tree/libusb-max-transfer
|
@sonatique There is a new test release here: https://github.com/mcuee/libusb-win32/releases/tag/test_release_max_transfer Please test if this fixes your problem. I have tested all the relevant cases here, and to me this seems fixed. Might want to raise a bug with Microsoft, as I am fairly sure the driver is not supposed to crash when the client requests too much data. But for libusb0, this fixes the problem. As usual, please disable driver signing enforcement or allow test signing. |
Just wonder if this is related to the transfer size limited imposed by Microsoft. Windows 8/8.1 limit, Microsoft inbox USB driver does not support xHCI for Windows 7 so it is up to 3rd party driver Control transfer: Interrupt transfer Bulk transfer Isochronous transfer 1024 * MaximumPacketSize for high speed (xHCI, EHCI) |
Shall we change the following to the limit as mentioned here?
BTW, I do not see that we limit the control transfer t0 4kB. |
Reference: |
@mcuee you are right. According to docs: "The MaximumTransferSize member of the USBD_PIPE_INFORMATION structure is obsolete." We will need to address this, but lets do this in a separate bug. I still think the current patch is correct, but the use of pipe information is of course not 100% correct. However, I think currently this has no impact, as USBD to today just answers 64K for all enpoint types, and this seems to work. @sonatique Can you please verify that the current patch fixes your problem? |
@dontech : of course I will verify the latest patch ASAP. Unfortunately, I can not do it before January 22, sorry for the delay. |
@dontech : I have been able to test your latest build. I have one good news and one bad news: Good news: I tested with my "loopback test" program for transfer size ranging from 16KBytes to 1GBytes: no problem, no corruptions, no BSOD, perfect! Thank you very much for this! Bad news: I then tested with my full stack real-world application. As opposed to my simple "loopback test", this application uses more than one concurrent transfer. By default it uses 16 transfers, each of 16KBytes. To summarize: Version of #45 : Version of this issue #51 : This is 100% reproducible. I am sorry I didn't correctly test for more then one concurrent transfers when you made changes in #45 . My conclusion is that you broke something when you made the changes in #45 . Therefore I would advise you not to officially publish the current state of the code. I will modify my "loopback test" program to use more than one concurrent transfer and push it ASAP, such that you can experience this by yourself. Thanks a lot. |
@dontech Well, I quickly modified my loopback test program for handling multiple transfers at once, but it doesn't show the issue... The difference in between this simple version and my full application is that in loopback test each transfer has a fixed constant buffer pointer and buffer contents are verified sequentially, given transfer callback all happens within the same "handling thread". Given that verifying buffers is a "long" process, I am suspecting that all transfers are done while I am verifying the first one, I then resubmit it, verify the second, etc. but de facto there might be only one active transfer at a time. In my full application what I do is that, in callback function, I put the transfer buffer pointer into a list then quickly allocate a new buffer, set the transfer buffer pointer with it and submit the transfer again. I will try to improve loopback test for simulating this, but it might not be easy. In the meantime: do you have any idea how your changes could create the behavior I am observing? Thanks. |
Hi @sonatique OK, I think i know what is going on. The problem here is that you rack up 10 transfers which are all directly submitted to the underlying driver (USBD). Since the last patch, each transfer is broken into several USBD requests (like usbk). When you submit multiple requests at the same time, there is no guarantee that the reads will be done in order. So the first transfer might do: 1: packet 1, packet 2, packet 6 And second will do: 2: packet 3, packet 4, packet 5 Are you sure you are missing data, and that they are not just out of order? Thanks, /pedro |
@dontech : thanks for the explanation. I guess your explanation is absolutely correct, but then, this is a big problem, at least for met (and for other people using more than one concurrent transfers) ... :-( I tested with libusbK.sys and I have no issues: I can have multiple concurrent transfers, each of 2MBytes or so... Thanks. Do you think that there is any chance that you could fix this while retaining the possibility of having transfer >64K? |
Just verified it. This is exactly what is happening. libusb0-sys:[transfer_complete] sequence 7: 64 bytes transmitted, maximum_packet_size=64, totalLength=128 We cannot fix this without serializing what goes into the driver. We could put a lock on it or something. |
The basic problem is that you expect the data to arrive in the same order as your read requests. |
This must mean that libusbk has a lock on the transfers, which only unlocks when the transfer is complete. |
Yes, indeed. There have been several discussion on libusb-1.0 github regarding whether this is guaranteed to happen in all cases, and the conclusion was: yes. And in fact it is the case with latest official libusb-win32 release. Looks like I'll have to choose in between more than one transfer or transfer size >64K... Damned. |
|
|
Yeah, this s not related to my latest patch, but I fully understand the problem. I am going to carry this to a new bug. |
No description provided.
The text was updated successfully, but these errors were encountered: