* [PATCH tty-next 0/7] n_tty cleanup + trace additions
@ 2013-11-22 15:59 Peter Hurley
2013-11-22 15:59 ` [PATCH tty-next 1/7] n_tty: Merge .receive_buf() flavors Peter Hurley
` (7 more replies)
0 siblings, 8 replies; 27+ messages in thread
From: Peter Hurley @ 2013-11-22 15:59 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-kernel, linux-serial, Peter Hurley
Greg,
This patchset contains no bug fixes only misc cleanup for the
N_TTY line discipline.
Patches 1-4 cleans up code duplication remnants from 3.12's changes.
Patch 5 simplifies the snarled logic of how much data is considered
'input available'. I was getting tired of hand checking that logic
every time I touched minimum_to_wake.
Patch 6 reduces unnecessary wake ups.
Patch 7 extends the trace usage to aid in identifying and fixing
input flow-control bugs (this was used to fix the readline() bug).
I also expect to use it to reduce input processing restarts. It's
use is disabled by default and only enabled with the file-local
define N_TTY_TRACE (similar to TTY_DEBUG_HANGUP in tty_io.c).
Regards,
Peter Hurley (7):
n_tty: Merge .receive_buf() flavors
n_tty: Un-inline slow-path n_tty_receive_char()
n_tty: Un-inline slow-path n_tty_receive_char_closing()
n_tty: Refactor PARMRK doubling checks
n_tty: Refactor input_available_p() by call site
n_tty: Only perform wakeups for waiters
n_tty: trace input/read flow control
drivers/tty/n_tty.c | 159 ++++++++++++++++++++++++++++++++--------------------
1 file changed, 97 insertions(+), 62 deletions(-)
--
1.8.1.2
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH tty-next 1/7] n_tty: Merge .receive_buf() flavors
2013-11-22 15:59 [PATCH tty-next 0/7] n_tty cleanup + trace additions Peter Hurley
@ 2013-11-22 15:59 ` Peter Hurley
2013-11-22 15:59 ` [PATCH tty-next 2/7] n_tty: Un-inline slow-path n_tty_receive_char() Peter Hurley
` (6 subsequent siblings)
7 siblings, 0 replies; 27+ messages in thread
From: Peter Hurley @ 2013-11-22 15:59 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-kernel, linux-serial, Peter Hurley
N_TTY's direct and flow-controlled flavors of the .receive_buf()
method are nearly identical; fold together.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
drivers/tty/n_tty.c | 43 ++++++++++++++++---------------------------
1 file changed, 16 insertions(+), 27 deletions(-)
diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index 5f1c5f3..3eb54c7 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -1675,32 +1675,9 @@ static void __receive_buf(struct tty_struct *tty, const unsigned char *cp,
}
}
-static void n_tty_receive_buf(struct tty_struct *tty, const unsigned char *cp,
- char *fp, int count)
-{
- int room, n;
-
- down_read(&tty->termios_rwsem);
-
- while (1) {
- room = receive_room(tty);
- n = min(count, room);
- if (!n)
- break;
- __receive_buf(tty, cp, fp, n);
- cp += n;
- if (fp)
- fp += n;
- count -= n;
- }
-
- tty->receive_room = room;
- n_tty_check_throttle(tty);
- up_read(&tty->termios_rwsem);
-}
-
-static int n_tty_receive_buf2(struct tty_struct *tty, const unsigned char *cp,
- char *fp, int count)
+static int
+n_tty_receive_buf_common(struct tty_struct *tty, const unsigned char *cp,
+ char *fp, int count, int flow)
{
struct n_tty_data *ldata = tty->disc_data;
int room, n, rcvd = 0;
@@ -1711,7 +1688,7 @@ static int n_tty_receive_buf2(struct tty_struct *tty, const unsigned char *cp,
room = receive_room(tty);
n = min(count, room);
if (!n) {
- if (!room)
+ if (flow && !room)
ldata->no_room = 1;
break;
}
@@ -1730,6 +1707,18 @@ static int n_tty_receive_buf2(struct tty_struct *tty, const unsigned char *cp,
return rcvd;
}
+static void n_tty_receive_buf(struct tty_struct *tty, const unsigned char *cp,
+ char *fp, int count)
+{
+ n_tty_receive_buf_common(tty, cp, fp, count, 0);
+}
+
+static int n_tty_receive_buf2(struct tty_struct *tty, const unsigned char *cp,
+ char *fp, int count)
+{
+ return n_tty_receive_buf_common(tty, cp, fp, count, 1);
+}
+
int is_ignored(int sig)
{
return (sigismember(¤t->blocked, sig) ||
--
1.8.1.2
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH tty-next 2/7] n_tty: Un-inline slow-path n_tty_receive_char()
2013-11-22 15:59 [PATCH tty-next 0/7] n_tty cleanup + trace additions Peter Hurley
2013-11-22 15:59 ` [PATCH tty-next 1/7] n_tty: Merge .receive_buf() flavors Peter Hurley
@ 2013-11-22 15:59 ` Peter Hurley
2013-11-22 15:59 ` [PATCH tty-next 3/7] n_tty: Un-inline slow-path n_tty_receive_char_closing() Peter Hurley
` (5 subsequent siblings)
7 siblings, 0 replies; 27+ messages in thread
From: Peter Hurley @ 2013-11-22 15:59 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-kernel, linux-serial, Peter Hurley
Commit e60d27c4d8b33ba20896b76b6558f061bc6460ff,
n_tty: Factor LNEXT processing from per-char i/o path,
mistakenly inlined the non-inline alias, n_tty_receive_char(),
for the inline function, n_tty_receive_char_inline().
As n_tty_receive_char() is intended for slow-path char
processing only, un-inline it.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
drivers/tty/n_tty.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index 3eb54c7..3808d3d 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -1418,7 +1418,7 @@ n_tty_receive_char_inline(struct tty_struct *tty, unsigned char c)
put_tty_queue(c, ldata);
}
-static inline void n_tty_receive_char(struct tty_struct *tty, unsigned char c)
+static void n_tty_receive_char(struct tty_struct *tty, unsigned char c)
{
n_tty_receive_char_inline(tty, c);
}
--
1.8.1.2
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH tty-next 3/7] n_tty: Un-inline slow-path n_tty_receive_char_closing()
2013-11-22 15:59 [PATCH tty-next 0/7] n_tty cleanup + trace additions Peter Hurley
2013-11-22 15:59 ` [PATCH tty-next 1/7] n_tty: Merge .receive_buf() flavors Peter Hurley
2013-11-22 15:59 ` [PATCH tty-next 2/7] n_tty: Un-inline slow-path n_tty_receive_char() Peter Hurley
@ 2013-11-22 15:59 ` Peter Hurley
2013-11-22 15:59 ` [PATCH tty-next 4/7] n_tty: Refactor PARMRK doubling checks Peter Hurley
` (4 subsequent siblings)
7 siblings, 0 replies; 27+ messages in thread
From: Peter Hurley @ 2013-11-22 15:59 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-kernel, linux-serial, Peter Hurley
Although n_tty_receive_char_closing() only has one call-site,
let the compiler inline instead.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
drivers/tty/n_tty.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index 3808d3d..61edeaf 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -1443,8 +1443,7 @@ n_tty_receive_char_fast(struct tty_struct *tty, unsigned char c)
put_tty_queue(c, ldata);
}
-static inline void
-n_tty_receive_char_closing(struct tty_struct *tty, unsigned char c)
+static void n_tty_receive_char_closing(struct tty_struct *tty, unsigned char c)
{
if (I_ISTRIP(tty))
c &= 0x7f;
--
1.8.1.2
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH tty-next 4/7] n_tty: Refactor PARMRK doubling checks
2013-11-22 15:59 [PATCH tty-next 0/7] n_tty cleanup + trace additions Peter Hurley
` (2 preceding siblings ...)
2013-11-22 15:59 ` [PATCH tty-next 3/7] n_tty: Un-inline slow-path n_tty_receive_char_closing() Peter Hurley
@ 2013-11-22 15:59 ` Peter Hurley
2013-11-22 15:59 ` [PATCH tty-next 5/7] n_tty: Refactor input_available_p() by call site Peter Hurley
` (3 subsequent siblings)
7 siblings, 0 replies; 27+ messages in thread
From: Peter Hurley @ 2013-11-22 15:59 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-kernel, linux-serial, Peter Hurley
Perform PARMRK doubling checks explicitly; remove ternary idiom
and local variable.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
drivers/tty/n_tty.c | 14 +++++---------
1 file changed, 5 insertions(+), 9 deletions(-)
diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index 61edeaf..9298b68 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -1258,7 +1258,6 @@ static int
n_tty_receive_char_special(struct tty_struct *tty, unsigned char c)
{
struct n_tty_data *ldata = tty->disc_data;
- int parmrk;
if (I_IXON(tty)) {
if (c == START_CHAR(tty)) {
@@ -1343,8 +1342,6 @@ n_tty_receive_char_special(struct tty_struct *tty, unsigned char c)
}
if ((c == EOL_CHAR(tty)) ||
(c == EOL2_CHAR(tty) && L_IEXTEN(tty))) {
- parmrk = (c == (unsigned char) '\377' && I_PARMRK(tty))
- ? 1 : 0;
/*
* XXX are EOL_CHAR and EOL2_CHAR echoed?!?
*/
@@ -1359,7 +1356,7 @@ n_tty_receive_char_special(struct tty_struct *tty, unsigned char c)
* XXX does PARMRK doubling happen for
* EOL_CHAR and EOL2_CHAR?
*/
- if (parmrk)
+ if (c == (unsigned char) '\377' && I_PARMRK(tty))
put_tty_queue(c, ldata);
handle_newline:
@@ -1373,7 +1370,6 @@ handle_newline:
}
}
- parmrk = (c == (unsigned char) '\377' && I_PARMRK(tty)) ? 1 : 0;
if (L_ECHO(tty)) {
finish_erasing(ldata);
if (c == '\n')
@@ -1387,7 +1383,8 @@ handle_newline:
commit_echoes(tty);
}
- if (parmrk)
+ /* PARMRK doubling check */
+ if (c == (unsigned char) '\377' && I_PARMRK(tty))
put_tty_queue(c, ldata);
put_tty_queue(c, ldata);
@@ -1398,7 +1395,6 @@ static inline void
n_tty_receive_char_inline(struct tty_struct *tty, unsigned char c)
{
struct n_tty_data *ldata = tty->disc_data;
- int parmrk;
if (tty->stopped && !tty->flow_stopped && I_IXON(tty) && I_IXANY(tty)) {
start_tty(tty);
@@ -1412,8 +1408,8 @@ n_tty_receive_char_inline(struct tty_struct *tty, unsigned char c)
echo_char(c, tty);
commit_echoes(tty);
}
- parmrk = (c == (unsigned char) '\377' && I_PARMRK(tty)) ? 1 : 0;
- if (parmrk)
+ /* PARMRK doubling check */
+ if (c == (unsigned char) '\377' && I_PARMRK(tty))
put_tty_queue(c, ldata);
put_tty_queue(c, ldata);
}
--
1.8.1.2
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH tty-next 5/7] n_tty: Refactor input_available_p() by call site
2013-11-22 15:59 [PATCH tty-next 0/7] n_tty cleanup + trace additions Peter Hurley
` (3 preceding siblings ...)
2013-11-22 15:59 ` [PATCH tty-next 4/7] n_tty: Refactor PARMRK doubling checks Peter Hurley
@ 2013-11-22 15:59 ` Peter Hurley
2013-11-24 0:22 ` One Thousand Gnomes
2013-11-24 0:26 ` One Thousand Gnomes
2013-11-22 15:59 ` [PATCH tty-next 6/7] n_tty: Only perform wakeups for waiters Peter Hurley
` (2 subsequent siblings)
7 siblings, 2 replies; 27+ messages in thread
From: Peter Hurley @ 2013-11-22 15:59 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-kernel, linux-serial, Peter Hurley
Distinguish if caller is n_tty_poll() or n_tty_read(), and
set the read/wakeup threshold accordingly.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
drivers/tty/n_tty.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index 9298b68..8f2356e 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -1878,14 +1878,15 @@ static int n_tty_open(struct tty_struct *tty)
return 0;
}
-static inline int input_available_p(struct tty_struct *tty, int amt)
+static inline int input_available_p(struct tty_struct *tty, int poll)
{
struct n_tty_data *ldata = tty->disc_data;
+ int amt = poll && !TIME_CHAR(tty) ? MIN_CHAR(tty) : 1;
if (ldata->icanon && !L_EXTPROC(tty)) {
if (ldata->canon_head != ldata->read_tail)
return 1;
- } else if (read_cnt(ldata) >= (amt ? amt : 1))
+ } else if (read_cnt(ldata) >= amt)
return 1;
return 0;
@@ -2384,7 +2385,7 @@ static unsigned int n_tty_poll(struct tty_struct *tty, struct file *file,
poll_wait(file, &tty->read_wait, wait);
poll_wait(file, &tty->write_wait, wait);
- if (input_available_p(tty, TIME_CHAR(tty) ? 0 : MIN_CHAR(tty)))
+ if (input_available_p(tty, 1))
mask |= POLLIN | POLLRDNORM;
else if (ldata->icanon)
n_tty_set_room(tty);
--
1.8.1.2
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH tty-next 6/7] n_tty: Only perform wakeups for waiters
2013-11-22 15:59 [PATCH tty-next 0/7] n_tty cleanup + trace additions Peter Hurley
` (4 preceding siblings ...)
2013-11-22 15:59 ` [PATCH tty-next 5/7] n_tty: Refactor input_available_p() by call site Peter Hurley
@ 2013-11-22 15:59 ` Peter Hurley
2013-11-24 0:23 ` One Thousand Gnomes
2013-11-22 15:59 ` [PATCH tty-next 7/7] n_tty: trace input/read flow control Peter Hurley
2013-12-02 19:24 ` [PATCH v2 tty-next 0/8] n_tty cleanup + trace additions Peter Hurley
7 siblings, 1 reply; 27+ messages in thread
From: Peter Hurley @ 2013-11-22 15:59 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-kernel, linux-serial, Peter Hurley
Only wakeup the _waiting_ reader, polls and/or writer(s).
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
drivers/tty/n_tty.c | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index 8f2356e..aae28a6 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -275,7 +275,8 @@ static void n_tty_check_unthrottle(struct tty_struct *tty)
return;
n_tty_set_room(tty);
n_tty_write_wakeup(tty->link);
- wake_up_interruptible_poll(&tty->link->write_wait, POLLOUT);
+ if (waitqueue_active(&tty->link->write_wait))
+ wake_up_interruptible_poll(&tty->link->write_wait, POLLOUT);
return;
}
@@ -350,7 +351,8 @@ static void n_tty_packet_mode_flush(struct tty_struct *tty)
spin_lock_irqsave(&tty->ctrl_lock, flags);
if (tty->link->packet) {
tty->ctrl_status |= TIOCPKT_FLUSHREAD;
- wake_up_interruptible(&tty->link->read_wait);
+ if (waitqueue_active(&tty->link->read_wait))
+ wake_up_interruptible(&tty->link->read_wait);
}
spin_unlock_irqrestore(&tty->ctrl_lock, flags);
}
@@ -1156,7 +1158,8 @@ static void n_tty_receive_break(struct tty_struct *tty)
put_tty_queue('\0', ldata);
}
put_tty_queue('\0', ldata);
- wake_up_interruptible(&tty->read_wait);
+ if (waitqueue_active(&tty->read_wait))
+ wake_up_interruptible(&tty->read_wait);
}
/**
@@ -1214,7 +1217,8 @@ static void n_tty_receive_parity_error(struct tty_struct *tty, unsigned char c)
put_tty_queue('\0', ldata);
else
put_tty_queue(c, ldata);
- wake_up_interruptible(&tty->read_wait);
+ if (waitqueue_active(&tty->read_wait))
+ wake_up_interruptible(&tty->read_wait);
}
static void
@@ -1808,8 +1812,10 @@ static void n_tty_set_termios(struct tty_struct *tty, struct ktermios *old)
start_tty(tty);
/* The termios change make the tty ready for I/O */
- wake_up_interruptible(&tty->write_wait);
- wake_up_interruptible(&tty->read_wait);
+ if (waitqueue_active(&tty->write_wait))
+ wake_up_interruptible(&tty->write_wait);
+ if (waitqueue_active(&tty->read_wait))
+ wake_up_interruptible(&tty->read_wait);
}
/**
--
1.8.1.2
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH tty-next 7/7] n_tty: trace input/read flow control
2013-11-22 15:59 [PATCH tty-next 0/7] n_tty cleanup + trace additions Peter Hurley
` (5 preceding siblings ...)
2013-11-22 15:59 ` [PATCH tty-next 6/7] n_tty: Only perform wakeups for waiters Peter Hurley
@ 2013-11-22 15:59 ` Peter Hurley
2013-11-24 0:25 ` One Thousand Gnomes
2013-12-02 19:24 ` [PATCH v2 tty-next 0/8] n_tty cleanup + trace additions Peter Hurley
7 siblings, 1 reply; 27+ messages in thread
From: Peter Hurley @ 2013-11-22 15:59 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-kernel, linux-serial, Peter Hurley
Instrument .receive_buf() and read() paths with trace_printk's
to aid in debugging flow control changes.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
drivers/tty/n_tty.c | 76 ++++++++++++++++++++++++++++++++++++++++++-----------
1 file changed, 60 insertions(+), 16 deletions(-)
diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index aae28a6..68fa3a4 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -83,9 +83,14 @@
#undef N_TTY_TRACE
#ifdef N_TTY_TRACE
-# define n_tty_trace(f, args...) trace_printk(f, ##args)
+# define n_tty_trace(tty, f, args...) \
+ do { \
+ char buf[64]; \
+ trace_printk("[%s] " f, tty_name(tty, buf), ##args); \
+ } while (0)
#else
-# define n_tty_trace(f, args...)
+# define n_tty_trace(tty, f, args...) \
+ do { } while (0)
#endif
struct n_tty_data {
@@ -179,8 +184,13 @@ static int receive_room(struct tty_struct *tty)
* that erase characters will be handled. Other excess
* characters will be beeped.
*/
- if (left <= 0)
+ if (left <= 0) {
left = ldata->icanon && ldata->canon_head == ldata->read_tail;
+ if (left) {
+ n_tty_trace(tty, "overflow, head:%zu tail:%zu\n",
+ ldata->read_head, ldata->read_tail);
+ }
+ }
return left;
}
@@ -200,11 +210,14 @@ static int receive_room(struct tty_struct *tty)
static void n_tty_set_room(struct tty_struct *tty)
{
struct n_tty_data *ldata = tty->disc_data;
+ int avail;
/* Did this open up the receive buffer? We may need to flip */
- if (unlikely(ldata->no_room) && receive_room(tty)) {
+ if (unlikely(ldata->no_room) && (avail = receive_room(tty))) {
ldata->no_room = 0;
+ n_tty_trace(tty, "restart worker (avail = %d)\n", avail);
+
WARN_RATELIMIT(tty->port->itty == NULL,
"scheduling with invalid itty\n");
/* see if ldisc has been killed - if so, this means that
@@ -240,6 +253,8 @@ static ssize_t chars_in_buffer(struct tty_struct *tty)
static void n_tty_write_wakeup(struct tty_struct *tty)
{
+ n_tty_trace(tty, "\n");
+
if (tty->fasync && test_and_clear_bit(TTY_DO_WRITE_WAKEUP, &tty->flags))
kill_fasync(&tty->fasync, SIGIO, POLL_OUT);
}
@@ -254,13 +269,16 @@ static void n_tty_check_throttle(struct tty_struct *tty)
* canonical mode and don't have a newline yet!
*/
while (1) {
- int throttled;
+ int throttled, avail;
tty_set_flow_change(tty, TTY_THROTTLE_SAFE);
- if (receive_room(tty) >= TTY_THRESHOLD_THROTTLE)
+ avail = receive_room(tty);
+ if (avail >= TTY_THRESHOLD_THROTTLE)
break;
throttled = tty_throttle_safe(tty);
- if (!throttled)
+ if (!throttled) {
+ n_tty_trace(tty, "throttled (avail = %d)/n", avail);
break;
+ }
}
__tty_set_flow_change(tty, 0);
}
@@ -289,16 +307,19 @@ static void n_tty_check_unthrottle(struct tty_struct *tty)
*/
while (1) {
- int unthrottled;
+ int unthrottled, used;
tty_set_flow_change(tty, TTY_UNTHROTTLE_SAFE);
- if (chars_in_buffer(tty) > TTY_THRESHOLD_UNTHROTTLE)
+ used = chars_in_buffer(tty);
+ if (used > TTY_THRESHOLD_UNTHROTTLE)
break;
if (!tty->count)
break;
n_tty_set_room(tty);
unthrottled = tty_unthrottle_safe(tty);
- if (!unthrottled)
+ if (!unthrottled) {
+ n_tty_trace(tty, "unthrottled (used = %d)\n", used);
break;
+ }
}
__tty_set_flow_change(tty, 0);
}
@@ -374,6 +395,9 @@ static void n_tty_packet_mode_flush(struct tty_struct *tty)
static void n_tty_flush_buffer(struct tty_struct *tty)
{
down_write(&tty->termios_rwsem);
+
+ n_tty_trace(tty, "\n");
+
reset_buffer_flags(tty->disc_data);
n_tty_set_room(tty);
@@ -770,6 +794,10 @@ static size_t __process_echoes(struct tty_struct *tty)
* of echo overrun before the next commit), then discard enough
* data at the tail to prevent a subsequent overrun */
while (ldata->echo_commit - tail >= ECHO_DISCARD_WATERMARK) {
+
+ n_tty_trace(tty, "discard echoes (%zu - %zu > %d)\n",
+ ldata->echo_commit, tail, ECHO_DISCARD_WATERMARK);
+
if (echo_buf(ldata, tail) == ECHO_OP_START) {
if (echo_buf(ldata, tail + 1) == ECHO_OP_ERASE_TAB)
tail += 3;
@@ -1367,6 +1395,7 @@ handle_newline:
set_bit(ldata->read_head & (N_TTY_BUF_SIZE - 1), ldata->read_flags);
put_tty_queue(c, ldata);
ldata->canon_head = ldata->read_head;
+ n_tty_trace(tty, "canon_head:%zu\n", ldata->canon_head);
kill_fasync(&tty->fasync, SIGIO, POLL_IN);
if (waitqueue_active(&tty->read_wait))
wake_up_interruptible(&tty->read_wait);
@@ -1668,6 +1697,7 @@ static void __receive_buf(struct tty_struct *tty, const unsigned char *cp,
if ((!ldata->icanon && (read_cnt(ldata) >= ldata->minimum_to_wake)) ||
L_EXTPROC(tty)) {
+ n_tty_trace(tty, "reader wakeup, count:%zd\n", read_cnt(ldata));
kill_fasync(&tty->fasync, SIGIO, POLL_IN);
if (waitqueue_active(&tty->read_wait))
wake_up_interruptible(&tty->read_wait);
@@ -1687,6 +1717,8 @@ n_tty_receive_buf_common(struct tty_struct *tty, const unsigned char *cp,
room = receive_room(tty);
n = min(count, room);
if (!n) {
+ if (!room)
+ n_tty_trace(tty, "buffer full\n");
if (flow && !room)
ldata->no_room = 1;
break;
@@ -1703,6 +1735,8 @@ n_tty_receive_buf_common(struct tty_struct *tty, const unsigned char *cp,
n_tty_check_throttle(tty);
up_read(&tty->termios_rwsem);
+ n_tty_trace(tty, "rcvd:%d\n", rcvd);
+
return rcvd;
}
@@ -1811,6 +1845,8 @@ static void n_tty_set_termios(struct tty_struct *tty, struct ktermios *old)
if (!I_IXON(tty) && old && (old->c_iflag & IXON) && !tty->flow_stopped)
start_tty(tty);
+ n_tty_trace(tty, "canon:%d\n", ldata->icanon);
+
/* The termios change make the tty ready for I/O */
if (waitqueue_active(&tty->write_wait))
wake_up_interruptible(&tty->write_wait);
@@ -1890,10 +1926,14 @@ static inline int input_available_p(struct tty_struct *tty, int poll)
int amt = poll && !TIME_CHAR(tty) ? MIN_CHAR(tty) : 1;
if (ldata->icanon && !L_EXTPROC(tty)) {
- if (ldata->canon_head != ldata->read_tail)
+ if (ldata->canon_head != ldata->read_tail) {
+ n_tty_trace(tty, "line avail\n");
return 1;
- } else if (read_cnt(ldata) >= amt)
+ }
+ } else if (read_cnt(ldata) >= amt) {
+ n_tty_trace(tty, "avail:%zu amt:%d\n", read_cnt(ldata), amt);
return 1;
+ }
return 0;
}
@@ -1984,8 +2024,8 @@ static int canon_copy_from_read_buf(struct tty_struct *tty,
tail = ldata->read_tail & (N_TTY_BUF_SIZE - 1);
size = min_t(size_t, tail + n, N_TTY_BUF_SIZE);
- n_tty_trace("%s: nr:%zu tail:%zu n:%zu size:%zu\n",
- __func__, *nr, tail, n, size);
+ n_tty_trace(tty, "nr:%zu tail:%zu n:%zu size:%zu\n",
+ *nr, tail, n, size);
eol = find_next_bit(ldata->read_flags, size, tail);
more = n - (size - tail);
@@ -2009,8 +2049,8 @@ static int canon_copy_from_read_buf(struct tty_struct *tty,
eof_push = !n && ldata->read_tail != ldata->line_start;
}
- n_tty_trace("%s: eol:%zu found:%d n:%zu c:%zu size:%zu more:%zu\n",
- __func__, eol, found, n, c, size, more);
+ n_tty_trace(tty, "eol:%zu found:%d n:%zu c:%zu size:%zu more:%zu\n",
+ eol, found, n, c, size, more);
if (n > size) {
ret = copy_to_user(*b, read_buf_addr(ldata, tail), size);
@@ -2202,6 +2242,8 @@ static ssize_t n_tty_read(struct tty_struct *tty, struct file *file,
n_tty_set_room(tty);
up_read(&tty->termios_rwsem);
+ n_tty_trace(tty, "blocking\n");
+
timeout = schedule_timeout(timeout);
down_read(&tty->termios_rwsem);
@@ -2259,6 +2301,8 @@ static ssize_t n_tty_read(struct tty_struct *tty, struct file *file,
if (b - buf)
retval = b - buf;
+ n_tty_trace(tty, "ret:%zd\n", retval);
+
return retval;
}
--
1.8.1.2
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH tty-next 5/7] n_tty: Refactor input_available_p() by call site
2013-11-22 15:59 ` [PATCH tty-next 5/7] n_tty: Refactor input_available_p() by call site Peter Hurley
@ 2013-11-24 0:22 ` One Thousand Gnomes
2013-11-24 0:26 ` One Thousand Gnomes
1 sibling, 0 replies; 27+ messages in thread
From: One Thousand Gnomes @ 2013-11-24 0:22 UTC (permalink / raw)
To: Peter Hurley; +Cc: Greg Kroah-Hartman, Jiri Slaby, linux-kernel, linux-serial
On Fri, 22 Nov 2013 10:59:23 -0500
Peter Hurley <peter@hurleysoftware.com> wrote:
> Distinguish if caller is n_tty_poll() or n_tty_read(), and
> set the read/wakeup threshold accordingly.
That looks very wrong to me. The poll() and read() wakeup should always
match. In addition if MIN_CHAR is set without a timeout then it should not
wake up until MIN_CHAR characters are present. The MIN_CHAR feature is
used by various (these days obscure) protocols to optimise block transfer
rates.
Changing this is also a userspace visible API change.
So NAK.
The underlying intention of the system (and SYS5.4) is that you can do
MIN_CHAR = 64
alarm(some_timeout)
while(1) {
poll();
events
read(chunksized blocks)
}
while it's hardly as critical with modern fast hardware its still the API
and necessary to avoid SWS type behaviours in the queue turning it from
block/syscall to byte I/O.
Alan
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH tty-next 6/7] n_tty: Only perform wakeups for waiters
2013-11-22 15:59 ` [PATCH tty-next 6/7] n_tty: Only perform wakeups for waiters Peter Hurley
@ 2013-11-24 0:23 ` One Thousand Gnomes
2013-11-24 2:29 ` Peter Hurley
0 siblings, 1 reply; 27+ messages in thread
From: One Thousand Gnomes @ 2013-11-24 0:23 UTC (permalink / raw)
To: Peter Hurley; +Cc: Greg Kroah-Hartman, Jiri Slaby, linux-kernel, linux-serial
On Fri, 22 Nov 2013 10:59:24 -0500
Peter Hurley <peter@hurleysoftware.com> wrote:
> Only wakeup the _waiting_ reader, polls and/or writer(s).
>
> Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
> ---
> drivers/tty/n_tty.c | 18 ++++++++++++------
> 1 file changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
> index 8f2356e..aae28a6 100644
> --- a/drivers/tty/n_tty.c
> +++ b/drivers/tty/n_tty.c
> @@ -275,7 +275,8 @@ static void n_tty_check_unthrottle(struct tty_struct *tty)
> return;
> n_tty_set_room(tty);
> n_tty_write_wakeup(tty->link);
> - wake_up_interruptible_poll(&tty->link->write_wait, POLLOUT);
> + if (waitqueue_active(&tty->link->write_wait))
> + wake_up_interruptible_poll(&tty->link->write_wait, POLLOUT);
Does this actually microbenchmark faster ?
Alan
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH tty-next 7/7] n_tty: trace input/read flow control
2013-11-22 15:59 ` [PATCH tty-next 7/7] n_tty: trace input/read flow control Peter Hurley
@ 2013-11-24 0:25 ` One Thousand Gnomes
2013-11-24 2:38 ` Peter Hurley
0 siblings, 1 reply; 27+ messages in thread
From: One Thousand Gnomes @ 2013-11-24 0:25 UTC (permalink / raw)
To: Peter Hurley; +Cc: Greg Kroah-Hartman, Jiri Slaby, linux-kernel, linux-serial
On Fri, 22 Nov 2013 10:59:25 -0500
Peter Hurley <peter@hurleysoftware.com> wrote:
> Instrument .receive_buf() and read() paths with trace_printk's
> to aid in debugging flow control changes.
tty devices have a device, we have a dev_dbg() layer. The old tty trace
predates this but there isn't really any excuse for not using it now that
I can see ?
Alan
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH tty-next 5/7] n_tty: Refactor input_available_p() by call site
2013-11-22 15:59 ` [PATCH tty-next 5/7] n_tty: Refactor input_available_p() by call site Peter Hurley
2013-11-24 0:22 ` One Thousand Gnomes
@ 2013-11-24 0:26 ` One Thousand Gnomes
2013-11-24 2:01 ` Peter Hurley
1 sibling, 1 reply; 27+ messages in thread
From: One Thousand Gnomes @ 2013-11-24 0:26 UTC (permalink / raw)
To: Peter Hurley; +Cc: Greg Kroah-Hartman, Jiri Slaby, linux-kernel, linux-serial
On Fri, 22 Nov 2013 10:59:23 -0500
Peter Hurley <peter@hurleysoftware.com> wrote:
> Distinguish if caller is n_tty_poll() or n_tty_read(), and
> set the read/wakeup threshold accordingly.
>
> Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Doh ignore previous.. yes that patch is right. I should pay attention to
0/7 8)
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH tty-next 5/7] n_tty: Refactor input_available_p() by call site
2013-11-24 0:26 ` One Thousand Gnomes
@ 2013-11-24 2:01 ` Peter Hurley
2013-11-24 16:18 ` One Thousand Gnomes
0 siblings, 1 reply; 27+ messages in thread
From: Peter Hurley @ 2013-11-24 2:01 UTC (permalink / raw)
To: One Thousand Gnomes
Cc: Greg Kroah-Hartman, Jiri Slaby, linux-kernel, linux-serial
On 11/23/2013 07:26 PM, One Thousand Gnomes wrote:
> On Fri, 22 Nov 2013 10:59:23 -0500
> Peter Hurley <peter@hurleysoftware.com> wrote:
>
>> Distinguish if caller is n_tty_poll() or n_tty_read(), and
>> set the read/wakeup threshold accordingly.
>>
>> Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
>
> Doh ignore previous.. yes that patch is right. I should pay attention to
> 0/7 8)
Maybe I should add more to the commit message from the cover letter
description ?
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH tty-next 6/7] n_tty: Only perform wakeups for waiters
2013-11-24 0:23 ` One Thousand Gnomes
@ 2013-11-24 2:29 ` Peter Hurley
0 siblings, 0 replies; 27+ messages in thread
From: Peter Hurley @ 2013-11-24 2:29 UTC (permalink / raw)
To: One Thousand Gnomes
Cc: Greg Kroah-Hartman, Jiri Slaby, linux-kernel, linux-serial
On 11/23/2013 07:23 PM, One Thousand Gnomes wrote:
> On Fri, 22 Nov 2013 10:59:24 -0500
> Peter Hurley <peter@hurleysoftware.com> wrote:
>
>> Only wakeup the _waiting_ reader, polls and/or writer(s).
>>
>> Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
>> ---
>> drivers/tty/n_tty.c | 18 ++++++++++++------
>> 1 file changed, 12 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
>> index 8f2356e..aae28a6 100644
>> --- a/drivers/tty/n_tty.c
>> +++ b/drivers/tty/n_tty.c
>> @@ -275,7 +275,8 @@ static void n_tty_check_unthrottle(struct tty_struct *tty)
>> return;
>> n_tty_set_room(tty);
>> n_tty_write_wakeup(tty->link);
>> - wake_up_interruptible_poll(&tty->link->write_wait, POLLOUT);
>> + if (waitqueue_active(&tty->link->write_wait))
>> + wake_up_interruptible_poll(&tty->link->write_wait, POLLOUT);
>
> Does this actually microbenchmark faster ?
Getting on and off the write_wait queue is actually pretty expensive for
the "other" pty (the writer), and the unnecessary wakeup from the reader
doesn't help.
The other chunks are gratuitous.
Regards,
Peter Hurley
PS - This came up because there is some worst-case behavior that I'm looking
into fixing. When the userspace reader is very far behind (say because it's
reading char-by-char), it doesn't make sense to keep restarting the input
processing worker.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH tty-next 7/7] n_tty: trace input/read flow control
2013-11-24 0:25 ` One Thousand Gnomes
@ 2013-11-24 2:38 ` Peter Hurley
2013-11-26 13:00 ` Peter Hurley
0 siblings, 1 reply; 27+ messages in thread
From: Peter Hurley @ 2013-11-24 2:38 UTC (permalink / raw)
To: One Thousand Gnomes
Cc: Greg Kroah-Hartman, Jiri Slaby, linux-kernel, linux-serial
On 11/23/2013 07:25 PM, One Thousand Gnomes wrote:
> On Fri, 22 Nov 2013 10:59:25 -0500
> Peter Hurley <peter@hurleysoftware.com> wrote:
>
>> Instrument .receive_buf() and read() paths with trace_printk's
>> to aid in debugging flow control changes.
>
> tty devices have a device, we have a dev_dbg() layer. The old tty trace
> predates this but there isn't really any excuse for not using it now that
> I can see ?
I was using the ftrace facility because it has significantly less performance
impact than printk (which was an important factor while debugging flow control
problems).
That said, I could further macro-ize n_tty_trace() with selectable facility
(which would be useful when debugging problems that end in cpu death).
Regards,
Peter Hurley
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH tty-next 5/7] n_tty: Refactor input_available_p() by call site
2013-11-24 2:01 ` Peter Hurley
@ 2013-11-24 16:18 ` One Thousand Gnomes
0 siblings, 0 replies; 27+ messages in thread
From: One Thousand Gnomes @ 2013-11-24 16:18 UTC (permalink / raw)
To: Peter Hurley; +Cc: Greg Kroah-Hartman, Jiri Slaby, linux-kernel, linux-serial
On Sat, 23 Nov 2013 21:01:56 -0500
Peter Hurley <peter@hurleysoftware.com> wrote:
> On 11/23/2013 07:26 PM, One Thousand Gnomes wrote:
> > On Fri, 22 Nov 2013 10:59:23 -0500
> > Peter Hurley <peter@hurleysoftware.com> wrote:
> >
> >> Distinguish if caller is n_tty_poll() or n_tty_read(), and
> >> set the read/wakeup threshold accordingly.
> >>
> >> Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
> >
> > Doh ignore previous.. yes that patch is right. I should pay attention to
> > 0/7 8)
>
> Maybe I should add more to the commit message from the cover letter
> description ?
Maybe I should just read the code more carefully 8).
Alan
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH tty-next 7/7] n_tty: trace input/read flow control
2013-11-24 2:38 ` Peter Hurley
@ 2013-11-26 13:00 ` Peter Hurley
0 siblings, 0 replies; 27+ messages in thread
From: Peter Hurley @ 2013-11-26 13:00 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: One Thousand Gnomes, Jiri Slaby, linux-kernel, linux-serial
On 11/23/2013 09:38 PM, Peter Hurley wrote:
> On 11/23/2013 07:25 PM, One Thousand Gnomes wrote:
>> On Fri, 22 Nov 2013 10:59:25 -0500
>> Peter Hurley <peter@hurleysoftware.com> wrote:
>>
>>> Instrument .receive_buf() and read() paths with trace_printk's
>>> to aid in debugging flow control changes.
>>
>> tty devices have a device, we have a dev_dbg() layer. The old tty trace
>> predates this but there isn't really any excuse for not using it now that
>> I can see ?
>
> I was using the ftrace facility because it has significantly less performance
> impact than printk (which was an important factor while debugging flow control
> problems).
>
> That said, I could further macro-ize n_tty_trace() with selectable facility
> (which would be useful when debugging problems that end in cpu death).
Greg,
I don't really mind if you don't want to take this patch; it is useful but
it does clutter up the code.
Regards,
Peter Hurley
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v2 tty-next 0/8] n_tty cleanup + trace additions
2013-11-22 15:59 [PATCH tty-next 0/7] n_tty cleanup + trace additions Peter Hurley
` (6 preceding siblings ...)
2013-11-22 15:59 ` [PATCH tty-next 7/7] n_tty: trace input/read flow control Peter Hurley
@ 2013-12-02 19:24 ` Peter Hurley
2013-12-02 19:24 ` [PATCH v2 tty-next 1/8] n_tty: Merge .receive_buf() flavors Peter Hurley
` (7 more replies)
7 siblings, 8 replies; 27+ messages in thread
From: Peter Hurley @ 2013-12-02 19:24 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Jiri Slaby, linux-serial, linux-kernel, One Thousand Gnomes,
Peter Hurley
Greg,
This patchset is a v2 respin which contains no bug fixes, only
misc cleanup for the N_TTY line discipline and significant
additions to debug tracing.
As I mentioned in a previous email, I can hold on to patches 7 and 8
and keep them out-of-tree if you'd rather not have instrumented code
in the tty layer.
v2:
- Patches 1-6 are unchanged from v1.
- Patch 7 changes: make n_tty_trace() configurable to output via
pr_xxxx() or dev_dbg() rather than trace_printk() per Alan's comments
- Patch 8 adds echo buffer indices tracing (separately selectable)
v1:
Patches 1-4 cleans up code duplication remnants from 3.12's changes.
Patch 5 simplifies the snarled logic of how much data is considered
'input available'. I was getting tired of hand checking that logic
every time I touched minimum_to_wake.
Patch 6 reduces unnecessary wake ups.
Patch 7 extends the trace usage to aid in identifying and fixing
input flow-control bugs (this was used to fix the readline() bug).
I also expect to use it to reduce input processing restarts. It's
use is disabled by default and only enabled with the file-local
define N_TTY_TRACE (similar to TTY_DEBUG_HANGUP in tty_io.c).
Regards,
Peter Hurley (8):
n_tty: Merge .receive_buf() flavors
n_tty: Un-inline slow-path n_tty_receive_char()
n_tty: Un-inline slow-path n_tty_receive_char_closing()
n_tty: Refactor PARMRK doubling checks
n_tty: Refactor input_available_p() by call site
n_tty: Only perform wakeups for waiters
n_tty: Extend debug tracing
n_tty: Trace echo output with separate trace configuration
drivers/tty/n_tty.c | 174 +++++++++++++++++++++++++++++++++-------------------
1 file changed, 112 insertions(+), 62 deletions(-)
--
1.8.1.2
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v2 tty-next 1/8] n_tty: Merge .receive_buf() flavors
2013-12-02 19:24 ` [PATCH v2 tty-next 0/8] n_tty cleanup + trace additions Peter Hurley
@ 2013-12-02 19:24 ` Peter Hurley
2013-12-02 19:24 ` [PATCH v2 tty-next 2/8] n_tty: Un-inline slow-path n_tty_receive_char() Peter Hurley
` (6 subsequent siblings)
7 siblings, 0 replies; 27+ messages in thread
From: Peter Hurley @ 2013-12-02 19:24 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Jiri Slaby, linux-serial, linux-kernel, One Thousand Gnomes,
Peter Hurley
N_TTY's direct and flow-controlled flavors of the .receive_buf()
method are nearly identical; fold together.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
drivers/tty/n_tty.c | 43 ++++++++++++++++---------------------------
1 file changed, 16 insertions(+), 27 deletions(-)
diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index d4d63b0..446ccfd 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -1677,32 +1677,9 @@ static void __receive_buf(struct tty_struct *tty, const unsigned char *cp,
}
}
-static void n_tty_receive_buf(struct tty_struct *tty, const unsigned char *cp,
- char *fp, int count)
-{
- int room, n;
-
- down_read(&tty->termios_rwsem);
-
- while (1) {
- room = receive_room(tty);
- n = min(count, room);
- if (!n)
- break;
- __receive_buf(tty, cp, fp, n);
- cp += n;
- if (fp)
- fp += n;
- count -= n;
- }
-
- tty->receive_room = room;
- n_tty_check_throttle(tty);
- up_read(&tty->termios_rwsem);
-}
-
-static int n_tty_receive_buf2(struct tty_struct *tty, const unsigned char *cp,
- char *fp, int count)
+static int
+n_tty_receive_buf_common(struct tty_struct *tty, const unsigned char *cp,
+ char *fp, int count, int flow)
{
struct n_tty_data *ldata = tty->disc_data;
int room, n, rcvd = 0;
@@ -1713,7 +1690,7 @@ static int n_tty_receive_buf2(struct tty_struct *tty, const unsigned char *cp,
room = receive_room(tty);
n = min(count, room);
if (!n) {
- if (!room)
+ if (flow && !room)
ldata->no_room = 1;
break;
}
@@ -1732,6 +1709,18 @@ static int n_tty_receive_buf2(struct tty_struct *tty, const unsigned char *cp,
return rcvd;
}
+static void n_tty_receive_buf(struct tty_struct *tty, const unsigned char *cp,
+ char *fp, int count)
+{
+ n_tty_receive_buf_common(tty, cp, fp, count, 0);
+}
+
+static int n_tty_receive_buf2(struct tty_struct *tty, const unsigned char *cp,
+ char *fp, int count)
+{
+ return n_tty_receive_buf_common(tty, cp, fp, count, 1);
+}
+
int is_ignored(int sig)
{
return (sigismember(¤t->blocked, sig) ||
--
1.8.1.2
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v2 tty-next 2/8] n_tty: Un-inline slow-path n_tty_receive_char()
2013-12-02 19:24 ` [PATCH v2 tty-next 0/8] n_tty cleanup + trace additions Peter Hurley
2013-12-02 19:24 ` [PATCH v2 tty-next 1/8] n_tty: Merge .receive_buf() flavors Peter Hurley
@ 2013-12-02 19:24 ` Peter Hurley
2013-12-02 19:24 ` [PATCH v2 tty-next 3/8] n_tty: Un-inline slow-path n_tty_receive_char_closing() Peter Hurley
` (5 subsequent siblings)
7 siblings, 0 replies; 27+ messages in thread
From: Peter Hurley @ 2013-12-02 19:24 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Jiri Slaby, linux-serial, linux-kernel, One Thousand Gnomes,
Peter Hurley
Commit e60d27c4d8b33ba20896b76b6558f061bc6460ff,
n_tty: Factor LNEXT processing from per-char i/o path,
mistakenly inlined the non-inline alias, n_tty_receive_char(),
for the inline function, n_tty_receive_char_inline().
As n_tty_receive_char() is intended for slow-path char
processing only, un-inline it.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
drivers/tty/n_tty.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index 446ccfd..86de3d0 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -1420,7 +1420,7 @@ n_tty_receive_char_inline(struct tty_struct *tty, unsigned char c)
put_tty_queue(c, ldata);
}
-static inline void n_tty_receive_char(struct tty_struct *tty, unsigned char c)
+static void n_tty_receive_char(struct tty_struct *tty, unsigned char c)
{
n_tty_receive_char_inline(tty, c);
}
--
1.8.1.2
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v2 tty-next 3/8] n_tty: Un-inline slow-path n_tty_receive_char_closing()
2013-12-02 19:24 ` [PATCH v2 tty-next 0/8] n_tty cleanup + trace additions Peter Hurley
2013-12-02 19:24 ` [PATCH v2 tty-next 1/8] n_tty: Merge .receive_buf() flavors Peter Hurley
2013-12-02 19:24 ` [PATCH v2 tty-next 2/8] n_tty: Un-inline slow-path n_tty_receive_char() Peter Hurley
@ 2013-12-02 19:24 ` Peter Hurley
2013-12-02 19:24 ` [PATCH v2 tty-next 4/8] n_tty: Refactor PARMRK doubling checks Peter Hurley
` (4 subsequent siblings)
7 siblings, 0 replies; 27+ messages in thread
From: Peter Hurley @ 2013-12-02 19:24 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Jiri Slaby, linux-serial, linux-kernel, One Thousand Gnomes,
Peter Hurley
Although n_tty_receive_char_closing() only has one call-site,
let the compiler inline instead.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
drivers/tty/n_tty.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index 86de3d0..f8a76d3 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -1445,8 +1445,7 @@ n_tty_receive_char_fast(struct tty_struct *tty, unsigned char c)
put_tty_queue(c, ldata);
}
-static inline void
-n_tty_receive_char_closing(struct tty_struct *tty, unsigned char c)
+static void n_tty_receive_char_closing(struct tty_struct *tty, unsigned char c)
{
if (I_ISTRIP(tty))
c &= 0x7f;
--
1.8.1.2
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v2 tty-next 4/8] n_tty: Refactor PARMRK doubling checks
2013-12-02 19:24 ` [PATCH v2 tty-next 0/8] n_tty cleanup + trace additions Peter Hurley
` (2 preceding siblings ...)
2013-12-02 19:24 ` [PATCH v2 tty-next 3/8] n_tty: Un-inline slow-path n_tty_receive_char_closing() Peter Hurley
@ 2013-12-02 19:24 ` Peter Hurley
2013-12-02 19:24 ` [PATCH v2 tty-next 5/8] n_tty: Refactor input_available_p() by call site Peter Hurley
` (3 subsequent siblings)
7 siblings, 0 replies; 27+ messages in thread
From: Peter Hurley @ 2013-12-02 19:24 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Jiri Slaby, linux-serial, linux-kernel, One Thousand Gnomes,
Peter Hurley
Perform PARMRK doubling checks explicitly; remove ternary idiom
and local variable.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
drivers/tty/n_tty.c | 14 +++++---------
1 file changed, 5 insertions(+), 9 deletions(-)
diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index f8a76d3..008b6e5 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -1260,7 +1260,6 @@ static int
n_tty_receive_char_special(struct tty_struct *tty, unsigned char c)
{
struct n_tty_data *ldata = tty->disc_data;
- int parmrk;
if (I_IXON(tty)) {
if (c == START_CHAR(tty)) {
@@ -1345,8 +1344,6 @@ n_tty_receive_char_special(struct tty_struct *tty, unsigned char c)
}
if ((c == EOL_CHAR(tty)) ||
(c == EOL2_CHAR(tty) && L_IEXTEN(tty))) {
- parmrk = (c == (unsigned char) '\377' && I_PARMRK(tty))
- ? 1 : 0;
/*
* XXX are EOL_CHAR and EOL2_CHAR echoed?!?
*/
@@ -1361,7 +1358,7 @@ n_tty_receive_char_special(struct tty_struct *tty, unsigned char c)
* XXX does PARMRK doubling happen for
* EOL_CHAR and EOL2_CHAR?
*/
- if (parmrk)
+ if (c == (unsigned char) '\377' && I_PARMRK(tty))
put_tty_queue(c, ldata);
handle_newline:
@@ -1375,7 +1372,6 @@ handle_newline:
}
}
- parmrk = (c == (unsigned char) '\377' && I_PARMRK(tty)) ? 1 : 0;
if (L_ECHO(tty)) {
finish_erasing(ldata);
if (c == '\n')
@@ -1389,7 +1385,8 @@ handle_newline:
commit_echoes(tty);
}
- if (parmrk)
+ /* PARMRK doubling check */
+ if (c == (unsigned char) '\377' && I_PARMRK(tty))
put_tty_queue(c, ldata);
put_tty_queue(c, ldata);
@@ -1400,7 +1397,6 @@ static inline void
n_tty_receive_char_inline(struct tty_struct *tty, unsigned char c)
{
struct n_tty_data *ldata = tty->disc_data;
- int parmrk;
if (tty->stopped && !tty->flow_stopped && I_IXON(tty) && I_IXANY(tty)) {
start_tty(tty);
@@ -1414,8 +1410,8 @@ n_tty_receive_char_inline(struct tty_struct *tty, unsigned char c)
echo_char(c, tty);
commit_echoes(tty);
}
- parmrk = (c == (unsigned char) '\377' && I_PARMRK(tty)) ? 1 : 0;
- if (parmrk)
+ /* PARMRK doubling check */
+ if (c == (unsigned char) '\377' && I_PARMRK(tty))
put_tty_queue(c, ldata);
put_tty_queue(c, ldata);
}
--
1.8.1.2
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v2 tty-next 5/8] n_tty: Refactor input_available_p() by call site
2013-12-02 19:24 ` [PATCH v2 tty-next 0/8] n_tty cleanup + trace additions Peter Hurley
` (3 preceding siblings ...)
2013-12-02 19:24 ` [PATCH v2 tty-next 4/8] n_tty: Refactor PARMRK doubling checks Peter Hurley
@ 2013-12-02 19:24 ` Peter Hurley
2013-12-02 19:24 ` [PATCH v2 tty-next 6/8] n_tty: Only perform wakeups for waiters Peter Hurley
` (2 subsequent siblings)
7 siblings, 0 replies; 27+ messages in thread
From: Peter Hurley @ 2013-12-02 19:24 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Jiri Slaby, linux-serial, linux-kernel, One Thousand Gnomes,
Peter Hurley
Distinguish if caller is n_tty_poll() or n_tty_read(), and
set the read/wakeup threshold accordingly.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
drivers/tty/n_tty.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index 008b6e5..aae171e 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -1881,14 +1881,15 @@ static int n_tty_open(struct tty_struct *tty)
return 0;
}
-static inline int input_available_p(struct tty_struct *tty, int amt)
+static inline int input_available_p(struct tty_struct *tty, int poll)
{
struct n_tty_data *ldata = tty->disc_data;
+ int amt = poll && !TIME_CHAR(tty) ? MIN_CHAR(tty) : 1;
if (ldata->icanon && !L_EXTPROC(tty)) {
if (ldata->canon_head != ldata->read_tail)
return 1;
- } else if (read_cnt(ldata) >= (amt ? amt : 1))
+ } else if (read_cnt(ldata) >= amt)
return 1;
return 0;
@@ -2393,7 +2394,7 @@ static unsigned int n_tty_poll(struct tty_struct *tty, struct file *file,
poll_wait(file, &tty->read_wait, wait);
poll_wait(file, &tty->write_wait, wait);
- if (input_available_p(tty, TIME_CHAR(tty) ? 0 : MIN_CHAR(tty)))
+ if (input_available_p(tty, 1))
mask |= POLLIN | POLLRDNORM;
if (tty->packet && tty->link->ctrl_status)
mask |= POLLPRI | POLLIN | POLLRDNORM;
--
1.8.1.2
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v2 tty-next 6/8] n_tty: Only perform wakeups for waiters
2013-12-02 19:24 ` [PATCH v2 tty-next 0/8] n_tty cleanup + trace additions Peter Hurley
` (4 preceding siblings ...)
2013-12-02 19:24 ` [PATCH v2 tty-next 5/8] n_tty: Refactor input_available_p() by call site Peter Hurley
@ 2013-12-02 19:24 ` Peter Hurley
2013-12-02 19:24 ` [PATCH v2 tty-next 7/8] n_tty: Extend debug tracing Peter Hurley
2013-12-02 19:24 ` [PATCH v2 tty-next 8/8] n_tty: Trace echo output with separate trace configuration Peter Hurley
7 siblings, 0 replies; 27+ messages in thread
From: Peter Hurley @ 2013-12-02 19:24 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Jiri Slaby, linux-serial, linux-kernel, One Thousand Gnomes,
Peter Hurley
Only wakeup the _waiting_ reader, polls and/or writer(s).
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
drivers/tty/n_tty.c | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index aae171e..5254082 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -275,7 +275,8 @@ static void n_tty_check_unthrottle(struct tty_struct *tty)
return;
n_tty_set_room(tty);
n_tty_write_wakeup(tty->link);
- wake_up_interruptible_poll(&tty->link->write_wait, POLLOUT);
+ if (waitqueue_active(&tty->link->write_wait))
+ wake_up_interruptible_poll(&tty->link->write_wait, POLLOUT);
return;
}
@@ -350,7 +351,8 @@ static void n_tty_packet_mode_flush(struct tty_struct *tty)
spin_lock_irqsave(&tty->ctrl_lock, flags);
if (tty->link->packet) {
tty->ctrl_status |= TIOCPKT_FLUSHREAD;
- wake_up_interruptible(&tty->link->read_wait);
+ if (waitqueue_active(&tty->link->read_wait))
+ wake_up_interruptible(&tty->link->read_wait);
}
spin_unlock_irqrestore(&tty->ctrl_lock, flags);
}
@@ -1158,7 +1160,8 @@ static void n_tty_receive_break(struct tty_struct *tty)
put_tty_queue('\0', ldata);
}
put_tty_queue('\0', ldata);
- wake_up_interruptible(&tty->read_wait);
+ if (waitqueue_active(&tty->read_wait))
+ wake_up_interruptible(&tty->read_wait);
}
/**
@@ -1216,7 +1219,8 @@ static void n_tty_receive_parity_error(struct tty_struct *tty, unsigned char c)
put_tty_queue('\0', ldata);
else
put_tty_queue(c, ldata);
- wake_up_interruptible(&tty->read_wait);
+ if (waitqueue_active(&tty->read_wait))
+ wake_up_interruptible(&tty->read_wait);
}
static void
@@ -1811,8 +1815,10 @@ static void n_tty_set_termios(struct tty_struct *tty, struct ktermios *old)
start_tty(tty);
/* The termios change make the tty ready for I/O */
- wake_up_interruptible(&tty->write_wait);
- wake_up_interruptible(&tty->read_wait);
+ if (waitqueue_active(&tty->write_wait))
+ wake_up_interruptible(&tty->write_wait);
+ if (waitqueue_active(&tty->read_wait))
+ wake_up_interruptible(&tty->read_wait);
}
/**
--
1.8.1.2
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v2 tty-next 7/8] n_tty: Extend debug tracing
2013-12-02 19:24 ` [PATCH v2 tty-next 0/8] n_tty cleanup + trace additions Peter Hurley
` (5 preceding siblings ...)
2013-12-02 19:24 ` [PATCH v2 tty-next 6/8] n_tty: Only perform wakeups for waiters Peter Hurley
@ 2013-12-02 19:24 ` Peter Hurley
2013-12-09 0:54 ` Greg Kroah-Hartman
2013-12-02 19:24 ` [PATCH v2 tty-next 8/8] n_tty: Trace echo output with separate trace configuration Peter Hurley
7 siblings, 1 reply; 27+ messages in thread
From: Peter Hurley @ 2013-12-02 19:24 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Jiri Slaby, linux-serial, linux-kernel, One Thousand Gnomes,
Peter Hurley
Add configurable trace support to allow tracing with dev_dbg,
pr_dbg, or trace_printk. Instrument .receive_buf() and read()
paths with traces to aid in debugging flow control changes.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
drivers/tty/n_tty.c | 79 ++++++++++++++++++++++++++++++++++++++++++-----------
1 file changed, 63 insertions(+), 16 deletions(-)
diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index 5254082..26ae9d7 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -80,12 +80,18 @@
#define ECHO_BLOCK 256
#define ECHO_DISCARD_WATERMARK N_TTY_BUF_SIZE - (ECHO_BLOCK + 32)
+#define n_tty_trace_facility trace_printk
+#define _n_tty_trace(tty, f, args...) \
+ do { \
+ char buf[64]; \
+ n_tty_trace_facility("[%s] " f, tty_name(tty, buf), ##args); \
+ } while (0)
#undef N_TTY_TRACE
#ifdef N_TTY_TRACE
-# define n_tty_trace(f, args...) trace_printk(f, ##args)
+# define n_tty_trace(tty, f, args...) _n_tty_trace(tty, f, args)
#else
-# define n_tty_trace(f, args...)
+# define n_tty_trace(tty, f, args...) do { } while (0)
#endif
struct n_tty_data {
@@ -179,8 +185,13 @@ static int receive_room(struct tty_struct *tty)
* that erase characters will be handled. Other excess
* characters will be beeped.
*/
- if (left <= 0)
+ if (left <= 0) {
left = ldata->icanon && ldata->canon_head == ldata->read_tail;
+ if (left) {
+ n_tty_trace(tty, "overflow, head:%zu tail:%zu\n",
+ ldata->read_head, ldata->read_tail);
+ }
+ }
return left;
}
@@ -200,11 +211,14 @@ static int receive_room(struct tty_struct *tty)
static void n_tty_set_room(struct tty_struct *tty)
{
struct n_tty_data *ldata = tty->disc_data;
+ int avail;
/* Did this open up the receive buffer? We may need to flip */
- if (unlikely(ldata->no_room) && receive_room(tty)) {
+ if (unlikely(ldata->no_room) && (avail = receive_room(tty))) {
ldata->no_room = 0;
+ n_tty_trace(tty, "restart worker (avail = %d)\n", avail);
+
WARN_RATELIMIT(tty->port->itty == NULL,
"scheduling with invalid itty\n");
/* see if ldisc has been killed - if so, this means that
@@ -240,6 +254,8 @@ static ssize_t chars_in_buffer(struct tty_struct *tty)
static void n_tty_write_wakeup(struct tty_struct *tty)
{
+ n_tty_trace(tty, "\n");
+
if (tty->fasync && test_and_clear_bit(TTY_DO_WRITE_WAKEUP, &tty->flags))
kill_fasync(&tty->fasync, SIGIO, POLL_OUT);
}
@@ -254,13 +270,16 @@ static void n_tty_check_throttle(struct tty_struct *tty)
* canonical mode and don't have a newline yet!
*/
while (1) {
- int throttled;
+ int throttled, avail;
tty_set_flow_change(tty, TTY_THROTTLE_SAFE);
- if (receive_room(tty) >= TTY_THRESHOLD_THROTTLE)
+ avail = receive_room(tty);
+ if (avail >= TTY_THRESHOLD_THROTTLE)
break;
throttled = tty_throttle_safe(tty);
- if (!throttled)
+ if (!throttled) {
+ n_tty_trace(tty, "throttled (avail = %d)/n", avail);
break;
+ }
}
__tty_set_flow_change(tty, 0);
}
@@ -289,16 +308,19 @@ static void n_tty_check_unthrottle(struct tty_struct *tty)
*/
while (1) {
- int unthrottled;
+ int unthrottled, used;
tty_set_flow_change(tty, TTY_UNTHROTTLE_SAFE);
- if (chars_in_buffer(tty) > TTY_THRESHOLD_UNTHROTTLE)
+ used = chars_in_buffer(tty);
+ if (used > TTY_THRESHOLD_UNTHROTTLE)
break;
if (!tty->count)
break;
n_tty_set_room(tty);
unthrottled = tty_unthrottle_safe(tty);
- if (!unthrottled)
+ if (!unthrottled) {
+ n_tty_trace(tty, "unthrottled (used = %d)\n", used);
break;
+ }
}
__tty_set_flow_change(tty, 0);
}
@@ -374,6 +396,9 @@ static void n_tty_packet_mode_flush(struct tty_struct *tty)
static void n_tty_flush_buffer(struct tty_struct *tty)
{
down_write(&tty->termios_rwsem);
+
+ n_tty_trace(tty, "\n");
+
reset_buffer_flags(tty->disc_data);
n_tty_set_room(tty);
@@ -770,6 +795,10 @@ static size_t __process_echoes(struct tty_struct *tty)
* of echo overrun before the next commit), then discard enough
* data at the tail to prevent a subsequent overrun */
while (ldata->echo_commit - tail >= ECHO_DISCARD_WATERMARK) {
+
+ n_tty_trace(tty, "discard echoes (%zu - %zu > %d)\n",
+ ldata->echo_commit, tail, ECHO_DISCARD_WATERMARK);
+
if (echo_buf(ldata, tail) == ECHO_OP_START) {
if (echo_buf(ldata, tail + 1) == ECHO_OP_ERASE_TAB)
tail += 3;
@@ -1369,6 +1398,7 @@ handle_newline:
set_bit(ldata->read_head & (N_TTY_BUF_SIZE - 1), ldata->read_flags);
put_tty_queue(c, ldata);
ldata->canon_head = ldata->read_head;
+ n_tty_trace(tty, "canon_head:%zu\n", ldata->canon_head);
kill_fasync(&tty->fasync, SIGIO, POLL_IN);
if (waitqueue_active(&tty->read_wait))
wake_up_interruptible(&tty->read_wait);
@@ -1670,6 +1700,7 @@ static void __receive_buf(struct tty_struct *tty, const unsigned char *cp,
if ((!ldata->icanon && (read_cnt(ldata) >= ldata->minimum_to_wake)) ||
L_EXTPROC(tty)) {
+ n_tty_trace(tty, "reader wakeup, count:%zd\n", read_cnt(ldata));
kill_fasync(&tty->fasync, SIGIO, POLL_IN);
if (waitqueue_active(&tty->read_wait))
wake_up_interruptible(&tty->read_wait);
@@ -1689,6 +1720,8 @@ n_tty_receive_buf_common(struct tty_struct *tty, const unsigned char *cp,
room = receive_room(tty);
n = min(count, room);
if (!n) {
+ if (!room)
+ n_tty_trace(tty, "buffer full\n");
if (flow && !room)
ldata->no_room = 1;
break;
@@ -1705,6 +1738,8 @@ n_tty_receive_buf_common(struct tty_struct *tty, const unsigned char *cp,
n_tty_check_throttle(tty);
up_read(&tty->termios_rwsem);
+ n_tty_trace(tty, "rcvd:%d\n", rcvd);
+
return rcvd;
}
@@ -1814,6 +1849,10 @@ static void n_tty_set_termios(struct tty_struct *tty, struct ktermios *old)
if (!I_IXON(tty) && old && (old->c_iflag & IXON) && !tty->flow_stopped)
start_tty(tty);
+ n_tty_trace(tty, "mode:%s tail:%zu canon:%zu head:%zu\n",
+ ldata->icanon ? "canon" : "raw", ldata->read_tail,
+ ldata->canon_head, ldata->read_head);
+
/* The termios change make the tty ready for I/O */
if (waitqueue_active(&tty->write_wait))
wake_up_interruptible(&tty->write_wait);
@@ -1893,10 +1932,14 @@ static inline int input_available_p(struct tty_struct *tty, int poll)
int amt = poll && !TIME_CHAR(tty) ? MIN_CHAR(tty) : 1;
if (ldata->icanon && !L_EXTPROC(tty)) {
- if (ldata->canon_head != ldata->read_tail)
+ if (ldata->canon_head != ldata->read_tail) {
+ n_tty_trace(tty, "line avail\n");
return 1;
- } else if (read_cnt(ldata) >= amt)
+ }
+ } else if (read_cnt(ldata) >= amt) {
+ n_tty_trace(tty, "avail:%zu amt:%d\n", read_cnt(ldata), amt);
return 1;
+ }
return 0;
}
@@ -1993,8 +2036,8 @@ static int canon_copy_from_read_buf(struct tty_struct *tty,
tail = ldata->read_tail & (N_TTY_BUF_SIZE - 1);
size = min_t(size_t, tail + n, N_TTY_BUF_SIZE);
- n_tty_trace("%s: nr:%zu tail:%zu n:%zu size:%zu\n",
- __func__, *nr, tail, n, size);
+ n_tty_trace(tty, "nr:%zu tail:%zu n:%zu size:%zu\n",
+ *nr, tail, n, size);
eol = find_next_bit(ldata->read_flags, size, tail);
more = n - (size - tail);
@@ -2018,8 +2061,8 @@ static int canon_copy_from_read_buf(struct tty_struct *tty,
eof_push = !n && ldata->read_tail != ldata->line_start;
}
- n_tty_trace("%s: eol:%zu found:%d n:%zu c:%zu size:%zu more:%zu\n",
- __func__, eol, found, n, c, size, more);
+ n_tty_trace(tty, "eol:%zu found:%d n:%zu c:%zu size:%zu more:%zu\n",
+ eol, found, n, c, size, more);
if (n > size) {
ret = copy_to_user(*b, read_buf_addr(ldata, tail), size);
@@ -2211,6 +2254,8 @@ static ssize_t n_tty_read(struct tty_struct *tty, struct file *file,
n_tty_set_room(tty);
up_read(&tty->termios_rwsem);
+ n_tty_trace(tty, "blocking\n");
+
timeout = schedule_timeout(timeout);
down_read(&tty->termios_rwsem);
@@ -2268,6 +2313,8 @@ static ssize_t n_tty_read(struct tty_struct *tty, struct file *file,
if (b - buf)
retval = b - buf;
+ n_tty_trace(tty, "ret:%zd\n", retval);
+
return retval;
}
--
1.8.1.2
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v2 tty-next 8/8] n_tty: Trace echo output with separate trace configuration
2013-12-02 19:24 ` [PATCH v2 tty-next 0/8] n_tty cleanup + trace additions Peter Hurley
` (6 preceding siblings ...)
2013-12-02 19:24 ` [PATCH v2 tty-next 7/8] n_tty: Extend debug tracing Peter Hurley
@ 2013-12-02 19:24 ` Peter Hurley
7 siblings, 0 replies; 27+ messages in thread
From: Peter Hurley @ 2013-12-02 19:24 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Jiri Slaby, linux-serial, linux-kernel, One Thousand Gnomes,
Peter Hurley
Add trace support for echo output but leave it separately selectable/
buildable from the base trace support.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
drivers/tty/n_tty.c | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index 26ae9d7..bd01d97 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -94,6 +94,13 @@
# define n_tty_trace(tty, f, args...) do { } while (0)
#endif
+#undef N_TTY_TRACE_ECHOES
+#ifdef N_TTY_TRACE_ECHOES
+# define n_tty_trace_echoes(tty, f, args...) _n_tty_trace(tty, f, args)
+#else
+# define n_tty_trace_echoes(tty, f, args...) do { } while (0)
+#endif
+
struct n_tty_data {
/* producer-published */
size_t read_head;
@@ -791,13 +798,18 @@ static size_t __process_echoes(struct tty_struct *tty)
}
}
+ n_tty_trace_echoes(tty, "echoes:%zu,%zu commit:%zu head:%zu\n",
+ ldata->echo_tail, tail, ldata->echo_commit,
+ ldata->echo_head);
+
/* If the echo buffer is nearly full (so that the possibility exists
* of echo overrun before the next commit), then discard enough
* data at the tail to prevent a subsequent overrun */
while (ldata->echo_commit - tail >= ECHO_DISCARD_WATERMARK) {
- n_tty_trace(tty, "discard echoes (%zu - %zu > %d)\n",
- ldata->echo_commit, tail, ECHO_DISCARD_WATERMARK);
+ n_tty_trace_echoes(tty, "discard echoes (%zu - %zu > %d)\n",
+ ldata->echo_commit, tail,
+ ECHO_DISCARD_WATERMARK);
if (echo_buf(ldata, tail) == ECHO_OP_START) {
if (echo_buf(ldata, tail + 1) == ECHO_OP_ERASE_TAB)
--
1.8.1.2
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH v2 tty-next 7/8] n_tty: Extend debug tracing
2013-12-02 19:24 ` [PATCH v2 tty-next 7/8] n_tty: Extend debug tracing Peter Hurley
@ 2013-12-09 0:54 ` Greg Kroah-Hartman
0 siblings, 0 replies; 27+ messages in thread
From: Greg Kroah-Hartman @ 2013-12-09 0:54 UTC (permalink / raw)
To: Peter Hurley; +Cc: Jiri Slaby, linux-serial, linux-kernel, One Thousand Gnomes
On Mon, Dec 02, 2013 at 02:24:47PM -0500, Peter Hurley wrote:
> Add configurable trace support to allow tracing with dev_dbg,
> pr_dbg, or trace_printk. Instrument .receive_buf() and read()
> paths with traces to aid in debugging flow control changes.
>
> Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
> ---
> drivers/tty/n_tty.c | 79 ++++++++++++++++++++++++++++++++++++++++++-----------
> 1 file changed, 63 insertions(+), 16 deletions(-)
If we are going to do tracing in the tty layer, why not use the
in-kernel tracing functionality instead of having this mis-mash of
different ways to get tracing data out?
I'd much rather prefer that, so I've stopped applying things in this
series at this point, I've applied the 6 previous ones.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2013-12-09 8:19 UTC | newest]
Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-11-22 15:59 [PATCH tty-next 0/7] n_tty cleanup + trace additions Peter Hurley
2013-11-22 15:59 ` [PATCH tty-next 1/7] n_tty: Merge .receive_buf() flavors Peter Hurley
2013-11-22 15:59 ` [PATCH tty-next 2/7] n_tty: Un-inline slow-path n_tty_receive_char() Peter Hurley
2013-11-22 15:59 ` [PATCH tty-next 3/7] n_tty: Un-inline slow-path n_tty_receive_char_closing() Peter Hurley
2013-11-22 15:59 ` [PATCH tty-next 4/7] n_tty: Refactor PARMRK doubling checks Peter Hurley
2013-11-22 15:59 ` [PATCH tty-next 5/7] n_tty: Refactor input_available_p() by call site Peter Hurley
2013-11-24 0:22 ` One Thousand Gnomes
2013-11-24 0:26 ` One Thousand Gnomes
2013-11-24 2:01 ` Peter Hurley
2013-11-24 16:18 ` One Thousand Gnomes
2013-11-22 15:59 ` [PATCH tty-next 6/7] n_tty: Only perform wakeups for waiters Peter Hurley
2013-11-24 0:23 ` One Thousand Gnomes
2013-11-24 2:29 ` Peter Hurley
2013-11-22 15:59 ` [PATCH tty-next 7/7] n_tty: trace input/read flow control Peter Hurley
2013-11-24 0:25 ` One Thousand Gnomes
2013-11-24 2:38 ` Peter Hurley
2013-11-26 13:00 ` Peter Hurley
2013-12-02 19:24 ` [PATCH v2 tty-next 0/8] n_tty cleanup + trace additions Peter Hurley
2013-12-02 19:24 ` [PATCH v2 tty-next 1/8] n_tty: Merge .receive_buf() flavors Peter Hurley
2013-12-02 19:24 ` [PATCH v2 tty-next 2/8] n_tty: Un-inline slow-path n_tty_receive_char() Peter Hurley
2013-12-02 19:24 ` [PATCH v2 tty-next 3/8] n_tty: Un-inline slow-path n_tty_receive_char_closing() Peter Hurley
2013-12-02 19:24 ` [PATCH v2 tty-next 4/8] n_tty: Refactor PARMRK doubling checks Peter Hurley
2013-12-02 19:24 ` [PATCH v2 tty-next 5/8] n_tty: Refactor input_available_p() by call site Peter Hurley
2013-12-02 19:24 ` [PATCH v2 tty-next 6/8] n_tty: Only perform wakeups for waiters Peter Hurley
2013-12-02 19:24 ` [PATCH v2 tty-next 7/8] n_tty: Extend debug tracing Peter Hurley
2013-12-09 0:54 ` Greg Kroah-Hartman
2013-12-02 19:24 ` [PATCH v2 tty-next 8/8] n_tty: Trace echo output with separate trace configuration Peter Hurley
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).