[PATCH] new tty buffering access fix

[PATCH] new tty buffering access fix

[Paul Fulghum]

Fix typos in new tty buffering that incorrectly
access and update buffers in pending queue.

Signed-off-by: Paul Fulghum <paulkf@microgate.com>
Acked-by: Alan Cox <alan@redhat.com>


--- linux-2.6.15/drivers/char/tty_io.c	2006-01-11 16:22:10.000000000 -0600
+++ linux-2.6.15-mg/drivers/char/tty_io.c	2006-01-11 16:21:52.000000000 -0600
@@ -312,7 +312,7 @@ 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.head, *n;
+	struct tty_buffer *b = tty->buf.tail, *n;
 	int left = 0;
 
 	/* OPTIMISATION: We could keep a per tty "zero" sized buffer to
@@ -326,7 +326,6 @@ int tty_buffer_request_room(struct tty_s
 	n = tty_buffer_find(tty, size);
 	if(n == NULL)
 		return left;
-	n->next = b;
 	if(b != NULL)
 		b->next = n;
 	else
@@ -2751,6 +2750,8 @@ static void flush_to_ldisc(void *private
 	spin_lock_irqsave(&tty->read_lock, flags);
 	while((tbuf = tty->buf.head) != NULL) {
 		tty->buf.head = tbuf->next;
+		if (tty->buf.head == NULL)
+			tty->buf.tail = NULL;
 		spin_unlock_irqrestore(&tty->read_lock, flags);
 		/* printk("Process buffer %p for %d\n", tbuf, tbuf->used); */
 		disc->receive_buf(tty, tbuf->char_buf_ptr,
@@ -2759,7 +2760,6 @@ static void flush_to_ldisc(void *private
 		spin_lock_irqsave(&tty->read_lock, flags);
 		tty_buffer_free(tty, tbuf);
 	}
-	tty->buf.tail = NULL;
 	spin_unlock_irqrestore(&tty->read_lock, flags);
 out:
 	tty_ldisc_deref(disc);

Re: [PATCH] new tty buffering access fix

[Andrew Morton]

Paul Fulghum <paulkf@microgate.com> wrote:
>
>  Fix typos in new tty buffering that incorrectly
>  access and update buffers in pending queue.

Curious.  How did this manage to sneak through without anyone noticing? 
Does tty_buffer_request_room() mostly work, or do only rarely-used drivers
use it, or what?

Re: [PATCH] new tty buffering access fix

[Paul Fulghum]

On Wed, 2006-01-11 at 16:08 -0800, Andrew Morton wrote:
> Curious.  How did this manage to sneak through without anyone noticing? 
> Does tty_buffer_request_room() mostly work, or do only rarely-used drivers
> use it, or what?

At common speeds you never have more than one
buffer on the queue when the function in question is called.
So looking at the head is the same as looking at the tail.

In my recent testing, I cranked the speed up and at 460800bps
the queue became corrupted when more than one buffer was pending.
This is the important part of the patch.

With this patch, I've been running multiple devices (synclink_gt)
simultaneously on an SMP machine from 38400bps to 921600bps and
the new buffering is solid.

The part of the patch in flush_to_ldisc closes a tiny hole
where an ISR might access the queue tail after flush_to_ldisc
has removed it from the queue.

-- 
Paul Fulghum
Microgate Systems, Ltd

Re: [PATCH] new tty buffering access fix

[Alan Cox]

On Mer, 2006-01-11 at 16:08 -0800, Andrew Morton wrote:
> Paul Fulghum <paulkf@microgate.com> wrote:
> >
> >  Fix typos in new tty buffering that incorrectly
> >  access and update buffers in pending queue.
> 
> Curious.  How did this manage to sneak through without anyone noticing? 
> Does tty_buffer_request_room() mostly work, or do only rarely-used drivers
> use it, or what?

Bit of both. It works unless you use request_room _and_ queue buffers
very fast. Most drivers do neither some do one only, but fortunately
Paul did both