Re: [PATCH 1/3] tty: Sort out the USB sysrq changes that wrecked performance

From: Fengwei Yin
Date: Thu Jul 09 2009 - 19:30:23 EST


On Thu, Jul 9, 2009 at 11:49 PM, Fengwei Yin<yfw.kernel@xxxxxxxxx> wrote:
> Hi Alan,
>
> On Thu, Jul 9, 2009 at 8:35 PM, Alan Cox<alan@xxxxxxxxxxxxxxxxxxx> wrote:
>> From: Alan Cox <alan@xxxxxxxxxxxxxxx>
>>
>> We can't go around calling all sorts of magic per character functions at
>> full rate 3G data speed.
>>
>> Signed-off-by: Alan Cox <alan@xxxxxxxxxxxxxxx>
>> ---
>>
>>  drivers/usb/serial/generic.c |   15 +++++++++++----
>>  1 files changed, 11 insertions(+), 4 deletions(-)
>>
>>
>> diff --git a/drivers/usb/serial/generic.c b/drivers/usb/serial/generic.c
>> index 932d624..e9aa7a4 100644
>> --- a/drivers/usb/serial/generic.c
>> +++ b/drivers/usb/serial/generic.c
>> @@ -424,10 +424,17 @@ static void flush_and_resubmit_read_urb(struct usb_serial_port *port)
>>        if (!tty)
>>                goto done;
>>
>> -       /* Push data to tty */
>> -       for (i = 0; i < urb->actual_length; i++, ch++) {
>> -               if (!usb_serial_handle_sysrq_char(port, *ch))
>> -                       tty_insert_flip_char(tty, *ch, TTY_NORMAL);
>> +       /* The per character mucking around with sysrq path it too slow for
>> +          stuff like 3G modems, so shortcircuit it in the 99.9999999% of cases
>> +          where the USB serial is not a console anyway */
>> +       if (!port->console || !port->sysrq)
>> +               tty_insert_flip_string(tty, ch, urb->actual_length);
>> +       else {
>> +               /* Push data to tty */
>> +               for (i = 0; i < urb->actual_length; i++, ch++) {
>> +                       if (!usb_serial_handle_sysrq_char(port, *ch))
>> +                               tty_insert_flip_char(tty, *ch, TTY_NORMAL);
>> +               }
>>        }
>>        tty_flip_buffer_push(tty);
>>        tty_kref_put(tty);
>>
>
> What about the change like this:
>
> diff --git a/drivers/usb/serial/generic.c b/drivers/usb/serial/generic.c
> index 932d624..b4fd33b 100644
> --- a/drivers/usb/serial/generic.c
> +++ b/drivers/usb/serial/generic.c
> @@ -424,10 +424,26 @@ static void flush_and_resubmit_read_urb(struct
> usb_serial_port *port)
>        if (!tty)
>                goto done;
>
> -       /* Push data to tty */
> -       for (i = 0; i < urb->actual_length; i++, ch++) {
> -               if (!usb_serial_handle_sysrq_char(port, *ch))
> -                       tty_insert_flip_char(tty, *ch, TTY_NORMAL);
> +        /* The per character mucking around with sysrq path it too slow for
> +           stuff like 3G modems, so shortcircuit it in the 99.9999999% of cases
> +           where the USB serial is not a console anyway */
> +        if (!port->console || !port->sysrq)
> +                tty_insert_flip_string(tty, ch, urb->actual_length);
> +        else {
> +               /*
> +                * Most of data shouldn't be sysrq request even when
> +                * tty is a printk console.
> +                */
> +               if (time_before(jiffies, port->sysrq)) {
> +                       /* Push data to tty */
> +                       for (i = 0; i < urb->actual_length; i++, ch++) {
> +                               if (!usb_serial_handle_sysrq_char(port, *ch))
> +                                       tty_insert_flip_char(tty, *ch,
> +                                                       TTY_NORMAL);
> +                       }
> +               } else {
> +                       tty_insert_flip_string(tty, ch, urb->actual_length);
> +               }
>        }
>        tty_flip_buffer_push(tty);
>        tty_kref_put(tty);
>
Oh. Just ignore my change. If it's console, we don't need this "optimization".

Regards
Yin, Fengwei

>
> Regards
> Yin, Fengwei
>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to majordomo@xxxxxxxxxxxxxxx
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at  http://www.tux.org/lkml/
>>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/