Prev: Linking problem using autotool
Next: why do the Muslims bow down to ward the Ka'
From: Jef Driesen on 4 Jan 2010 17:40
On 04/01/10 17:09, David Schwartz wrote:
> Aha!
>
> /**
> * tty_schedule_flip - push characters to ldisc
> * @tty: tty to push from
> *
> * Takes any pending buffers and transfers their ownership to the
> * ldisc side of the queue. It then schedules those characters
> for
> * processing by the line discipline.
> *
> * Locking: Takes tty->buf.lock
> */
>
> void tty_schedule_flip(struct tty_struct *tty)
> {
> unsigned long flags;
> spin_lock_irqsave(&tty->buf.lock, flags);
> if (tty->buf.tail != NULL)
> tty->buf.tail->commit = tty->buf.tail->used;
> spin_unlock_irqrestore(&tty->buf.lock, flags);
> schedule_delayed_work(&tty->buf.work, 1);
> }
> EXPORT_SYMBOL(tty_schedule_flip);
>
> This is the problem. This code specifically asks that the wakeup be
> delayed one jiffy. Changing the "1" to a "0" should eliminate the
> problem, though performance might we worse for "bulk data" cases.
> (Consider a program that 'dribbles' bytes into the pty.)
>
> My bet is the coder assumed that ttys would always be much slower than
> the tick rate and so it made sense to accumulate characters rather
> than scheduling a consumer more than once in a single tick.

I think writing byte by byte is a bad idea anyway. Or will the
performance also suffer when larger amounts of data are written at once?
I suppose not.

I patched the ubuntu kernel, but I don't notice any change :-(
(I don't have much experience building kernels, but I think I did it right.)
From: David Schwartz on 4 Jan 2010 21:31
On Jan 4, 2:40 pm, Jef Driesen <jefdrie...(a)hotmail.com.invalid> wrote:

> I think writing byte by byte is a bad idea anyway. Or will the
> performance also suffer when larger amounts of data are written at once?
> I suppose not.

I don't think so. However, it's also possible that this code hides a
race condition. For example, what if the code looks like this:

1) Do some processing.
2) Schedule to wake the other end after a delay.
3) Do some more processing.

Here, removing the delay will cause the other process to sometimes run
before we do the processing in step 3. The delay may actually be
needed to ensure this doesn't happen.

> I patched the ubuntu kernel, but I don't notice any change :-(
> (I don't have much experience building kernels, but I think I did it right.)

Are you sure you installed and ran your new kernel? The 'uname -v'
command will tell you.

DS
From: Jef Driesen on 5 Jan 2010 03:42
On 5/01/2010 3:31, David Schwartz wrote:
> On Jan 4, 2:40 pm, Jef Driesen<jefdrie...(a)hotmail.com.invalid> wrote:
>
>> I think writing byte by byte is a bad idea anyway. Or will the
>> performance also suffer when larger amounts of data are written at once?
>> I suppose not.
>
> I don't think so. However, it's also possible that this code hides a
> race condition. For example, what if the code looks like this:
>
> 1) Do some processing.
> 2) Schedule to wake the other end after a delay.
> 3) Do some more processing.
>
> Here, removing the delay will cause the other process to sometimes run
> before we do the processing in step 3. The delay may actually be
> needed to ensure this doesn't happen.
>
>> I patched the ubuntu kernel, but I don't notice any change :-(
>> (I don't have much experience building kernels, but I think I did it right.)
>
> Are you sure you installed and ran your new kernel? The 'uname -v'
> command will tell you.

Yes, I rebuild the ubuntu kernel packages, following the instructions on
this page [1] (I used the apt-get source method). When installing the
resulting deb package, the previously installed Ubuntu package got
replaced (and I installed an older kernel too, in case something went
wrong). After rebooting, uname confirms I'm running the rebuild kernel:

$uname -v
#53 SMP Mon Jan 4 21:00:47 CET 2010

[1] https://help.ubuntu.com/community/Kernel/Compile
From: David Schwartz on 5 Jan 2010 09:06
On Jan 5, 12:42 am, Jef Driesen <jefdrie...(a)hotmail.com.invalid>
wrote:

> Yes, I rebuild the ubuntu kernel packages, following the instructions on
> this page [1] (I used the apt-get source method). When installing the
> resulting deb package, the previously installed Ubuntu package got
> replaced (and I installed an older kernel too, in case something went
> wrong). After rebooting, uname confirms I'm running the rebuild kernel:

*sigh* It may be that it rounds up and zero and one have the same
effect. I think you can change:
schedule_delayed_work(&tty->buf.work, 1);
to:
flush_to_ldisc(&tty->buf.work.work);
And it will process it right then and there. But I'm not really sure
it's safe.

DS
From: Rainer Weikusat on 5 Jan 2010 09:56
David Schwartz <davids(a)webmaster.com> writes:
> Jef Driesen <jefdrie...(a)hotmail.com.invalid> wrote:
>
>> Yes, I rebuild the ubuntu kernel packages, following the instructions on
>> this page [1] (I used the apt-get source method). When installing the
>> resulting deb package, the previously installed Ubuntu package got
>> replaced (and I installed an older kernel too, in case something went
>> wrong). After rebooting, uname confirms I'm running the rebuild kernel:
>
> *sigh* It may be that it rounds up and zero and one have the same
> effect. I think you can change:
> schedule_delayed_work(&tty->buf.work, 1);
> to:
> flush_to_ldisc(&tty->buf.work.work);
> And it will process it right then and there. But I'm not really sure
> it's safe.

Some discussion of this is available here

http://lkml.indiana.edu/hypermail/linux/kernel/0707.2/index.html#1566

as part of the 'Use tty_schedule in VT code'-thread. The 2.6.32.2
kernel I am presently using apparently includes a 'tty low latency
mode' and the code there looks like this:

void tty_flip_buffer_push(struct tty_struct *tty)
{
unsigned long flags;
spin_lock_irqsave(&tty->buf.lock, flags);
if (tty->buf.tail != NULL)
tty->buf.tail->commit = tty->buf.tail->used;
spin_unlock_irqrestore(&tty->buf.lock, flags);

if (tty->low_latency)
flush_to_ldisc(&tty->buf.work.work);
else
schedule_delayed_work(&tty->buf.work, 1);
}

And the comment above it warns that 'This function must not be called
from IRQ context if tty->low_latency is set' (tty_flip_buffer_push is
called from pty_write).
First  |  Prev  |  Next  |  Last
Pages: 1 2 3 4 5
Prev: Linking problem using autotool
Next: why do the Muslims bow down to ward the Ka'