pppd oopses current linu's git tree on disconnect

pppd oopses current linu's git tree on disconnect

[Diego Calleja]

I got this on my log files (56k serial modem, pentium 3 smp machine, the
machine didn't hang, in fact I could connect again to send this bug
report). If needed, here's the dmesg: http://www.terra.es/personal/diegocg/dmesg
and .config: http://www.terra.es/personal/diegocg/.config. I've never seen
this so I assumed it could be a problem with the "TTY layer buffering revamp"

Jan 18 22:07:23 estel pppd[5338]: Serial link appears to be disconnected.
Jan 18 22:07:23 estel pppd[5338]: Connect time 296.0 minutes.
Jan 18 22:07:23 estel pppd[5338]: Sent 18290428 bytes, received 40448937 bytes.
Jan 18 22:07:23 estel pppd[5338]: Script /etc/ppp/ip-down started (pid 8044)
Jan 18 22:07:23 estel pppd[5338]: sent [LCP TermReq id=0x2 "Peer not responding"]
Jan 18 22:07:23 estel pppd[5338]: rcvd [LCP TermReq id=0x2 "Peer not responding"]
Jan 18 22:07:23 estel pppd[5338]: sent [LCP TermAck id=0x2]
Jan 18 22:07:23 estel pppd[5338]: rcvd [LCP TermAck id=0x2]
Jan 18 22:07:23 estel pppd[5338]: Connection terminated.
Jan 18 22:07:24 estel kernel: Unable to handle kernel paging request at virtual address f554fbf8
Jan 18 22:07:24 estel kernel:  printing eip:
Jan 18 22:07:24 estel kernel: c01fb9e7
Jan 18 22:07:24 estel kernel: *pde = 0043e067
Jan 18 22:07:24 estel kernel: *pte = 3554f000
Jan 18 22:07:24 estel kernel: Oops: 0000 [#1]
Jan 18 22:07:24 estel kernel: PREEMPT SMP DEBUG_PAGEALLOC
Jan 18 22:07:24 estel kernel: Modules linked in: ipt_REJECT xt_tcpudp radeon lp thermal fan button processor ac ipt_MASQUERADE iptable_nat ip
_nat ip_conntrack iptable_filter ip_tables x_tables usbhid parport_pc parport pcspkr floppy ohci_hcd usbcore e100 ide_cd cdrom unix
Jan 18 22:07:24 estel kernel: CPU:    1
Jan 18 22:07:24 estel kernel: EIP:    0060:[tty_buffer_free_all+35/73]    Not tainted VLI
Jan 18 22:07:24 estel kernel: EFLAGS: 00010282   (2.6.16-rc1)
Jan 18 22:07:24 estel kernel: EIP is at tty_buffer_free_all+0x23/0x49
Jan 18 22:07:24 estel kernel: eax: 0000000a   ebx: efab57f8   ecx: 3421a000   edx: f554fbf8
Jan 18 22:07:24 estel kernel: esi: efab57f8   edi: 00000000   ebp: c1aedea8   esp: c1aedea4
Jan 18 22:07:24 estel kernel: ds: 007b   es: 007b   ss: 0068
Jan 18 22:07:24 estel kernel: Process pppd (pid: 5338, threadinfo=c1aed000 task=f407dac0)
Jan 18 22:07:24 estel kernel: Stack: <0>00000000 c1aedec0 c01fd737 00000001 efab57f8 00000001 00000000 c1aedf54
Jan 18 22:07:24 estel kernel:        c01ff251 f43c7f4c 00aedee8 00000000 00000000 00000001 00000000 00000000
Jan 18 22:07:24 estel kernel:        00000001 c1aedef0 c02c3d79 c1aedf18 c01465ca 0808b81c f0cb7c10 f465ae04
Jan 18 22:07:24 estel kernel: Call Trace:
Jan 18 22:07:24 estel kernel:  [show_stack_log_lvl+170/181] show_stack_log_lvl+0xaa/0xb5
Jan 18 22:07:24 estel kernel:  [show_registers+306/414] show_registers+0x132/0x19e
Jan 18 22:07:24 estel kernel:  [die+360/493] die+0x168/0x1ed
Jan 18 22:07:24 estel kernel:  [do_page_fault+921/1239] do_page_fault+0x399/0x4d7
Jan 18 22:07:24 estel kernel:  [error_code+79/84] error_code+0x4f/0x54
Jan 18 22:07:24 estel kernel:  [release_mem+499/512] release_mem+0x1f3/0x200
Jan 18 22:07:24 estel kernel:  [release_dev+1651/1719] release_dev+0x673/0x6b7
Jan 18 22:07:24 estel kernel:  [tty_release+18/28] tty_release+0x12/0x1c
Jan 18 22:07:24 estel kernel:  [__fput+171/362] __fput+0xab/0x16a
Jan 18 22:07:24 estel kernel:  [fput+23/27] fput+0x17/0x1b
Jan 18 22:07:24 estel kernel:  [filp_close+81/91] filp_close+0x51/0x5b
Jan 18 22:07:24 estel kernel:  [sys_close+115/135] sys_close+0x73/0x87
Jan 18 22:07:24 estel kernel:  [sysenter_past_esp+84/117] sysenter_past_esp+0x54/0x75
Jan 18 22:07:24 estel kernel: Code: d8 5b 5e 5f 5d c3 90 90 55 89 e5 53 89 c3 eb 0f 8b 02 89 83 3c 01 00 00 89 d0 e8 8e 65 f5 ff 8b 93 3c 01
00 00 85 d2 75 e7 eb 0f <8b> 02 89 83 44 01 00 00 89 d0 e8 73 65 f5 ff 8b 93 44 01 00 00

Re: pppd oopses current linu's git tree on disconnect

[Paul Fulghum]

On Thu, 2006-01-19 at 01:06 +0100, Diego Calleja wrote:
> I got this on my log files (56k serial modem, pentium 3 smp machine, the
> machine didn't hang, in fact I could connect again to send this bug
> report). If needed, here's the dmesg: http://www.terra.es/personal/diegocg/dmesg
> and .config: http://www.terra.es/personal/diegocg/.config. I've never seen
> this so I assumed it could be a problem with the "TTY layer buffering revamp"

Can you try the attached patch please?
Does this occur frequently?

Thanks

-- 
Paul Fulghum
Microgate Systems, Ltd
--- linux-2.6.16-rc1/include/linux/tty_flip.h	2006-01-19 10:18:49.000000000 -0600
+++ linux-2.6.16-rc1-mg/include/linux/tty_flip.h	2006-01-19 10:20:50.000000000 -0600
@@ -16,13 +16,23 @@ extern int tty_prepare_flip_string_flags
 _INLINE_ int tty_insert_flip_char(struct tty_struct *tty,
 				   unsigned char ch, char flag)
 {
-	struct tty_buffer *tb = tty->buf.tail;
+	struct tty_buffer *tb;
+	unsigned long flags;
+	int rc;
+
+	spin_lock_irqsave(&tty->read_lock, flags);
+
+	tb = tty->buf.tail;
 	if (tb && tb->used < tb->size) {
 		tb->flag_buf_ptr[tb->used] = flag;
 		tb->char_buf_ptr[tb->used++] = ch;
+		spin_unlock_irqrestore(&tty->read_lock, flags);
 		return 1;
 	}
-	return tty_insert_flip_string_flags(tty, &ch, &flag, 1);
+	rc = tty_insert_flip_string_flags(tty, &ch, &flag, 1);
+
+	spin_unlock_irqrestore(&tty->read_lock, flags);
+	return rc;
 }
 
 _INLINE_ void tty_schedule_flip(struct tty_struct *tty)

Re: pppd oopses current linu's git tree on disconnect

[Diego Calleja]

El Thu, 19 Jan 2006 11:33:59 -0600,
Paul Fulghum <paulkf@microgate.com> escribió:

> Can you try the attached patch please?
> Does this occur frequently?

Not at all - I've tried to trigger it tons of times and didn't happen again,
I even put a pon/poff in a loop but nothing happened; so I can't confirm if
your patch does fix it, but I'm running the patch and nothing bad seems to
happen.

Re: pppd oopses current linu's git tree on disconnect

[Paul Fulghum]

Diego Calleja wrote:
>>Does this occur frequently?
> 
> Not at all - I've tried to trigger it tons of times and didn't happen again,
> I even put a pon/poff in a loop but nothing happened; so I can't confirm if
> your patch does fix it, but I'm running the patch and nothing bad seems to
> happen.

The only way I can see for the oops you reported to occur
is for the pending or free list of tty buffers to get corrupted.
This causes the oops when trying to free all the buffers.

The buffer handling code looks correct, but there is a locking
(or lack of) issue that could mess up the lists on an SMP machine.
The serial ISR does not hold the tty->read_lock when pushing data
which is used to synchronize access to the tty buffer list.
The test patch adds this lock to the function used
(by the standard serial driver) to push the data .

The chances of this happening increase with the speed and
continued use of the serial port. Your original report showed
a lengthy PPP session. So bringing the link down and up without
significant data transfer will probably not trigger this.

Thanks,
Paul

--
Paul Fulghum
Microgate Systems, Ltd

Re: pppd oopses current linu's git tree on disconnect

[Diego Calleja]

El Thu, 19 Jan 2006 16:39:51 -0600,
Paul Fulghum <paulkf@microgate.com> escribió:


> The chances of this happening increase with the speed and
> continued use of the serial port. Your original report showed
> a lengthy PPP session. So bringing the link down and up without
> significant data transfer will probably not trigger this.


Ok - I seem to be able to reproduce it in a unpatched kernel using
a lengthy session. I will try your patch and report back.

Re: pppd oopses current linu's git tree on disconnect

[Diego Calleja]

El Mon, 23 Jan 2006 03:42:43 +0100,
Diego Calleja <diegocg@gmail.com> escribió:

> Ok - I seem to be able to reproduce it in a unpatched kernel using
> a lengthy session. I will try your patch and report back.

It doesn't seems to happen today at least, after a lentghty session
similar to the one which triggered the bug.

Re: pppd oopses current linu's git tree on disconnect

[Paul Fulghum]

On Tue, 2006-01-24 at 04:48 +0100, Diego Calleja wrote:
> El Mon, 23 Jan 2006 03:42:43 +0100,
> Diego Calleja <diegocg@gmail.com> escribió:
> 
> > Ok - I seem to be able to reproduce it in a unpatched kernel using
> > a lengthy session. I will try your patch and report back.
> 
> It doesn't seems to happen today at least, after a lentghty session
> similar to the one which triggered the bug.

I reasonably sure that your problem is lack
of locking for the new tty buffering scheme.
There is a question of how to best fix it
with minimal code change and impact on performance.

Alan:

Look at the attached patches against 2.6.16-rc1

1. I added an active flag for each tty buffer

2. I added locking to anything that manipulates the
   buffer lists or sets/clears an active flag
   This uses a lock dedicated to the tty buffer lists
   instead of using the tty->read_lock to reduce lock
   contention.

3. Calls to tty insertion functions set the active
   flag for the tail buffer:
   tty_buffer_request_room()
   tty_insert_flip_string()
   tty_insert_flip_string_flags()
   tty_prepare_flip_string()
   tty_prepare_flip_string_flags()
   tty_insert_flip_char()

4. The active flag is cleared when calling
   tty_schedule_flip()
   con_schedule_flip()
   tty_flip_buffer_push()

This lets the tail buffer be 'owned' by the driver
while filling it. flush_to_ldisc stops when an
active buffer is encountered. Calling the scheduling
functions in #4 releases the buffer for use by flush_to_ldisc.

This scheme centralizes the locking so every driver does not
need to be modified, and avoids excessive locking/unlocking
for each inserted character. The lock is grabbed once by
the driver to activate (get ownership of) the tail buffer.
The driver fills the buffer (possibly one char at a time).
When the driver is done, the buffer is marked inactive and
flush_to_ldisc can process it.

I've done some initial testing with success.
Let me know what you think.

--- linux-2.6.16-rc1/drivers/char/tty_io.c	2006-01-24 15:31:45.000000000 -0600
+++ linux-2.6.16-rc1-mg/drivers/char/tty_io.c	2006-01-24 15:33:28.000000000 -0600
@@ -253,6 +253,7 @@ static void tty_buffer_free_all(struct t
 
 static void tty_buffer_init(struct tty_struct *tty)
 {
+	spin_lock_init(&tty->buf.lock);
 	tty->buf.head = NULL;
 	tty->buf.tail = NULL;
 	tty->buf.free = NULL;
@@ -266,6 +267,7 @@ static struct tty_buffer *tty_buffer_all
 	p->used = 0;
 	p->size = size;
 	p->next = NULL;
+	p->active = 0;
 	p->char_buf_ptr = (char *)(p->data);
 	p->flag_buf_ptr = (unsigned char *)p->char_buf_ptr + size;
 /* 	printk("Flip create %p\n", p); */
@@ -312,25 +314,36 @@ static struct tty_buffer *tty_buffer_fin
 
 int tty_buffer_request_room(struct tty_struct *tty, size_t size)
 {
-	struct tty_buffer *b = tty->buf.tail, *n;
+	struct tty_buffer *b, *n;
 	int left = 0;
+	unsigned long flags;
+
+	spin_lock_irqsave(&tty->buf.lock, flags);
 
 	/* OPTIMISATION: We could keep a per tty "zero" sized buffer to
 	   remove this conditional if its worth it. This would be invisible
 	   to the callers */
-	if(b != NULL)
+	if ((b = tty->buf.tail) != NULL)
 		left = b->size - b->used;
 	if(left >= size)
-		return size;
+		goto done;
+
 	/* This is the slow path - looking for new buffers to use */
 	n = tty_buffer_find(tty, size);
-	if(n == NULL)
-		return left;
+	if(n == NULL) {
+		size = left;
+		goto done;
+	}
 	if(b != NULL)
 		b->next = n;
 	else
 		tty->buf.head = n;
 	tty->buf.tail = n;
+
+done:
+	if (tty->buf.tail != NULL)
+		tty->buf.tail->active = 1;
+	spin_unlock_irqrestore(&tty->buf.lock, flags);
 	return size;
 }
 
@@ -396,10 +409,12 @@ EXPORT_SYMBOL_GPL(tty_insert_flip_string
 int tty_prepare_flip_string(struct tty_struct *tty, unsigned char **chars, size_t size)
 {
 	int space = tty_buffer_request_room(tty, size);
-	struct tty_buffer *tb = tty->buf.tail;
-	*chars = tb->char_buf_ptr + tb->used;
-	memset(tb->flag_buf_ptr + tb->used, TTY_NORMAL, space);
-	tb->used += space;
+	if (space) {
+		struct tty_buffer *tb = tty->buf.tail;
+		*chars = tb->char_buf_ptr + tb->used;
+		memset(tb->flag_buf_ptr + tb->used, TTY_NORMAL, space);
+		tb->used += space;
+	}
 	return space;
 }
 
@@ -416,10 +431,12 @@ EXPORT_SYMBOL_GPL(tty_prepare_flip_strin
 int tty_prepare_flip_string_flags(struct tty_struct *tty, unsigned char **chars, char **flags, size_t size)
 {
 	int space = tty_buffer_request_room(tty, size);
-	struct tty_buffer *tb = tty->buf.tail;
-	*chars = tb->char_buf_ptr + tb->used;
-	*flags = tb->flag_buf_ptr + tb->used;
-	tb->used += space;
+	if (space) {
+		struct tty_buffer *tb = tty->buf.tail;
+		*chars = tb->char_buf_ptr + tb->used;
+		*flags = tb->flag_buf_ptr + tb->used;
+		tb->used += space;
+	}
 	return space;
 }
 
@@ -2748,7 +2765,7 @@ static void flush_to_ldisc(void *private
 		goto out;
 	}
 	spin_lock_irqsave(&tty->read_lock, flags);
-	while((tbuf = tty->buf.head) != NULL) {
+	while((tbuf = tty->buf.head) != NULL && !tbuf->active) {
 		tty->buf.head = tbuf->next;
 		if (tty->buf.head == NULL)
 			tty->buf.tail = NULL;
@@ -2852,6 +2869,12 @@ EXPORT_SYMBOL(tty_get_baud_rate);
 
 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->active = 0;
+	spin_unlock_irqrestore(&tty->buf.lock, flags);
+
 	if (tty->low_latency)
 		flush_to_ldisc((void *) tty);
 	else
--- linux-2.6.16-rc1/include/linux/tty.h	2006-01-17 09:31:30.000000000 -0600
+++ linux-2.6.16-rc1-mg/include/linux/tty.h	2006-01-24 14:12:19.000000000 -0600
@@ -57,6 +57,7 @@ struct tty_buffer {
 	unsigned char *flag_buf_ptr;
 	int used;
 	int size;
+	int active;
 	/* Data points here */
 	unsigned long data[0];
 };
@@ -64,6 +65,7 @@ struct tty_buffer {
 struct tty_bufhead {
 	struct work_struct		work;
 	struct semaphore pty_sem;
+	spinlock_t lock;
 	struct tty_buffer *head;	/* Queue head */
 	struct tty_buffer *tail;	/* Active buffer */
 	struct tty_buffer *free;	/* Free queue head */
--- linux-2.6.16-rc1/include/linux/tty_flip.h	2006-01-24 12:24:47.000000000 -0600
+++ linux-2.6.16-rc1-mg/include/linux/tty_flip.h	2006-01-24 14:46:59.000000000 -0600
@@ -17,7 +17,7 @@ _INLINE_ int tty_insert_flip_char(struct
 				   unsigned char ch, char flag)
 {
 	struct tty_buffer *tb = tty->buf.tail;
-	if (tb && tb->used < tb->size) {
+	if (tb && tb->active && tb->used < tb->size) {
 		tb->flag_buf_ptr[tb->used] = flag;
 		tb->char_buf_ptr[tb->used++] = ch;
 		return 1;
@@ -27,6 +27,11 @@ _INLINE_ int tty_insert_flip_char(struct
 
 _INLINE_ 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->active = 0;
+	spin_unlock_irqrestore(&tty->buf.lock, flags);
 	schedule_delayed_work(&tty->buf.work, 1);
 }
 
--- linux-2.6.16-rc1/include/linux/kbd_kern.h	2006-01-17 09:31:29.000000000 -0600
+++ linux-2.6.16-rc1-mg/include/linux/kbd_kern.h	2006-01-24 15:38:19.000000000 -0600
@@ -151,6 +151,11 @@ extern unsigned int keymap_count;
 
 static inline void con_schedule_flip(struct tty_struct *t)
 {
+	unsigned long flags;
+	spin_lock_irqsave(&t->buf.lock, flags);
+	if (t->buf.tail != NULL)
+		t->buf.tail->active = 0;
+	spin_unlock_irqrestore(&t->buf.lock, flags);
 	schedule_work(&t->buf.work);
 }
 

Re: pppd oopses current linu's git tree on disconnect

[Alan Cox]

On Maw, 2006-01-24 at 16:06 -0600, Paul Fulghum wrote:
> I reasonably sure that your problem is lack
> of locking for the new tty buffering scheme.
> There is a question of how to best fix it
> with minimal code change and impact on performance.

Yeah the new tty code assumed the same locking rules as the old tty code
and nobody on the planet followed them since 2.2.

> I've done some initial testing with success.
> Let me know what you think.

I think you've been reading my mind, only you've actually come up with a
slightly neater variant than I have half coded here.

>  int tty_prepare_flip_string(struct tty_struct *tty, unsigned char **chars, size_t size)
>  {
>  	int space = tty_buffer_request_room(tty, size);
> -	struct tty_buffer *tb = tty->buf.tail;
> -	*chars = tb->char_buf_ptr + tb->used;
> -	memset(tb->flag_buf_ptr + tb->used, TTY_NORMAL, space);
> -	tb->used += space;
> +	if (space) {
> +		struct tty_buffer *tb = tty->buf.tail;
> +		*chars = tb->char_buf_ptr + tb->used;
> +		memset(tb->flag_buf_ptr + tb->used, TTY_NORMAL, space);
> +		tb->used += space;
> +	}

This seems unrelated and also not useful. 0 space is such a special case
that it isn't worth the check - and it works fine anyway with 0.

> +	if (space) {
> +		struct tty_buffer *tb = tty->buf.tail;
> +		*chars = tb->char_buf_ptr + tb->used;
> +		*flags = tb->flag_buf_ptr + tb->used;
> +		tb->used += space;
> +	}

Ditto


> --- linux-2.6.16-rc1/include/linux/kbd_kern.h	2006-01-17 09:31:29.000000000 -0600
> +++ linux-2.6.16-rc1-mg/include/linux/kbd_kern.h	2006-01-24 15:38:19.000000000 -0600
> @@ -151,6 +151,11 @@ extern unsigned int keymap_count;
>  
>  static inline void con_schedule_flip(struct tty_struct *t)

Should die as a duplicate by the look of it, and the tty one probably
should cease to be inline.

Looks good to me.

Alan

Re: pppd oopses current linu's git tree on disconnect

[Paul Fulghum]

Alan Cox wrote:
> Yeah the new tty code assumed the same locking rules as the old tty code
> and nobody on the planet followed them since 2.2.

I could not find any code that used the tty read_lock
when pushing data. So at least its a clean start.

> I think you've been reading my mind, only you've actually come up with a
> slightly neater variant than I have half coded here.

OK, good.

>> int tty_prepare_flip_string(struct tty_struct *tty, unsigned char **chars, size_t size)
>> {
>> 	int space = tty_buffer_request_room(tty, size);
>>-	struct tty_buffer *tb = tty->buf.tail;
>>-	*chars = tb->char_buf_ptr + tb->used;
>>-	memset(tb->flag_buf_ptr + tb->used, TTY_NORMAL, space);
>>-	tb->used += space;
>>+	if (space) {
>>+		struct tty_buffer *tb = tty->buf.tail;
>>+		*chars = tb->char_buf_ptr + tb->used;
>>+		memset(tb->flag_buf_ptr + tb->used, TTY_NORMAL, space);
>>+		tb->used += space;
>>+	}

Unrelated, yes.
But if space == 0 then tty->buf.tail could be NULL
Touching tb could oops. I think you already do a similar
check in tty_insert_flip_string() etc.

>> static inline void con_schedule_flip(struct tty_struct *t)
> 
> Should die as a duplicate by the look of it, and the tty one probably
> should cease to be inline.

The only difference seems to be schedule_delayed_work()
in tty_schedule_flip() vs schedule_work() in con_schedule_flip().

All three:
tty_schedule_flip()
con_schedule_flip()
tty_flip_buffer_push()
seem to be duplicates other than that.

> Looks good to me.

There is still the esp and cyclades driver which
schedule the buf.work directly which need to be
switched to one of the above 3 functions.

I also found a case where the active flag
is not cleared correctly. (when a partial buffer is
filled and a new tail buffer is allocated before
calling one of the schedule functions.

I'll fix both of these up tomorrow, post a new
patch and continue testing.

Thanks,
Paul

--
Paul Fulghum
Microgate Systems, Ltd

Re: pppd oopses current linu's git tree on disconnect

[Alan Cox]

On Maw, 2006-01-24 at 17:44 -0600, Paul Fulghum wrote:
> But if space == 0 then tty->buf.tail could be NULL
> Touching tb could oops. I think you already do a similar
> check in tty_insert_flip_string() etc.

Might be worth using unlikely() hints there. I'd not considered the NULL
case.

Re: pppd oopses current linu's git tree on disconnect

[Paul Fulghum]

Here is a refined and more thoroughly tested patch
that implements the needed tty buffer locking.

Change from last:

1. fix tty_buffer_request_room() to clear active flag
   when moving buffer from tail position

2. fixup esp and cyclades drivers which scheduled the
   flip work directly. They now use tty_schedule_flip().

3. change tty->read_lock in flush_to_ldisc() to tty->buf.lock
   like the rest of the code. (stupid, duhhh)

I've beat on it all day, and it looks good.
This is ready for anyone else to actually apply and test.

Alan:

I left the redundant con_schedule_flip, tty_schedule_flip,
and tty_flip_buffer_push functions in place for now. They
each do slightly different things, and I don't want to mix
that optimization with the locking change.

--- linux-2.6.16-rc1/drivers/char/tty_io.c	2006-01-25 13:46:06.000000000 -0600
+++ linux-2.6.16-rc1-mg/drivers/char/tty_io.c	2006-01-25 13:50:15.000000000 -0600
@@ -253,6 +253,7 @@ static void tty_buffer_free_all(struct t
 
 static void tty_buffer_init(struct tty_struct *tty)
 {
+	spin_lock_init(&tty->buf.lock);
 	tty->buf.head = NULL;
 	tty->buf.tail = NULL;
 	tty->buf.free = NULL;
@@ -266,6 +267,7 @@ static struct tty_buffer *tty_buffer_all
 	p->used = 0;
 	p->size = size;
 	p->next = NULL;
+	p->active = 0;
 	p->char_buf_ptr = (char *)(p->data);
 	p->flag_buf_ptr = (unsigned char *)p->char_buf_ptr + size;
 /* 	printk("Flip create %p\n", p); */
@@ -312,25 +314,36 @@ static struct tty_buffer *tty_buffer_fin
 
 int tty_buffer_request_room(struct tty_struct *tty, size_t size)
 {
-	struct tty_buffer *b = tty->buf.tail, *n;
-	int left = 0;
+	struct tty_buffer *b, *n;
+	int left;
+	unsigned long flags;
+
+	spin_lock_irqsave(&tty->buf.lock, flags);
 
 	/* OPTIMISATION: We could keep a per tty "zero" sized buffer to
 	   remove this conditional if its worth it. This would be invisible
 	   to the callers */
-	if(b != NULL)
+	if ((b = tty->buf.tail) != NULL) {
 		left = b->size - b->used;
-	if(left >= size)
-		return size;
-	/* This is the slow path - looking for new buffers to use */
-	n = tty_buffer_find(tty, size);
-	if(n == NULL)
-		return left;
-	if(b != NULL)
-		b->next = n;
-	else
-		tty->buf.head = n;
-	tty->buf.tail = n;
+		b->active = 1;
+	} else
+		left = 0;
+
+	if (left < size) {
+		/* This is the slow path - looking for new buffers to use */
+		if ((n = tty_buffer_find(tty, size)) != NULL) {
+			if (b != NULL) {
+				b->next = n;
+				b->active = 0;
+			} else
+				tty->buf.head = n;
+			tty->buf.tail = n;
+			n->active = 1;
+		} else
+			size = left;
+	}
+
+	spin_unlock_irqrestore(&tty->buf.lock, flags);
 	return size;
 }
 
@@ -396,10 +409,12 @@ EXPORT_SYMBOL_GPL(tty_insert_flip_string
 int tty_prepare_flip_string(struct tty_struct *tty, unsigned char **chars, size_t size)
 {
 	int space = tty_buffer_request_room(tty, size);
-	struct tty_buffer *tb = tty->buf.tail;
-	*chars = tb->char_buf_ptr + tb->used;
-	memset(tb->flag_buf_ptr + tb->used, TTY_NORMAL, space);
-	tb->used += space;
+	if (likely(space)) {
+		struct tty_buffer *tb = tty->buf.tail;
+		*chars = tb->char_buf_ptr + tb->used;
+		memset(tb->flag_buf_ptr + tb->used, TTY_NORMAL, space);
+		tb->used += space;
+	}
 	return space;
 }
 
@@ -416,10 +431,12 @@ EXPORT_SYMBOL_GPL(tty_prepare_flip_strin
 int tty_prepare_flip_string_flags(struct tty_struct *tty, unsigned char **chars, char **flags, size_t size)
 {
 	int space = tty_buffer_request_room(tty, size);
-	struct tty_buffer *tb = tty->buf.tail;
-	*chars = tb->char_buf_ptr + tb->used;
-	*flags = tb->flag_buf_ptr + tb->used;
-	tb->used += space;
+	if (likely(space)) {
+		struct tty_buffer *tb = tty->buf.tail;
+		*chars = tb->char_buf_ptr + tb->used;
+		*flags = tb->flag_buf_ptr + tb->used;
+		tb->used += space;
+	}
 	return space;
 }
 
@@ -2747,20 +2764,20 @@ static void flush_to_ldisc(void *private
 		schedule_delayed_work(&tty->buf.work, 1);
 		goto out;
 	}
-	spin_lock_irqsave(&tty->read_lock, flags);
-	while((tbuf = tty->buf.head) != NULL) {
+	spin_lock_irqsave(&tty->buf.lock, flags);
+	while((tbuf = tty->buf.head) != NULL && !tbuf->active) {
 		tty->buf.head = tbuf->next;
 		if (tty->buf.head == NULL)
 			tty->buf.tail = NULL;
-		spin_unlock_irqrestore(&tty->read_lock, flags);
+		spin_unlock_irqrestore(&tty->buf.lock, flags);
 		/* printk("Process buffer %p for %d\n", tbuf, tbuf->used); */
 		disc->receive_buf(tty, tbuf->char_buf_ptr,
 				       tbuf->flag_buf_ptr,
 				       tbuf->used);
-		spin_lock_irqsave(&tty->read_lock, flags);
+		spin_lock_irqsave(&tty->buf.lock, flags);
 		tty_buffer_free(tty, tbuf);
 	}
-	spin_unlock_irqrestore(&tty->read_lock, flags);
+	spin_unlock_irqrestore(&tty->buf.lock, flags);
 out:
 	tty_ldisc_deref(disc);
 }
@@ -2852,6 +2869,12 @@ EXPORT_SYMBOL(tty_get_baud_rate);
 
 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->active = 0;
+	spin_unlock_irqrestore(&tty->buf.lock, flags);
+
 	if (tty->low_latency)
 		flush_to_ldisc((void *) tty);
 	else
--- linux-2.6.16-rc1/include/linux/tty.h	2006-01-17 09:31:30.000000000 -0600
+++ linux-2.6.16-rc1-mg/include/linux/tty.h	2006-01-24 14:12:19.000000000 -0600
@@ -57,6 +57,7 @@ struct tty_buffer {
 	unsigned char *flag_buf_ptr;
 	int used;
 	int size;
+	int active;
 	/* Data points here */
 	unsigned long data[0];
 };
@@ -64,6 +65,7 @@ struct tty_buffer {
 struct tty_bufhead {
 	struct work_struct		work;
 	struct semaphore pty_sem;
+	spinlock_t lock;
 	struct tty_buffer *head;	/* Queue head */
 	struct tty_buffer *tail;	/* Active buffer */
 	struct tty_buffer *free;	/* Free queue head */
--- linux-2.6.16-rc1/include/linux/tty_flip.h	2006-01-24 12:24:47.000000000 -0600
+++ linux-2.6.16-rc1-mg/include/linux/tty_flip.h	2006-01-24 14:46:59.000000000 -0600
@@ -17,7 +17,7 @@ _INLINE_ int tty_insert_flip_char(struct
 				   unsigned char ch, char flag)
 {
 	struct tty_buffer *tb = tty->buf.tail;
-	if (tb && tb->used < tb->size) {
+	if (tb && tb->active && tb->used < tb->size) {
 		tb->flag_buf_ptr[tb->used] = flag;
 		tb->char_buf_ptr[tb->used++] = ch;
 		return 1;
@@ -27,6 +27,11 @@ _INLINE_ int tty_insert_flip_char(struct
 
 _INLINE_ 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->active = 0;
+	spin_unlock_irqrestore(&tty->buf.lock, flags);
 	schedule_delayed_work(&tty->buf.work, 1);
 }
 
--- linux-2.6.16-rc1/include/linux/kbd_kern.h	2006-01-17 09:31:29.000000000 -0600
+++ linux-2.6.16-rc1-mg/include/linux/kbd_kern.h	2006-01-24 15:38:19.000000000 -0600
@@ -151,6 +151,11 @@ extern unsigned int keymap_count;
 
 static inline void con_schedule_flip(struct tty_struct *t)
 {
+	unsigned long flags;
+	spin_lock_irqsave(&t->buf.lock, flags);
+	if (t->buf.tail != NULL)
+		t->buf.tail->active = 0;
+	spin_unlock_irqrestore(&t->buf.lock, flags);
 	schedule_work(&t->buf.work);
 }
 
--- linux-2.6.16-rc1/drivers/char/esp.c	2006-01-17 09:31:19.000000000 -0600
+++ linux-2.6.16-rc1-mg/drivers/char/esp.c	2006-01-25 09:48:23.000000000 -0600
@@ -359,7 +359,7 @@ static inline void receive_chars_pio(str
 		}
 	}
 
-	schedule_delayed_work(&tty->buf.work, 1);
+	tty_schedule_flip(tty);
 
 	info->stat_flags &= ~ESP_STAT_RX_TIMEOUT;
 	release_pio_buffer(pio_buf);
@@ -426,7 +426,7 @@ static inline void receive_chars_dma_don
 			}
 			tty_insert_flip_char(tty, dma_buffer[num_bytes - 1], statflag);
 		}
-		schedule_delayed_work(&tty->buf.work, 1);
+		tty_schedule_flip(tty);
 	}
 
 	if (dma_bytes != num_bytes) {
--- linux-2.6.16-rc1/drivers/char/cyclades.c	2006-01-17 09:31:19.000000000 -0600
+++ linux-2.6.16-rc1-mg/drivers/char/cyclades.c	2006-01-25 09:51:12.000000000 -0600
@@ -1233,7 +1233,7 @@ cyy_interrupt(int irq, void *dev_id, str
                             }
                              info->idle_stats.recv_idle = jiffies;
                         }
-                        schedule_delayed_work(&tty->buf.work, 1);
+			tty_schedule_flip(tty);
                     }
                     /* end of service */
                     cy_writeb(base_addr+(CyRIR<<index), (save_xir & 0x3f));
@@ -1606,7 +1606,7 @@ cyz_handle_rx(struct cyclades_port *info
 	    }
 #endif
 	    info->idle_stats.recv_idle = jiffies;
-	    schedule_delayed_work(&tty->buf.work, 1);
+	    tty_schedule_flip(tty);
 	}
 	/* Update rx_get */
 	cy_writel(&buf_ctrl->rx_get, new_rx_get);
@@ -1809,7 +1809,7 @@ cyz_handle_cmd(struct cyclades_card *cin
 	if(delta_count)
 	    cy_sched_event(info, Cy_EVENT_DELTA_WAKEUP);
 	if(special_count)
-	    schedule_delayed_work(&tty->buf.work, 1);
+		tty_schedule_flip(tty);
     }
 }