Re: [PATCH v2 20/20] tty: Remove extra wakeup from pty write() path

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 06/15/2013 10:21 AM, Peter Hurley wrote:
Acquiring the write_wait queue spin lock now accounts for the largest
slice of cpu time on the tty write path. Two factors contribute to
this situation; a overly-pessimistic line discipline write loop which
_always_ sets up a wait loop even if i/o will immediately succeed, and
on ptys, a wakeup storm from reads and writes.

Writer wakeup does not need to be performed by the pty driver.
Firstly, since the actual i/o is performed within the write, the
line discipline write loop will continue while space remains in
the flip buffers. Secondly, when space becomes avail in the
line discipline receive buffer (and thus also in the flip buffers),
the pty unthrottle re-wakes the writer (non-flow-controlled line
disciplines unconditionally unthrottle the driver when data is
received). Thus, existing in-kernel i/o is guaranteed to advance.
Finally, writer wakeup occurs at the conclusion of the line discipline
write (in tty_write_unlock()). This guarantees that any user-space write
waiters are woken to continue additional i/o.

Greg,

I thought I should let you know I'm tracking down a bug/regression
related to this patch.

In certain unusual pty/ldisc configurations, i/o fails to make
forward progress. I still stand by my commit message above, so I'm
in the process of instrumenting the i/o path so I can uncover the
cause of the failure.

I would still recommend applying this patch to tty-next, as it
resolves a much more critical bug discussed here [1].

Doing a write_wakeup() from a driver .write() method is a no-no;
recursion is possible and results in a thread stack overrun.

Regards,
Peter Hurley

[1]
https://lkml.org/lkml/2013/7/1/308

Signed-off-by: Peter Hurley <peter@xxxxxxxxxxxxxxxxxx>
---
  drivers/tty/pty.c | 4 +---
  1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c
index 0634dd9..b9bc5be 100644
--- a/drivers/tty/pty.c
+++ b/drivers/tty/pty.c
@@ -121,10 +121,8 @@ static int pty_write(struct tty_struct *tty, const unsigned char *buf, int c)
  		/* Stuff the data into the input queue of the other end */
  		c = tty_insert_flip_string(to->port, buf, c);
  		/* And shovel */
-		if (c) {
+		if (c)
  			tty_flip_buffer_push(to->port);
-			tty_wakeup(tty);
-		}
  	}
  	return c;
  }


--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux PPP]     [Linux FS]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Linmodem]     [Device Mapper]     [Linux Kernel for ARM]

  Powered by Linux