[libftdi-1.0] Limit read buffer chunksize to 16384 on Linux

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[libftdi-1.0] Limit read buffer chunksize to 16384 on Linux

Jie Zhang-2
We can't set readbuffer_chunksize larger than MAX_BULK_BUFFER_LENGTH,
which is defined in libusb-1.0, on Linux. Otherwise, each USB read
request will be divided into multiple (to be exactly, 4) URBs. The
last 3 URBs of these 4 URBs will usually result in abnormal reaps
because we usually request small number of data. The abnormal reaps
will concatenate empty ftdi packets to other packet on Linux kernel
older than 2.6.32. libftdi cannot parse such combined packet. This
will cause issue for UrJTAG when using libftdi-1.0 with libusb-1.0. I
have committed this patch on the libftdi-1.0 git tree.


Jie


--
libftdi - see http://www.intra2net.com/en/developer/libftdi for details.
To unsubscribe send a mail to [hidden email]  

libftdi-limit-read-chunksize-to-16KB.diff (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

RE: [libftdi-1.0] Limit read buffer chunksize to 16384 on Linux

Michael Plante
Jie Zhang wrote:
>> We can't set readbuffer_chunksize larger than MAX_BULK_BUFFER_LENGTH,
>> which is defined in libusb-1.0, on Linux. Otherwise, each USB read
>> request will be divided into multiple (to be exactly, 4) URBs. The
>> last 3 URBs of these 4 URBs will usually result in abnormal reaps
>> because we usually request small number of data. The abnormal reaps
>> will concatenate empty ftdi packets to other packet on Linux kernel
>> older than 2.6.32. libftdi cannot parse such combined packet. This
>> will cause issue for UrJTAG when using libftdi-1.0 with libusb-1.0. I
>> have committed this patch on the libftdi-1.0 git tree.

In other words, the status bytes issue only happened with very large reads,
or is there something else going on that I'm missing?  If the former, I
hadn't seen that context on any of the emails...  Anyway, good catch

Thanks,
Michael


--
libftdi - see http://www.intra2net.com/en/developer/libftdi for details.
To unsubscribe send a mail to [hidden email]  

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [libftdi-1.0] Limit read buffer chunksize to 16384 on Linux

Jie Zhang-2
On Sun, Feb 14, 2010 at 9:33 AM, Michael Plante
<[hidden email]> wrote:

> Jie Zhang wrote:
>>> We can't set readbuffer_chunksize larger than MAX_BULK_BUFFER_LENGTH,
>>> which is defined in libusb-1.0, on Linux. Otherwise, each USB read
>>> request will be divided into multiple (to be exactly, 4) URBs. The
>>> last 3 URBs of these 4 URBs will usually result in abnormal reaps
>>> because we usually request small number of data. The abnormal reaps
>>> will concatenate empty ftdi packets to other packet on Linux kernel
>>> older than 2.6.32. libftdi cannot parse such combined packet. This
>>> will cause issue for UrJTAG when using libftdi-1.0 with libusb-1.0. I
>>> have committed this patch on the libftdi-1.0 git tree.
>
> In other words, the status bytes issue only happened with very large reads,
> or is there something else going on that I'm missing?  If the former, I
> hadn't seen that context on any of the emails...  Anyway, good catch
>
Yes. It should only happen with very large reads. But libftdi always
calls libusb_bulk_transfer with size = ftdi->readbuffer_chunksize,
which is set to (64 * 1024) in UrJTAG. I can decrease it in UrJTAG,
but I think libftdi is a better place to do that in case other libftdi
users might set it to very large size.

Jie

--
libftdi - see http://www.intra2net.com/en/developer/libftdi for details.
To unsubscribe send a mail to [hidden email]

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [libftdi-1.0] Limit read buffer chunksize to 16384 on Linux

Jim Paris
In reply to this post by Jie Zhang-2
Jie Zhang wrote:

> We can't set readbuffer_chunksize larger than MAX_BULK_BUFFER_LENGTH,
> which is defined in libusb-1.0, on Linux. Otherwise, each USB read
> request will be divided into multiple (to be exactly, 4) URBs. The
> last 3 URBs of these 4 URBs will usually result in abnormal reaps
> because we usually request small number of data. The abnormal reaps
> will concatenate empty ftdi packets to other packet on Linux kernel
> older than 2.6.32. libftdi cannot parse such combined packet. This
> will cause issue for UrJTAG when using libftdi-1.0 with libusb-1.0. I
> have committed this patch on the libftdi-1.0 git tree.
>
>
> Jie
>
>
> --
> libftdi - see http://www.intra2net.com/en/developer/libftdi for details.
> To unsubscribe send a mail to [hidden email]  
> Index: src/ftdi.c
> ===================================================================
> --- src/ftdi.c (revision 3932)
> +++ src/ftdi.c (working copy)
> @@ -1621,6 +1621,14 @@ int ftdi_read_data_set_chunksize(struct
>      // Invalidate all remaining data
>      ftdi->readbuffer_offset = 0;
>      ftdi->readbuffer_remaining = 0;
> +#ifdef __linux__
> +    /* We can't set readbuffer_chunksize larger than MAX_BULK_BUFFER_LENGTH,
> +       which is defined in libusb-1.0.  Otherwise, each USB read request will
> +       be divided into multiple URBs.  This will cause issues on Linux kernel
> +       older than 2.6.32.  */
> +    if (chunksize > 16384)
> +        chunksize = 16384;

Shouldn't this be

> +    if (chunksize > MAX_BULK_BUFFER_LENGTH)
> +        chunksize = MAX_BULK_BUFFER_LENGTH;

-jim

--
libftdi - see http://www.intra2net.com/en/developer/libftdi for details.
To unsubscribe send a mail to [hidden email]  

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [libftdi-1.0] Limit read buffer chunksize to 16384 on Linux

Jie Zhang-2
On Sun, Feb 14, 2010 at 5:58 PM, Jim Paris <[hidden email]> wrote:

>> +#ifdef __linux__
>> +    /* We can't set readbuffer_chunksize larger than MAX_BULK_BUFFER_LENGTH,
>> +       which is defined in libusb-1.0.  Otherwise, each USB read request will
>> +       be divided into multiple URBs.  This will cause issues on Linux kernel
>> +       older than 2.6.32.  */
>> +    if (chunksize > 16384)
>> +        chunksize = 16384;
>
> Shouldn't this be
>
>> +    if (chunksize > MAX_BULK_BUFFER_LENGTH)
>> +        chunksize = MAX_BULK_BUFFER_LENGTH;
>
MAX_BULK_BUFFER_LENGTH is a macro defined in an internal header file
in libusb. So we can't use it here.


Jie

--
libftdi - see http://www.intra2net.com/en/developer/libftdi for details.
To unsubscribe send a mail to [hidden email]

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [libftdi-1.0] Limit read buffer chunksize to 16384 on Linux

Thomas Jarosch
Hello Jie,

On Sunday, 14. February 2010 12:17:58 Jie Zhang wrote:

> On Sun, Feb 14, 2010 at 5:58 PM, Jim Paris <[hidden email]> wrote:
> >> +#ifdef __linux__
> >> +    /* We can't set readbuffer_chunksize larger than
> >> MAX_BULK_BUFFER_LENGTH, +       which is defined in libusb-1.0.
> >>  Otherwise, each USB read request will +       be divided into
> >> multiple URBs.  This will cause issues on Linux kernel +       older
> >> than 2.6.32.  */
> >> +    if (chunksize > 16384)
> >> +        chunksize = 16384;
> >
> > Shouldn't this be
> >
> >> +    if (chunksize > MAX_BULK_BUFFER_LENGTH)
> >> +        chunksize = MAX_BULK_BUFFER_LENGTH;
>
> MAX_BULK_BUFFER_LENGTH is a macro defined in an internal header file
> in libusb. So we can't use it here.

Hmm, here's a comment from libusb 1.0:

        /* usbfs places a 16kb limit on bulk URBs. we divide up larger requests
         * into smaller units to meet such restriction, then fire off all the
         * units at once. it would be simpler if we just fired one unit at a time,
         * but there is a big performance gain through doing it this way. */
        int num_urbs = transfer->length / MAX_BULK_BUFFER_LENGTH;

Just guess what happens if the usbfs restriction is resolved
and libusb adjusts the interface... this -will- break.

Is there some data structure we could read out?
Or include the internal header file during the build
stage of libftdi to determine the value?

btw: Thanks for applying the other patches!

Cheers,
Thomas

--
libftdi - see http://www.intra2net.com/en/developer/libftdi for details.
To unsubscribe send a mail to [hidden email]  

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [libftdi-1.0] Limit read buffer chunksize to 16384 on Linux

Jie Zhang
On 02/15/2010 05:36 PM, Thomas Jarosch wrote:

> Hello Jie,
>
> On Sunday, 14. February 2010 12:17:58 Jie Zhang wrote:
>> On Sun, Feb 14, 2010 at 5:58 PM, Jim Paris<[hidden email]>  wrote:
>>>> +#ifdef __linux__
>>>> +    /* We can't set readbuffer_chunksize larger than
>>>> MAX_BULK_BUFFER_LENGTH, +       which is defined in libusb-1.0.
>>>>   Otherwise, each USB read request will +       be divided into
>>>> multiple URBs.  This will cause issues on Linux kernel +       older
>>>> than 2.6.32.  */
>>>> +    if (chunksize>  16384)
>>>> +        chunksize = 16384;
>>>
>>> Shouldn't this be
>>>
>>>> +    if (chunksize>  MAX_BULK_BUFFER_LENGTH)
>>>> +        chunksize = MAX_BULK_BUFFER_LENGTH;
>>
>> MAX_BULK_BUFFER_LENGTH is a macro defined in an internal header file
>> in libusb. So we can't use it here.
>
> Hmm, here's a comment from libusb 1.0:
>
> /* usbfs places a 16kb limit on bulk URBs. we divide up larger requests
> * into smaller units to meet such restriction, then fire off all the
> * units at once. it would be simpler if we just fired one unit at a time,
> * but there is a big performance gain through doing it this way. */
> int num_urbs = transfer->length / MAX_BULK_BUFFER_LENGTH;
>
> Just guess what happens if the usbfs restriction is resolved
> and libusb adjusts the interface... this -will- break.
>
> Is there some data structure we could read out?

AFAIK, no.

> Or include the internal header file during the build
> stage of libftdi to determine the value?
>
But the internal header file is not installed. We can't require source
code of libusb is available when building libftdi. But I think we can
ask on libusb mailing list if it's OK to move the macro definition out
to libusb.h. I will do that.


Jie

--
libftdi - see http://www.intra2net.com/en/developer/libftdi for details.
To unsubscribe send a mail to [hidden email]  

Loading...