serial/ftdi_sio byte loss / performance regression

2013-06-06 Thread Tomaž Šolc
Hi

I have noticed that the ftdi_sio serial driver in recent kernel versions
has very bad performance when used through the Python's serial library.

As a test case I have a custom device that will send a continuous block
of 5k characters once every few seconds over a RS-232 line (115200 baud)
to an Olimex programmer (based on FT2232C, also tried one with FT2232H).

Programmer is connected to a Linux system where a simple Python script
reads the device:

import serial
comm = serial.Serial("/dev/ttyUSB0", 115200)
while True:
line = comm.readline()

With kernels before 3.7.0 the script reads uncorrupted data while using
newer kernels (including 3.9.4) the Python script sees heavy byte loss.
"top" shows an 95% "idle" CPU. Only very slow transmissions (on the
order of tens of bytes per second) will come through uncorrupted.

Using git-bisect, I have found the commit that introduced this problem:

6f602912c9d0c84c2edbd446dd9f72660b701605
usb: serial: ftdi_sio: Add missing chars_in_buffer function

This might also be related with the unusual way Python serial library
reads the device. It uses select() with no timeout and single byte
read()s in a loop. strace output:

select(4, [3], [], [], NULL)= 1 (in [3])
read(3, "D", 1) = 1
select(4, [3], [], [], NULL)= 1 (in [3])
read(3, "E", 1) = 1
...

With sufficiently large read()s the byte loss can be eliminated.

With the commit above, each select() now causes an additional round trip
over USB to read the state of the hardware buffer. It's possible that
constant status querying triggers some bug in the hardware or the query
is simply too slow and causes overflows in the hardware buffer.

Thanks for your help
Tomaž
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: serial/ftdi_sio byte loss / performance regression

2013-06-06 Thread Johan Hovold
On Thu, Jun 06, 2013 at 10:50:36AM +0200, Tomaž Šolc wrote:
> Hi
> 
> I have noticed that the ftdi_sio serial driver in recent kernel versions
> has very bad performance when used through the Python's serial library.
> 
> As a test case I have a custom device that will send a continuous block
> of 5k characters once every few seconds over a RS-232 line (115200 baud)
> to an Olimex programmer (based on FT2232C, also tried one with FT2232H).
> 
> Programmer is connected to a Linux system where a simple Python script
> reads the device:
> 
> import serial
> comm = serial.Serial("/dev/ttyUSB0", 115200)
> while True:
>   line = comm.readline()
> 
> With kernels before 3.7.0 the script reads uncorrupted data while using
> newer kernels (including 3.9.4) the Python script sees heavy byte loss.
> "top" shows an 95% "idle" CPU. Only very slow transmissions (on the
> order of tens of bytes per second) will come through uncorrupted.
> 
> Using git-bisect, I have found the commit that introduced this problem:
> 
> 6f602912c9d0c84c2edbd446dd9f72660b701605
> usb: serial: ftdi_sio: Add missing chars_in_buffer function
> 
> This might also be related with the unusual way Python serial library
> reads the device. It uses select() with no timeout and single byte
> read()s in a loop. strace output:
> 
> select(4, [3], [], [], NULL)= 1 (in [3])
> read(3, "D", 1) = 1
> select(4, [3], [], [], NULL)= 1 (in [3])
> read(3, "E", 1) = 1
> ...
> 
> With sufficiently large read()s the byte loss can be eliminated.
> 
> With the commit above, each select() now causes an additional round trip
> over USB to read the state of the hardware buffer. It's possible that
> constant status querying triggers some bug in the hardware or the query
> is simply too slow and causes overflows in the hardware buffer.

