public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Stas Sergeev <stsp@list.ru>
To: Greg KH <gregkh@linuxfoundation.org>
Cc: Jarkko Huijts <jarkko.huijts@gmail.com>,
	Alan Cox <alan@linux.intel.com>,
	linux-usb@vger.kernel.org, linux-serial@vger.kernel.org,
	Linux kernel <linux-kernel@vger.kernel.org>,
	Caylan Van Larson <i@caylan.net>,
	"Rafael J. Wysocki" <rjw@sisk.pl>
Subject: Re: Regression: ftdi_sio is slow (since Wed Oct 10 15:05:06 2012)
Date: Sat, 04 May 2013 01:50:42 +0400	[thread overview]
Message-ID: <51843132.9040304@list.ru> (raw)
In-Reply-To: <20130503203419.GA25932@kroah.com>

04.05.2013 00:34, Greg KH пишет:
> On Fri, May 03, 2013 at 10:27:18PM +0400, Stas Sergeev wrote:
>> 03.05.2013 21:16, Greg KH пишет:
>>
>>> Sounds like an application is doing a foolish thing and should stop it.
>> Its not.
>> The app is quering only for _input_ (specifying only read fds
>> to select). But the select() in linux is implemented the way that
>> even when it polls for input, it will still call tty_chars_in_buffer()...
> I think that's the line dicipline doing this, not select itself.
Line discipline only provides the .poll method, like this:
---
static unsigned int n_tty_poll(struct tty_struct *tty, struct file *file,
poll_table *wait)
---
It doesn't look like the poll callback can find out whether it
is polling for input or for output. So it just returns all the flags
it can.
Any suggestions?

>>> There's no guarantee as to how long select or an ioctl will take, and
>>> now that we have fixed another bug, this device is slower.
>>>
>>> If you change hardware types to use a different usb to serial chip, that
>>> select call might take 4 times as long.  Are we somehow supposed to
>>> change the kernel to "fix" that?
>> Previously, the kernel was not calling to a device at all, so
>> select() was independent of the chip, and it was fast. I was
>> not aware you changed that willingly.
> I don't understand, what do you mean by this?  Some drivers just return
> the value of an internally held number, and don't query the device.
>
> The only way the FTDI driver can determine if the hardware buffer on the
> chip way out on the end of the USB cable is empty or not, is to query
> it.  So the driver now does so.
It does so only for one char. And the query takes longer than
to just xmit that char. So why do you think this even works as
expected?

>>>>>> I asked the customer to comment out
>>>>>> tty_chars_in_buffer(tty) < WAKEUP_CHARS
>>>>>> line in n_tty.c, and he said that cured his problems,
>>>>>> so I think my guess was right.
>>>>> What exactly is the "problem" being seen?
>>>> No idea.
>>>> Well, I can make a test-case that does 1000000 select() calls
>>>> in a loop and time it. This is probably the best I can do.
>>> That's really not a valid test case, as it's nothing that we ever
>>> optimize a serial driver for.  Throughput is the proper thing to care
>>> about, right?
>> Sure, but the throughput was not improved by the aforementioned
>> patch, so what was the upside of it?
>>
>>> To actually determine how many characters the device has in its buffer.
>> You are adding only 1 char, but the time to query TEMT is
>> probably longer than to xmit 1 char. So how could it help
>> in some real scenario? When you done quering TEMT, the
>> char is actually already sent, so the effect is quite the reverse.
>>
>> My scenario is:
>> the app calls select() before xmitting every char.
> Every character?  Why?
Because, as I said, this polls for an input, not output...
Ah, wait, it also does TIOCOUTQ ioctl to find out just how
much is buffered. Are you suggesting to stop using TIOCOUTQ
too? Just because it stopped to work fast enough to be of any
use at all?

> Don't call select for every character :)
How about the "we do not break userspace" rule?
And oh yes, it polls just for an input...

  reply	other threads:[~2013-05-03 20:52 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-05-03 15:02 Regression: ftdi_sio is slow (since Wed Oct 10 15:05:06 2012) Stas Sergeev
2013-05-03 16:30 ` Greg KH
2013-05-03 17:38   ` Stas Sergeev
2013-05-03 16:52     ` Greg KH
2013-05-03 18:05       ` Stas Sergeev
2013-05-03 17:16         ` Greg KH
2013-05-03 18:27           ` Stas Sergeev
2013-05-03 20:34             ` Greg KH
2013-05-03 21:50               ` Stas Sergeev [this message]
2013-05-04 11:15                 ` Johan Hovold
2013-05-04 11:39                   ` Peter Hurley
2013-05-05 18:29                     ` Johan Hovold
2013-05-05 18:32                       ` [PATCH 0/7] USB: serial: add wait_until_sent-support Johan Hovold
2013-05-05 18:32                         ` [PATCH 1/7] USB: serial: add wait_until_sent operation Johan Hovold
2013-05-05 18:32                         ` [PATCH 2/7] USB: serial: add generic wait_until_sent implementation Johan Hovold
2013-05-08 14:25                           ` Stas Sergeev
2013-05-08 15:48                             ` Johan Hovold
2013-05-08 15:51                           ` [PATCH v2 2/8] " Johan Hovold
2013-05-05 18:32                         ` [PATCH 3/7] USB: ftdi_sio: clean up get_modem_status Johan Hovold
2013-05-05 18:32                         ` [PATCH 4/7] USB: ftdi_sio: fix chars_in_buffer overhead Johan Hovold
2013-05-05 18:32                         ` [PATCH 5/7] USB: io_ti: " Johan Hovold
2013-05-05 18:32                         ` [PATCH 6/7] USB: ti_usb_3410_5052: " Johan Hovold
2013-05-05 18:32                         ` [PATCH 7/7] USB: serial: clean up chars_in_buffer Johan Hovold
     [not found]                         ` <81D166EE-BB85-4A72-A6FA-A1F6B5633CB0@caylan.net>
2013-05-20 10:07                           ` [PATCH 0/7] USB: serial: add wait_until_sent-support Johan Hovold
2013-05-04 12:44                   ` Regression: ftdi_sio is slow (since Wed Oct 10 15:05:06 2012) Stas Sergeev
2013-05-04  9:37               ` Stas Sergeev
2013-05-03 18:15       ` Stas Sergeev

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=51843132.9040304@list.ru \
    --to=stsp@list.ru \
    --cc=alan@linux.intel.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=i@caylan.net \
    --cc=jarkko.huijts@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-serial@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=rjw@sisk.pl \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox