Re: [Bug #14015] pty regressed again, breaking expect and gcc's testsuite

[OGAWA Hirofumi]

Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> writes:

> On Fri, 4 Sep 2009, Linus Torvalds wrote:
>> 
>> So I'm starting to suspect that the real bug is that we do that 
>> 'pty_space()' in pty_write() call at all. The _callers_ should already 
>> have done the write_room() check, and if somebody doesn't do it, then the 
>> tty buffering will eventually do a hard limit at the 65kB allocation mark.
>
> Ok, so the thought was right, but the patch was obviously not even 
> compiled, because the compiler points out that 'c' was not initialized.
>
> I'm sure you already figured the obvious meaning out, but here's a fixed 
> version.

This is not meaning to object to your patch though, I think we would be
good to fix pty_space(), not leaving as wrong. With fix it, I guess we
don't get strange behavior in the near of buffer limit.

Also, it seems the non-n_tty path doesn't use tty_write_room() check,
and instead it just try to write and check written bytes which returned
by tty->ops->write().

So, it will use the 64kb limit at least few paths, and I'm not sure
though, non-n_tty path (e.g. ppp) doesn't use tty_write_room() check
always. It may not be consistent if we removed pty_space() in pty_write().

So, it may not be issue though, I made this patch to fix
pty_space(). What do you think?

Well, anyway, I've tested your patch and this patch fixed the
gcc-testsuite on my machine.

Thanks.
-- 
OGAWA Hirofumi <hirofumi@xxxxxxxxxxxxxxxxxx>



Signed-off-by: OGAWA Hirofumi <hirofumi@xxxxxxxxxxxxxxxxxx>
---

diff -puN drivers/char/pty.c~tty-debug drivers/char/pty.c
--- linux-2.6/drivers/char/pty.c~tty-debug	2009-09-05 20:10:35.000000000 +0900
+++ linux-2.6-hirofumi/drivers/char/pty.c	2009-09-06 01:50:44.000000000 +0900
@@ -91,7 +91,7 @@ static void pty_unthrottle(struct tty_st
 
 static int pty_space(struct tty_struct *to)
 {
-	int n = 8192 - to->buf.memory_used;
+	int n = 8192 - tty_buffer_used(to);
 	if (n < 0)
 		return 0;
 	return n;
diff -puN drivers/char/tty_buffer.c~tty-debug drivers/char/tty_buffer.c
--- linux-2.6/drivers/char/tty_buffer.c~tty-debug	2009-09-05 21:02:48.000000000 +0900
+++ linux-2.6-hirofumi/drivers/char/tty_buffer.c	2009-09-05 21:08:47.000000000 +0900
@@ -231,6 +231,30 @@ int tty_buffer_request_room(struct tty_s
 EXPORT_SYMBOL_GPL(tty_buffer_request_room);
 
 /**
+ *	tty_buffer_used		-	return used tty buffer size
+ *	@tty: tty structure
+ *
+ *	Return used tty buffer size.
+ */
+size_t tty_buffer_used(struct tty_struct *tty)
+{
+	size_t size;
+	int left;
+	unsigned long flags;
+
+	spin_lock_irqsave(&tty->buf.lock, flags);
+	if (tty->buf.tail)
+		left = tty->buf.tail->size - tty->buf.tail->used;
+	else
+		left = 0;
+	size = tty->buf.memory_used - left;
+	spin_unlock_irqrestore(&tty->buf.lock, flags);
+
+	return size;
+}
+EXPORT_SYMBOL_GPL(tty_buffer_used);
+
+/**
  *	tty_insert_flip_string	-	Add characters to the tty buffer
  *	@tty: tty structure
  *	@chars: characters
diff -puN include/linux/tty_flip.h~tty-debug include/linux/tty_flip.h
--- linux-2.6/include/linux/tty_flip.h~tty-debug	2009-09-05 21:06:25.000000000 +0900
+++ linux-2.6-hirofumi/include/linux/tty_flip.h	2009-09-05 21:06:39.000000000 +0900
@@ -2,6 +2,7 @@
 #define _LINUX_TTY_FLIP_H
 
 extern int tty_buffer_request_room(struct tty_struct *tty, size_t size);
+extern size_t tty_buffer_used(struct tty_struct *tty);
 extern int tty_insert_flip_string(struct tty_struct *tty, const unsigned char *chars, size_t size);
 extern int tty_insert_flip_string_flags(struct tty_struct *tty, const unsigned char *chars, const char *flags, size_t size);
 extern int tty_prepare_flip_string(struct tty_struct *tty, unsigned char **chars, size_t size);
_
--
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/