You're absolutely right. This is a known issue (the select overhead)
that was just recently fixed by commit a37025b5c7 ("USB: ftdi_sio: fix
chars_in_buffer overhead") in v3.10-rc3. Care to give v3.10-rc4 a try?

Greg, perhaps we should consider backporting the wait-until-sent
patches (i.e. 0693196fe..4746b6c6e)?

Thanks,
Johan
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: serial/ftdi_sio byte loss / performance regression

2013-06-06 Thread Greg KH
On Thu, Jun 06, 2013 at 11:58:56AM +0200, Johan Hovold wrote:
> On Thu, Jun 06, 2013 at 10:50:36AM +0200, Tomaž Šolc wrote:
> > Hi
> > 
> > I have noticed that the ftdi_sio serial driver in recent kernel versions
> > has very bad performance when used through the Python's serial library.
> > 
> > As a test case I have a custom device that will send a continuous block
> > of 5k characters once every few seconds over a RS-232 line (115200 baud)
> > to an Olimex programmer (based on FT2232C, also tried one with FT2232H).
> > 
> > Programmer is connected to a Linux system where a simple Python script
> > reads the device:
> > 
> > import serial
> > comm = serial.Serial("/dev/ttyUSB0", 115200)
> > while True:
> > line = comm.readline()
> > 
> > With kernels before 3.7.0 the script reads uncorrupted data while using
> > newer kernels (including 3.9.4) the Python script sees heavy byte loss.
> > "top" shows an 95% "idle" CPU. Only very slow transmissions (on the
> > order of tens of bytes per second) will come through uncorrupted.
> > 
> > Using git-bisect, I have found the commit that introduced this problem:
> > 
> > 6f602912c9d0c84c2edbd446dd9f72660b701605
> > usb: serial: ftdi_sio: Add missing chars_in_buffer function
> > 
> > This might also be related with the unusual way Python serial library
> > reads the device. It uses select() with no timeout and single byte
> > read()s in a loop. strace output:
> > 
> > select(4, [3], [], [], NULL)= 1 (in [3])
> > read(3, "D", 1) = 1
> > select(4, [3], [], [], NULL)= 1 (in [3])
> > read(3, "E", 1) = 1
> > ...
> > 
> > With sufficiently large read()s the byte loss can be eliminated.
> > 
> > With the commit above, each select() now causes an additional round trip
> > over USB to read the state of the hardware buffer. It's possible that
> > constant status querying triggers some bug in the hardware or the query
> > is simply too slow and causes overflows in the hardware buffer.
> 
> You're absolutely right. This is a known issue (the select overhead)
> that was just recently fixed by commit a37025b5c7 ("USB: ftdi_sio: fix
> chars_in_buffer overhead") in v3.10-rc3. Care to give v3.10-rc4 a try?
> 
> Greg, perhaps we should consider backporting the wait-until-sent
> patches (i.e. 0693196fe..4746b6c6e)?

Yes, that's a good idea, I'll do that for the next round of stable
updates, after this next release tomorrow.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: serial/ftdi_sio byte loss / performance regression

2013-06-06 Thread Tomaž Šolc
On 06/06/2013 11:58 AM, Johan Hovold wrote:
> On Thu, Jun 06, 2013 at 10:50:36AM +0200, Tomaž Šolc wrote:
>> I have noticed that the ftdi_sio serial driver in recent kernel versions
>> has very bad performance when used through the Python's serial library.
>>
>> As a test case I have a custom device that will send a continuous block
>> of 5k characters once every few seconds over a RS-232 line (115200 baud)
>> to an Olimex programmer (based on FT2232C, also tried one with FT2232H).
>>
>> Programmer is connected to a Linux system where a simple Python script
>> reads the device:
>>
>> import serial
>> comm = serial.Serial("/dev/ttyUSB0", 115200)
>> while True:
>>  line = comm.readline()
>>
>> With kernels before 3.7.0 the script reads uncorrupted data while using
>> newer kernels (including 3.9.4) the Python script sees heavy byte loss.
>> "top" shows an 95% "idle" CPU. Only very slow transmissions (on the
>> order of tens of bytes per second) will come through uncorrupted.
>>
>> Using git-bisect, I have found the commit that introduced this problem:
>>
>> 6f602912c9d0c84c2edbd446dd9f72660b701605
>> usb: serial: ftdi_sio: Add missing chars_in_buffer function
>>
>> This might also be related with the unusual way Python serial library
>> reads the device. It uses select() with no timeout and single byte
>> read()s in a loop. strace output:
>>
>> select(4, [3], [], [], NULL)= 1 (in [3])
>> read(3, "D", 1) = 1
>> select(4, [3], [], [], NULL)= 1 (in [3])
>> read(3, "E", 1) = 1
>> ...
>>
>> With sufficiently large read()s the byte loss can be eliminated.
>>
>> With the commit above, each select() now causes an additional round trip
>> over USB to read the state of the hardware buffer. It's possible that
>> constant status querying triggers some bug in the hardware or the query
>> is simply too slow and causes overflows in the hardware buffer.
> 
> You're absolutely right. This is a known issue (the select overhead)
> that was just recently fixed by commit a37025b5c7 ("USB: ftdi_sio: fix
> chars_in_buffer overhead") in v3.10-rc3. Care to give v3.10-rc4 a try?

Johan, thanks for the prompt reply. I have tried v3.10-rc4 and indeed
the issue is fixed.

Best regards
Tomaž
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: serial/ftdi_sio byte loss / performance regression

2013-06-10 Thread Greg KH
On Thu, Jun 06, 2013 at 11:21:54AM -0700, Greg KH wrote:
> On Thu, Jun 06, 2013 at 11:58:56AM +0200, Johan Hovold wrote:
> > On Thu, Jun 06, 2013 at 10:50:36AM +0200, Tomaž Šolc wrote:
> > > Hi
> > > 
> > > I have noticed that the ftdi_sio serial driver in recent kernel versions
> > > has very bad performance when used through the Python's serial library.
> > > 
> > > As a test case I have a custom device that will send a continuous block
> > > of 5k characters once every few seconds over a RS-232 line (115200 baud)
> > > to an Olimex programmer (based on FT2232C, also tried one with FT2232H).
> > > 
> > > Programmer is connected to a Linux system where a simple Python script
> > > reads the device:
> > > 
> > > import serial
> > > comm = serial.Serial("/dev/ttyUSB0", 115200)
> > > while True:
> > >   line = comm.readline()
> > > 
> > > With kernels before 3.7.0 the script reads uncorrupted data while using
> > > newer kernels (including 3.9.4) the Python script sees heavy byte loss.
> > > "top" shows an 95% "idle" CPU. Only very slow transmissions (on the
> > > order of tens of bytes per second) will come through uncorrupted.
> > > 
> > > Using git-bisect, I have found the commit that introduced this problem:
> > > 
> > > 6f602912c9d0c84c2edbd446dd9f72660b701605
> > > usb: serial: ftdi_sio: Add missing chars_in_buffer function
> > > 
> > > This might also be related with the unusual way Python serial library
> > > reads the device. It uses select() with no timeout and single byte
> > > read()s in a loop. strace output:
> > > 
> > > select(4, [3], [], [], NULL)= 1 (in [3])
> > > read(3, "D", 1) = 1
> > > select(4, [3], [], [], NULL)= 1 (in [3])
> > > read(3, "E", 1) = 1
> > > ...
> > > 
> > > With sufficiently large read()s the byte loss can be eliminated.
> > > 
> > > With the commit above, each select() now causes an additional round trip
> > > over USB to read the state of the hardware buffer. It's possible that
> > > constant status querying triggers some bug in the hardware or the query
> > > is simply too slow and causes overflows in the hardware buffer.
> > 
> > You're absolutely right. This is a known issue (the select overhead)
> > that was just recently fixed by commit a37025b5c7 ("USB: ftdi_sio: fix
> > chars_in_buffer overhead") in v3.10-rc3. Care to give v3.10-rc4 a try?
> > 
> > Greg, perhaps we should consider backporting the wait-until-sent
> > patches (i.e. 0693196fe..4746b6c6e)?
> 
> Yes, that's a good idea, I'll do that for the next round of stable
> updates, after this next release tomorrow.

I applied these, plus a few others in order to get them all to apply and
build properly.

But the last patch, 4746b6c6e, didn't apply, and I don't think we really
need it for the 3.9 kernel, do we?

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: serial/ftdi_sio byte loss / performance regression

2013-06-11 Thread Johan Hovold
On Mon, Jun 10, 2013 at 02:39:06PM -0700, Greg KH wrote:
> On Thu, Jun 06, 2013 at 11:21:54AM -0700, Greg KH wrote:
> > On Thu, Jun 06, 2013 at 11:58:56AM +0200, Johan Hovold wrote:

> > > Greg, perhaps we should consider backporting the wait-until-sent
> > > patches (i.e. 0693196fe..4746b6c6e)?
> > 
> > Yes, that's a good idea, I'll do that for the next round of stable
> > updates, after this next release tomorrow.
> 
> I applied these, plus a few others in order to get them all to apply and
> build properly.
> 
> But the last patch, 4746b6c6e, didn't apply, and I don't think we really
> need it for the 3.9 kernel, do we?

No, you're right. It's merely a clean up and slight optimisation as no
chars_in_buffer needs the disconnect mutex after the other changes. Not
sure why it wouldn't apply, though.

I was a bit sloppy when I gave the commit refs above -- it should have
been

0693196^..b16634a

More specifically:

4746b6c USB: serial: clean up chars_in_buffer

This one is not strictly necessary, but should apply to v3.9.

ff93b19 USB: ti_usb_3410_5052: fix chars_in_buffer overhead

This should not be applied to v3.9 as the problem it fixes
wasn't introduced until v3.10-rc1.

It seems you had to add two other patches to get this one to
apply. They should be dropped as well.

b16634a USB: io_ti: fix chars_in_buffer overhead

Needed in 3.9.

a37025b USB: ftdi_sio: fix chars_in_buffer overhead
c413364 USB: ftdi_sio: clean up get_modem_status
dcf0105 USB: serial: add generic wait_until_sent implementation

These three are also needed in 3.9 (and also 3.8).

0693196 USB: serial: add wait_until_sent operation

Needed in 3.9 (and 3.8). (You noticed my refspec did not include
it and added it.)

Sorry about the confusion.

Johan
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: serial/ftdi_sio byte loss / performance regression

2013-06-11 Thread Greg KH
On Tue, Jun 11, 2013 at 12:43:25PM +0200, Johan Hovold wrote:
> On Mon, Jun 10, 2013 at 02:39:06PM -0700, Greg KH wrote:
> > On Thu, Jun 06, 2013 at 11:21:54AM -0700, Greg KH wrote:
> > > On Thu, Jun 06, 2013 at 11:58:56AM +0200, Johan Hovold wrote:
> 
> > > > Greg, perhaps we should consider backporting the wait-until-sent
> > > > patches (i.e. 0693196fe..4746b6c6e)?
> > > 
> > > Yes, that's a good idea, I'll do that for the next round of stable
> > > updates, after this next release tomorrow.
> > 
> > I applied these, plus a few others in order to get them all to apply and
> > build properly.
> > 
> > But the last patch, 4746b6c6e, didn't apply, and I don't think we really
> > need it for the 3.9 kernel, do we?
> 
> No, you're right. It's merely a clean up and slight optimisation as no
> chars_in_buffer needs the disconnect mutex after the other changes. Not
> sure why it wouldn't apply, though.
> 
> I was a bit sloppy when I gave the commit refs above -- it should have
> been
> 
>   0693196^..b16634a
> 
> More specifically:
> 
> 4746b6c USB: serial: clean up chars_in_buffer
> 
>   This one is not strictly necessary, but should apply to v3.9.

It doesn't apply to 3.9 at all, so it being not really necessary, I'll
not include it :)

> ff93b19 USB: ti_usb_3410_5052: fix chars_in_buffer overhead
> 
>   This should not be applied to v3.9 as the problem it fixes
>   wasn't introduced until v3.10-rc1.
> 
>   It seems you had to add two other patches to get this one to
>   apply. They should be dropped as well.

Ok, I've dropped all 3 of these now.

> b16634a USB: io_ti: fix chars_in_buffer overhead
> 
>   Needed in 3.9.

Good, I have that one.

> a37025b USB: ftdi_sio: fix chars_in_buffer overhead
> c413364 USB: ftdi_sio: clean up get_modem_status
> dcf0105 USB: serial: add generic wait_until_sent implementation
> 
>   These three are also needed in 3.9 (and also 3.8).

I have these three. 3.8-stable is dead as far as I'm concerned, so I
can't do anything about it :)

> 0693196 USB: serial: add wait_until_sent operation
> 
>   Needed in 3.9 (and 3.8). (You noticed my refspec did not include
>   it and added it.)

Yes, other things were breaking without it.

> Sorry about the confusion.

No worries, I've dropped the three patches listed above, so we should be
all good to go.  I'll do a 3.9.6-rc1 release today to get these all out
to people to test.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: serial/ftdi_sio byte loss / performance regression

2013-06-12 Thread Johan Hovold
> > a37025b USB: ftdi_sio: fix chars_in_buffer overhead
> > c413364 USB: ftdi_sio: clean up get_modem_status
> > dcf0105 USB: serial: add generic wait_until_sent implementation
> > 
> > These three are also needed in 3.9 (and also 3.8).
> 
> I have these three. 3.8-stable is dead as far as I'm concerned, so I
> can't do anything about it :)

Yeah, I know. I just put it in parenthesis for reference. Feels like
every kernel version is being picked up by someone these days. :)

Johan
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html