mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 0/2] Series short description
@ 2009-07-07 15:39 Alan Cox
  2009-07-07 15:39 ` [PATCH 1/2] pty: Rework the pty layer to use the normal buffering logic Alan Cox
  2009-07-07 15:39 ` [PATCH 2/2] tty: maintainers data was edited wrongly by someone Alan Cox
  0 siblings, 2 replies; 3+ messages in thread
From: Alan Cox @ 2009-07-07 15:39 UTC (permalink / raw)
  To: torvalds, linux-kernel

Next couple of fixes. The pty one is a bit more dramatic than I wanted for
an -rc release but it actually fixes the problem and at this point seems lower
risk than trying to botch up the existing code temporarily.

Having applied thar you can also then revert
a6540f731d506d9e82444cf0020e716613d4c46c

There are some USB patches to follow to fix various messes caused by sysrq
changes (serious performance regression and a kref leak) but I want those in
-next for a few days first.

---

Alan Cox (2):
      tty: maintainers data was edited wrongly by someone
      pty: Rework the pty layer to use the normal buffering logic


 MAINTAINERS        |   10 ++-
 drivers/char/pty.c |  156 ++++++++++++++++++++--------------------------------
 2 files changed, 65 insertions(+), 101 deletions(-)

-- 
My telephone support is a table


^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PATCH 1/2] pty: Rework the pty layer to use the normal buffering logic
  2009-07-07 15:39 [PATCH 0/2] Series short description Alan Cox
@ 2009-07-07 15:39 ` Alan Cox
  2009-07-07 15:39 ` [PATCH 2/2] tty: maintainers data was edited wrongly by someone Alan Cox
  1 sibling, 0 replies; 3+ messages in thread
From: Alan Cox @ 2009-07-07 15:39 UTC (permalink / raw)
  To: torvalds, linux-kernel

From: Alan Cox <alan@linux.intel.com>

This fixes the ppp problems and various other issues with call locking
caused by one side of a pty called in one locking context trying to match
another with differing rules on the other side. We also get a big slack
space to work with that means we can bury the flow control deadlock case
for any conceivable real world situation.

Signed-off-by: Alan Cox <alan@linux.intel.com>
---

 drivers/char/pty.c |  156 ++++++++++++++++++++--------------------------------
 1 files changed, 60 insertions(+), 96 deletions(-)


diff --git a/drivers/char/pty.c b/drivers/char/pty.c
index daebe1b..5448320 100644
--- a/drivers/char/pty.c
+++ b/drivers/char/pty.c
@@ -75,114 +75,88 @@ static void pty_close(struct tty_struct *tty, struct file *filp)
  */
 static void pty_unthrottle(struct tty_struct *tty)
 {
-	struct tty_struct *o_tty = tty->link;
-
-	if (!o_tty)
-		return;
-
-	tty_wakeup(o_tty);
+	tty_wakeup(tty->link);
 	set_bit(TTY_THROTTLED, &tty->flags);
 }
 
-/*
- * WSH 05/24/97: modified to
- *   (1) use space in tty->flip instead of a shared temp buffer
- *	 The flip buffers aren't being used for a pty, so there's lots
- *	 of space available.  The buffer is protected by a per-pty
- *	 semaphore that should almost never come under contention.
- *   (2) avoid redundant copying for cases where count >> receive_room
- * N.B. Calls from user space may now return an error code instead of
- * a count.
+/**
+ *	pty_space	-	report space left for writing
+ *	@to: tty we are writing into
  *
- * FIXME: Our pty_write method is called with our ldisc lock held but
- * not our partners. We can't just wait on the other one blindly without
- * risking deadlocks. At some point when everything has settled down we need
- * to look into making pty_write at least able to sleep over an ldisc change.
+ *	The tty buffers allow 64K but we sneak a peak and clip at 8K this
+ *	allows a lot of overspill room for echo and other fun messes to
+ *	be handled properly
+ */
+
+static int pty_space(struct tty_struct *to)
+{
+	int n = 8192 - to->buf.memory_used;
+	if (n < 0)
+		return 0;
+	return n;
+}
+
+/**
+ *	pty_write		-	write to a pty
+ *	@tty: the tty we write from
+ *	@buf: kernel buffer of data
+ *	@count: bytes to write
  *
- * The return on no ldisc is a bit counter intuitive but the logic works
- * like this. During an ldisc change the other end will flush its buffers. We
- * thus return the full length which is identical to the case where we had
- * proper locking and happened to queue the bytes just before the flush during
- * the ldisc change.
+ *	Our "hardware" write method. Data is coming from the ldisc which
+ *	may be in a non sleeping state. We simply throw this at the other
+ *	end of the link as if we were an IRQ handler receiving stuff for
+ *	the other side of the pty/tty pair.
  */
