serial/ftdi_sio byte loss / performance regression
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
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
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
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
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
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
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
> > 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