+
 static int pty_write(struct tty_struct *tty, const unsigned char *buf,
 								int count)
 {
 	struct tty_struct *to = tty->link;
-	struct tty_ldisc *ld;
-	int c = count;
-
-	if (!to || tty->stopped)
+	int c;
+	
+	if (tty->stopped)
 		return 0;
-	ld = tty_ldisc_ref(to);
-
-	if (ld) {
-		c = to->receive_room;
-		if (c > count)
-			c = count;
-		ld->ops->receive_buf(to, buf, NULL, c);
-		tty_ldisc_deref(ld);
+	
+	/* This isn't locked but our 8K is quite sloppy so no
+	   big deal */
+
+	c = pty_space(to);
+	if (c > count)
+		c = count;
+	if (c > 0) {
+		/* Stuff the data into the input queue of the other end */
+		c = tty_insert_flip_string(to, buf, c);
+		/* And shovel */
+		tty_flip_buffer_push(to);
+		tty_wakeup(tty);
 	}
 	return c;
 }
 
+/**
+ *	pty_write_room	-	write space
+ *	@tty: tty we are writing from
+ *
+ *	Report how many bytes the ldisc can send into the queue for
+ *	the other device.
+ */
+
 static int pty_write_room(struct tty_struct *tty)
 {
-	struct tty_struct *to = tty->link;
-
-	if (!to || tty->stopped)
-		return 0;
-
-	return to->receive_room;
+	return pty_space(tty->link);
 }
 
-/*
- *	WSH 05/24/97:  Modified for asymmetric MASTER/SLAVE behavior
- *	The chars_in_buffer() value is used by the ldisc select() function
- *	to hold off writing when chars_in_buffer > WAKEUP_CHARS (== 256).
- *	The pty driver chars_in_buffer() Master/Slave must behave differently:
- *
- *      The Master side needs to allow typed-ahead commands to accumulate
- *      while being canonicalized, so we report "our buffer" as empty until
- *	some threshold is reached, and then report the count. (Any count >
- *	WAKEUP_CHARS is regarded by select() as "full".)  To avoid deadlock
- *	the count returned must be 0 if no canonical data is available to be
- *	read. (The N_TTY ldisc.chars_in_buffer now knows this.)
+/**
+ *	pty_chars_in_buffer	-	characters currently in our tx queue
+ *	@tty: our tty
  *
- *	The Slave side passes all characters in raw mode to the Master side's
- *	buffer where they can be read immediately, so in this case we can
- *	return the true count in the buffer.
+ *	Report how much we have in the transmit queue. As everything is
+ *	instantly at the other end this is easy to implement.
  */
+
 static int pty_chars_in_buffer(struct tty_struct *tty)
 {
-	struct tty_struct *to = tty->link;
-	struct tty_ldisc *ld;
-	int count = 0;
-
-	/* We should get the line discipline lock for "tty->link" */
-	if (!to)
-		return 0;
-	/* We cannot take a sleeping reference here without deadlocking with
-	   an ldisc change - but it doesn't really matter */
-	ld = tty_ldisc_ref(to);
-	if (ld == NULL)
-		return 0;
-
-	/* The ldisc must report 0 if no characters available to be read */
-	if (ld->ops->chars_in_buffer)
-		count = ld->ops->chars_in_buffer(to);
-
-	tty_ldisc_deref(ld);
-
-	if (tty->driver->subtype == PTY_TYPE_SLAVE)
-		return count;
-
-	/* Master side driver ... if the other side's read buffer is less than
-	 * half full, return 0 to allow writers to proceed; otherwise return
-	 * the count.  This leaves a comfortable margin to avoid overflow,
-	 * and still allows half a buffer's worth of typed-ahead commands.
-	 */
-	return (count < N_TTY_BUF_SIZE/2) ? 0 : count;
+	return 0;
 }
 
 /* Set the lock flag on a pty */
@@ -202,20 +176,10 @@ static void pty_flush_buffer(struct tty_struct *tty)
 {
 	struct tty_struct *to = tty->link;
 	unsigned long flags;
-	struct tty_ldisc *ld;
 
 	if (!to)
 		return;
-	ld = tty_ldisc_ref(to);
-
-	/* The other end is changing discipline */
-	if (!ld)
-		return;
-
-	if (ld->ops->flush_buffer)
-		to->ldisc->ops->flush_buffer(to);
-	tty_ldisc_deref(ld);
-
+	/* tty_buffer_flush(to); FIXME */
 	if (to->packet) {
 		spin_lock_irqsave(&tty->ctrl_lock, flags);
 		tty->ctrl_status |= TIOCPKT_FLUSHWRITE;


^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PATCH 2/2] tty: maintainers data was edited wrongly by someone
  2009-07-07 15:39 [PATCH 0/2] Series short description Alan Cox
  2009-07-07 15:39 ` [PATCH 1/2] pty: Rework the pty layer to use the normal buffering logic Alan Cox
@ 2009-07-07 15:39 ` Alan Cox
  1 sibling, 0 replies; 3+ messages in thread
From: Alan Cox @ 2009-07-07 15:39 UTC (permalink / raw)
  To: torvalds, linux-kernel

From: Alan Cox <alan@linux.intel.com>

Move tty entries to tty not trivial. Noted by David Brownell.

Signed-off-by: Alan Cox <alan@linux.intel.com>
---

 MAINTAINERS |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)


diff --git a/MAINTAINERS b/MAINTAINERS
index 381190c..e4b1a3d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5803,17 +5803,17 @@ P:	Jiri Kosina
 M:	trivial@kernel.org
 T:	git git://git.kernel.org/pub/scm/linux/kernel/git/jikos/trivial.git
 S:	Maintained
-F:	drivers/char/tty_*
-F:	drivers/serial/serial_core.c
-F:	include/linux/serial_core.h
-F:	include/linux/serial.h
-F:	include/linux/tty.h
 
 TTY LAYER
 P:	Alan Cox
 M:	alan@lxorguk.ukuu.org.uk
 S:	Maintained
 T:	stgit http://zeniv.linux.org.uk/~alan/ttydev/
+F:	drivers/char/tty_*
+F:	drivers/serial/serial_core.c
+F:	include/linux/serial_core.h
+F:	include/linux/serial.h
+F:	include/linux/tty.h
 
 TULIP NETWORK DRIVERS
 P:	Grant Grundler


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2009-07-07 15:39 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-07-07 15:39 [PATCH 0/2] Series short description Alan Cox
2009-07-07 15:39 ` [PATCH 1/2] pty: Rework the pty layer to use the normal buffering logic Alan Cox
2009-07-07 15:39 ` [PATCH 2/2] tty: maintainers data was edited wrongly by someone Alan Cox

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®