All of lore.kernel.org
 help / color / mirror / Atom feed
* tty/serial eating \n regression in 3.13-rc1
@ 2013-11-26 21:53 Jason Gunthorpe
  2013-11-27 12:13 ` Peter Hurley
  0 siblings, 1 reply; 9+ messages in thread
From: Jason Gunthorpe @ 2013-11-26 21:53 UTC (permalink / raw)
  To: Peter Hurley, Greg Kroah-Hartman, Jiri Slaby; +Cc: linux-serial, linux-kernel

Hello Peter,

I have been testing 3.13-rc1 and I noticed a change in behavior in the
tty/serial layer. Specifically I have a login running on serial that
presents the usual login:/password: prompt.

3.10.12 does this:

login: admin
password: 
prompt>

While 3.13-rc1 does this:

login: admin
password: prompt>

That is to say, a \n got lost. I have also noticed while using the
serial console that '\n's are occasionally lost, and the lossage is
highly repeatable and seemingly based on number of chars being output
in a burst, eg entering 'foo bar' at a shell prompt might loose the \n,
while entering 'foo bar ' will not.

I ran a full git bisect search and determined that the attached patch
introduced the problem.

This seems like a bug - at least the commit comment seems like it
should not cause any change in behaviour. I can reproduce it and help
debug, if you can give some guidance, I am not a tty layer expert :)

I have seen this bug on ppc with an out-of-tree serial driver, and on
ARM kirkwood with the serial8250 driver. I did the bisection search on
PPC.

good 8bb495e3f02401ee6f76d1b1d77f3ac9f079e376 Linux 3.10
bad 6ce4eac1f600b34f2f7f58f9cd8f0503d79e42ae Linux 3.13-rc1
good 6e4664525b1db28f8c4e1130957f70a94c19213e Linux 3.11
bad 5e01dc7b26d9f24f39abace5da98ccbd6a5ceb52 Linux 3.12

bad cc998ff8811530be521f6b316f37ab7676a07938
bad 816434ec4a674fcdb3c2221a6dffdc8f34020550
good 751144271f4b63d5de9005ea4e5e6e5c7c6fd629
bad 40031da445fb4d269af9c7c445b2adf674f171e7
bad f66c83d059d1ed90968caa81d401f160912b063a

bad 1ef39808ca27837017a433f94aa7055cb8490e80 serial: sirf: add DT-binding document to describle detailed properties
bad 10d8b34a421716d55a4ff7c2d427c35e88b8fd60 serial: max310x: Driver rework
good 0f56bd2f6a97d8b0eb5c8f9bc04b83a6c16d1d48 tty: Use non-atomic state to signal flip buffer flush pending
bad a1dd30e9b4c38a4a177106741b03ac6a55777286 n_tty: Special case EXTPROC receive_buf() as raw mode
bad 29c7c5ca36d9c132cf9c37a09bc43626e790fb4c n_tty: Eliminate counter in __process_echoes
good addaebccf63a8c9f7c7a62ce1ceb9da4307c5a1c n_tty: Use separate head and tail indices for echo_buf
good 019ebdf9f26fd2e43b9e1af576835183e95dc82e n_tty: Eliminate echo_commit memory barrier
bad bc5b1ec5860a9bf52842be4a3a9d96e19f06c11d (*) n_tty: Only flush echo output if actually output
bad cbfd0340ae1993378fd47179db949e050e16e697 n_tty: Process echoes in blocks

==> cbfd0340ae1993378fd47179db949e050e16e697

Thanks,
Jason

>From cbfd0340ae1993378fd47179db949e050e16e697 Mon Sep 17 00:00:00 2001
From: Peter Hurley <peter@hurleysoftware.com>
Date: Sat, 15 Jun 2013 10:04:26 -0400
Subject: [PATCH] n_tty: Process echoes in blocks

Byte-by-byte echo output is painfully slow, requiring a lock/unlock
cycle for every input byte.

Instead, perform the echo output in blocks of 256 characters, and
at least once per flip buffer receive. Enough space is reserved in
the echo buffer to guarantee a full block can be saved without
overrunning the echo output. Overrun is prevented by discarding
the oldest echoes until enough space exists in the echo buffer
to receive at least a full block of new echoes.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/tty/n_tty.c | 70 +++++++++++++++++++++++++++++++++++------------------
 1 file changed, 47 insertions(+), 23 deletions(-)

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index 0f76b90..20983af 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -74,6 +74,11 @@
 #define ECHO_OP_SET_CANON_COL 0x81
 #define ECHO_OP_ERASE_TAB 0x82
 
+#define ECHO_COMMIT_WATERMARK	256
+#define ECHO_BLOCK		256
+#define ECHO_DISCARD_WATERMARK	N_TTY_BUF_SIZE - (ECHO_BLOCK + 32)
+
+
 #undef N_TTY_TRACE
 #ifdef N_TTY_TRACE
 # define n_tty_trace(f, args...)	trace_printk(f, ##args)
@@ -766,15 +771,40 @@ static void __process_echoes(struct tty_struct *tty)
 		}
 	}
 
+	/* 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) {
+		if (echo_buf(ldata, tail == ECHO_OP_START)) {
+			if (echo_buf(ldata, tail) == ECHO_OP_ERASE_TAB)
+				tail += 3;
+			else
+				tail += 2;
+		} else
+			tail++;
+	}
+
 	ldata->echo_tail = tail;
 }
 
 static void commit_echoes(struct tty_struct *tty)
 {
 	struct n_tty_data *ldata = tty->disc_data;
+	size_t nr, old;
+	size_t head;
+
+	head = ldata->echo_head;
+	old = ldata->echo_commit - ldata->echo_tail;
+
+	/* Process committed echoes if the accumulated # of bytes
+	 * is over the threshold (and try again each time another
+	 * block is accumulated) */
+	nr = head - ldata->echo_tail;
+	if (nr < ECHO_COMMIT_WATERMARK || (nr % ECHO_BLOCK > old % ECHO_BLOCK))
+		return;
 
 	mutex_lock(&ldata->output_lock);
-	ldata->echo_commit = ldata->echo_head;
+	ldata->echo_commit = head;
 	__process_echoes(tty);
 	mutex_unlock(&ldata->output_lock);
 
@@ -797,37 +827,29 @@ static void process_echoes(struct tty_struct *tty)
 		tty->ops->flush_chars(tty);
 }
 
+static void flush_echoes(struct tty_struct *tty)
+{
+	struct n_tty_data *ldata = tty->disc_data;
+
+	if (!L_ECHO(tty) || ldata->echo_commit == ldata->echo_head)
+		return;
+
+	mutex_lock(&ldata->output_lock);
+	ldata->echo_commit = ldata->echo_head;
+	__process_echoes(tty);
+	mutex_unlock(&ldata->output_lock);
+}
+
 /**
  *	add_echo_byte	-	add a byte to the echo buffer
  *	@c: unicode byte to echo
  *	@ldata: n_tty data
  *
  *	Add a character or operation byte to the echo buffer.
- *
- *	Locks: may claim output_lock to prevent concurrent modify of
- *	       echo_tail by process_echoes().
  */
 
-static void add_echo_byte(unsigned char c, struct n_tty_data *ldata)
+static inline void add_echo_byte(unsigned char c, struct n_tty_data *ldata)
 {
-	if (ldata->echo_head - ldata->echo_tail == N_TTY_BUF_SIZE) {
-		size_t head = ldata->echo_head;
-
-		mutex_lock(&ldata->output_lock);
-		/*
-		 * Since the buffer start position needs to be advanced,
-		 * be sure to step by a whole operation byte group.
-		 */
-		if (echo_buf(ldata, head) == ECHO_OP_START) {
-			if (echo_buf(ldata, head + 1) == ECHO_OP_ERASE_TAB)
-				ldata->echo_tail += 3;
-			else
-				ldata->echo_tail += 2;
-		} else
-			ldata->echo_tail++;
-		mutex_unlock(&ldata->output_lock);
-	}
-
 	*echo_buf_addr(ldata, ldata->echo_head++) = c;
 }
 
@@ -1515,6 +1537,8 @@ static void __receive_buf(struct tty_struct *tty, const unsigned char *cp,
 				break;
 			}
 		}
+
+		flush_echoes(tty);
 		if (tty->ops->flush_chars)
 			tty->ops->flush_chars(tty);
 	}
-- 
1.8.1.2


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

* Re: tty/serial eating \n regression in 3.13-rc1
  2013-11-26 21:53 tty/serial eating \n regression in 3.13-rc1 Jason Gunthorpe
@ 2013-11-27 12:13 ` Peter Hurley
  2013-11-27 18:13   ` Jason Gunthorpe
  0 siblings, 1 reply; 9+ messages in thread
From: Peter Hurley @ 2013-11-27 12:13 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Greg Kroah-Hartman, Jiri Slaby, linux-serial, linux-kernel

On 11/26/2013 04:53 PM, Jason Gunthorpe wrote:
> Hello Peter,
>
> I have been testing 3.13-rc1 and I noticed a change in behavior in the
> tty/serial layer. Specifically I have a login running on serial that
> presents the usual login:/password: prompt.
>
> 3.10.12 does this:
>
> login: admin
> password:
> prompt>
>
> While 3.13-rc1 does this:
>
> login: admin
> password: prompt>
>
> That is to say, a \n got lost. I have also noticed while using the
> serial console that '\n's are occasionally lost, and the lossage is
> highly repeatable and seemingly based on number of chars being output
> in a burst, eg entering 'foo bar' at a shell prompt might loose the \n,
> while entering 'foo bar ' will not.
>
> I ran a full git bisect search and determined that the attached patch
> introduced the problem.
>
> This seems like a bug - at least the commit comment seems like it
> should not cause any change in behaviour. I can reproduce it and help
> debug, if you can give some guidance, I am not a tty layer expert :)

Hi Jason,

Thanks for the bisect: this is definitely a bug and not designed behavior.
But just to be clear here, the error you're seeing is strictly wrt.
echo output; normal output remains uncorrupted?

Also, you're not seeing other echo drop-outs?

Although there are some fixes due for 3.13-rc2 concerning echoes, I
think this may be a smp weak memory ordering problem.

> I have seen this bug on ppc with an out-of-tree serial driver, and on
> ARM kirkwood with the serial8250 driver. I did the bisection search on
> PPC.

Ah, the games processors play. I assume the ppc is the host machine; the
serial terminal remained unchanged, yes?

Would you please test with the patch below which uses ftrace to
capture the output? [The trace only tracks the echo buffer indices, not
any of the data.]

When you see the lost '\n', copy /sys/kernel/debug/tracing/trace
to a file and send it to me (compressed if too big for the lists). Or
you can send it off-list, if you'd prefer.

[Apologies if the patch is white-space mangled; my mailer [Thunderbird]
is dain-bramaged. I think git am will still do the right thing.]

--- >% ----
Subject: [PATCH] n_tty: Add trace support for echo output

Debug patch.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
  drivers/tty/n_tty.c | 24 ++++++++++++++++++++++++
  1 file changed, 24 insertions(+)

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index 7cdd1eb..b055838 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -79,6 +79,12 @@
  #define ECHO_BLOCK		256
  #define ECHO_DISCARD_WATERMARK	N_TTY_BUF_SIZE - (ECHO_BLOCK + 32)

+#define trace_facility trace_printk
+#define _n_tty_trace(tty, f, args...)					\
+	do {								\
+		char buf[64];						\
+		trace_facility("[%s] " f, tty_name(tty, buf), ##args);	\
+	} while (0)

  #undef N_TTY_TRACE
  #ifdef N_TTY_TRACE
@@ -87,6 +93,15 @@
  # define n_tty_trace(f, args...)
  #endif

+#undef N_TTY_TRACE_ECHOES
+#define N_TTY_TRACE_ECHOES 1
+#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;
@@ -763,10 +778,19 @@ 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_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) == ECHO_OP_ERASE_TAB)
  				tail += 3;
-- 
1.8.1.2




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

* Re: tty/serial eating \n regression in 3.13-rc1
  2013-11-27 12:13 ` Peter Hurley
@ 2013-11-27 18:13   ` Jason Gunthorpe
  2013-11-28 15:26     ` >> On 11/26/2013 04:53 PM, Jason Gunthorpe wrote: Peter Hurley
  0 siblings, 1 reply; 9+ messages in thread
From: Jason Gunthorpe @ 2013-11-27 18:13 UTC (permalink / raw)
  To: Peter Hurley; +Cc: Greg Kroah-Hartman, Jiri Slaby, linux-serial, linux-kernel

On Wed, Nov 27, 2013 at 07:13:47AM -0500, Peter Hurley wrote:
> On 11/26/2013 04:53 PM, Jason Gunthorpe wrote:
> >Hello Peter,
> >
> >I have been testing 3.13-rc1 and I noticed a change in behavior in the
> >tty/serial layer. Specifically I have a login running on serial that
> >presents the usual login:/password: prompt.
> >
> >3.10.12 does this:
> >
> >login: admin
> >password:
> >prompt>
> >
> >While 3.13-rc1 does this:
> >
> >login: admin
> >password: prompt>
> >
> >That is to say, a \n got lost. I have also noticed while using the
> >serial console that '\n's are occasionally lost, and the lossage is
> >highly repeatable and seemingly based on number of chars being output
> >in a burst, eg entering 'foo bar' at a shell prompt might loose the \n,
> >while entering 'foo bar ' will not.
> >
> >I ran a full git bisect search and determined that the attached patch
> >introduced the problem.
> >
> >This seems like a bug - at least the commit comment seems like it
> >should not cause any change in behaviour. I can reproduce it and help
> >debug, if you can give some guidance, I am not a tty layer expert :)
> 
> Hi Jason,
> 
> Thanks for the bisect: this is definitely a bug and not designed behavior.
> But just to be clear here, the error you're seeing is strictly wrt.
> echo output; normal output remains uncorrupted?

I have only seen this in two instances, in each case it happens 100%
of the time, with perfect determinism.

The first is '\n' after 'password:'

The second is '\n' after entering a command in busybox's shell, and in
this case, so far only with one specific command. 'cat enable'.

Curiously 'cat enable ' does not cause the problem, inserting the
extra space seems suspiciously like a buffering math problem?

> Although there are some fixes due for 3.13-rc2 concerning echoes, I
> think this may be a smp weak memory ordering problem.

To clarify, the problematic kernel is running on ARM32 and PPC32 which
are both uniprocessor systems, and it happens 100% of the time, so 
I'd be surprised if it was something as subtle as memory ordering.

The other end of the serial is running Ubuntu's 3.5.0-32-generic,
however I have ssh'd into that machine from one running Ubuntu's
3.11.0-12-generic - and that environment doesn't change.

> When you see the lost '\n', copy /sys/kernel/debug/tracing/trace
> to a file and send it to me (compressed if too big for the lists). Or
> you can send it off-list, if you'd prefer.
> 
> [Apologies if the patch is white-space mangled; my mailer [Thunderbird]
> is dain-bramaged. I think git am will still do the right thing.]

I corrected the whitespace damage by hand, I'm happy with MIME if you
need to send another patch :)

Here is the trace of the login/password sequence, I typed into the
serial the sequence '\nadmin\nadmin\n'

# tracer: nop
#
# entries-in-buffer/entries-written: 7/7   #P:1
#
#                              _-----=> irqs-off
#                             / _----=> need-resched
#                            | / _---=> hardirq/softirq
#                            || / _--=> preempt-depth
#                            ||| /     delay
#           TASK-PID   CPU#  ||||    TIMESTAMP  FUNCTION
#              | |       |   ||||       |         |
     kworker/0:1-11    [000] ....    24.912264: __process_echoes: [ttyS0] echoes:0,1 commit:1 head:1
     kworker/0:1-11    [000] ....    25.448097: __process_echoes: [ttyS0] echoes:1,4 commit:4 head:4
     kworker/0:1-11    [000] ....    25.527935: __process_echoes: [ttyS0] echoes:4,5 commit:5 head:5
     kworker/0:1-11    [000] ....    25.591953: __process_echoes: [ttyS0] echoes:5,6 commit:6 head:6
     kworker/0:1-11    [000] ....    25.695950: __process_echoes: [ttyS0] echoes:6,7 commit:7 head:7
     kworker/0:1-11    [000] ....    25.799939: __process_echoes: [ttyS0] echoes:7,8 commit:8 head:8
     kworker/0:1-11    [000] ....    25.928091: __process_echoes: [ttyS0] echoes:8,9 commit:9 head:9

Here is a strace of the terminal program I am running on Ubuntu:

.. passwo
read(3, "r", 1)                         = 1
read(3, "d", 1)                         = 1
read(3, ":", 1)                         = 1
read(3, " ", 1)                         = 1
write(3, "a", 1)                        = 1
write(3, "d", 1)                        = 1
write(3, "m", 1)                        = 1
write(3, "i", 1)                        = 1
write(3, "n", 1)                        = 1
write(3, "\r", 1)                       = 1
read(3, "p", 1)                         = 1
read(3, "r", 1)                         = 1
read(3, "o", 1)                         = 1
.. mpt>

Here is a strace of the login programm running on the PPC

write(1, "password: ", 10)              = 10
ioctl(0, SNDCTL_TMR_START or SNDRV_TIMER_IOCTL_TREAD or TCSETS, {B115200 opost isig icanon -echo ...}) = 0
poll([{fd=0, events=POLLIN}], 1, 60000) = 1 ([{fd=0, revents=POLLIN}])
read(0, "admin\n", 300)                 = 6
ioctl(0, SNDCTL_TMR_START or SNDRV_TIMER_IOCTL_TREAD or TCSETS, {B115200 opost isig icanon echo ...}) = 0
[..]
write(1, "prompt>", 7)

Also, I discovered this sequence, which seems very telling:

login: admin
password: Invalid login.
login: 
a

I entered the sequence 'admin\na\na', which does this in 3.10:

login: admin
password:
Invalid login.
login: a

It looks like The \n that was supposed to precede 'Invalid login' was shifted until
the next input char 'a'. The cursor remains at 'login: X' then when I
type 'a' I get back '\na' - I hope this is clear :)

strace of login for the above:

read(0, "admin\n", 300)                 = 6
write(1, "password: ", 10)              = 10
ioctl(0, SNDCTL_TMR_START or SNDRV_TIMER_IOCTL_TREAD or TCSETS, {B115200 opost isig icanon -echo ...}) = 0
poll([{fd=0, events=POLLIN}], 1, 60000) = 1 ([{fd=0, revents=POLLIN}])
read(0, "a\n", 300)                     = 2
ioctl(0, SNDCTL_TMR_START or SNDRV_TIMER_IOCTL_TREAD or TCSETS, {B115200 opost isig icanon echo ...}) = 0
socket(PF_FILE, SOCK_DGRAM|SOCK_CLOEXEC, 0) = 4
nanosleep({1, 0}, NULL)                 = 0
write(1, "Invalid login.\n", 15)        = 15
write(1, "login: ", 7)          

Terminal is cooked, so the 'a' char doesn't reach login, the '\na'
echo back was created in the kernel.

Looks like the \n stuff when echo is turned off is broken?

Jason

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

* >> On 11/26/2013 04:53 PM, Jason Gunthorpe wrote:
  2013-11-27 18:13   ` Jason Gunthorpe
@ 2013-11-28 15:26     ` Peter Hurley
  2013-11-28 17:46       ` tty/serial eating \n regression in 3.13-rc1 Jason Gunthorpe
  0 siblings, 1 reply; 9+ messages in thread
From: Peter Hurley @ 2013-11-28 15:26 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Greg Kroah-Hartman, Jiri Slaby, linux-kernel, linux-serial, Peter Hurley

Jason,

Seriously, thanks for all the diagnostic info. The strace led me right
to the problem.

Please test out the attached patch and let me know if that solves
the login whitespace problem.

Also, please retest the 'cat enable' problem; I have doubts that this
patch fixes that problem, though.

PS - sending this via git send-email so whitespace shouldn't be
a problem.

--- >% ---
Subject: [PATCH] n_tty: Fix missing newline echo

When L_ECHONL is on, newlines are echoed regardless of the L_ECHO
state; if set, ensure accumulated echoes are flushed before finishing
the current input processing and before more output.

Reported-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/n_tty.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index ac8dfe6..3ab928f 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -855,7 +855,8 @@ static void process_echoes(struct tty_struct *tty)
 	struct n_tty_data *ldata = tty->disc_data;
 	size_t echoed;
 
-	if (!L_ECHO(tty) || ldata->echo_commit == ldata->echo_tail)
+	if ((!L_ECHO(tty) && !L_ECHONL(tty)) ||
+	    ldata->echo_commit == ldata->echo_tail)
 		return;
 
 	mutex_lock(&ldata->output_lock);
@@ -870,7 +871,8 @@ static void flush_echoes(struct tty_struct *tty)
 {
 	struct n_tty_data *ldata = tty->disc_data;
 
-	if (!L_ECHO(tty) || ldata->echo_commit == ldata->echo_head)
+	if ((!L_ECHO(tty) && !L_ECHONL(tty)) ||
+	    ldata->echo_commit == ldata->echo_head)
 		return;
 
 	mutex_lock(&ldata->output_lock);
-- 
1.8.1.2


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

* Re: tty/serial eating \n regression in 3.13-rc1
  2013-11-28 15:26     ` >> On 11/26/2013 04:53 PM, Jason Gunthorpe wrote: Peter Hurley
@ 2013-11-28 17:46       ` Jason Gunthorpe
  2013-11-29 12:39         ` Peter Hurley
  0 siblings, 1 reply; 9+ messages in thread
From: Jason Gunthorpe @ 2013-11-28 17:46 UTC (permalink / raw)
  To: Peter Hurley; +Cc: Greg Kroah-Hartman, Jiri Slaby, linux-kernel, linux-serial

On Thu, Nov 28, 2013 at 10:26:57AM -0500, Peter Hurley wrote:

> Please test out the attached patch and let me know if that solves
> the login whitespace problem.

Yes, it certainly does:
Tested-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>

My bisection search showed this should go into 3.12 stable and 3.13..

> Also, please retest the 'cat enable' problem; I have doubts that this
> patch fixes that problem, though.

Correct, this case is still broken, I had guessed they were the same..

Hmm.. it behaves that way in 3.10 as well.. And the issue happens when
the line exceeds 80 cols, and things visually look sane if my x
terminal is 80 cols. So this is something different and probably
expected behavior.

Thanks!
Jason

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

* Re: tty/serial eating \n regression in 3.13-rc1
  2013-11-28 17:46       ` tty/serial eating \n regression in 3.13-rc1 Jason Gunthorpe
@ 2013-11-29 12:39         ` Peter Hurley
  2013-11-29 17:32           ` Greg Kroah-Hartman
  0 siblings, 1 reply; 9+ messages in thread
From: Peter Hurley @ 2013-11-29 12:39 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Jason Gunthorpe, Jiri Slaby, linux-kernel, linux-serial

On 11/28/2013 12:46 PM, Jason Gunthorpe wrote:
> On Thu, Nov 28, 2013 at 10:26:57AM -0500, Peter Hurley wrote:
>
>> Please test out the attached patch and let me know if that solves
>> the login whitespace problem.
>
> Yes, it certainly does:
> Tested-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
>
> My bisection search showed this should go into 3.12 stable and 3.13..

Greg,

Please let me know if you need me to resend the earlier patch with
Jason's Tested-by and a proper Cc to stable.

Regards,
Peter Hurley


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

* Re: tty/serial eating \n regression in 3.13-rc1
  2013-11-29 12:39         ` Peter Hurley
@ 2013-11-29 17:32           ` Greg Kroah-Hartman
  2013-11-29 17:56               ` Peter Hurley
  0 siblings, 1 reply; 9+ messages in thread
From: Greg Kroah-Hartman @ 2013-11-29 17:32 UTC (permalink / raw)
  To: Peter Hurley; +Cc: Jason Gunthorpe, Jiri Slaby, linux-kernel, linux-serial

On Fri, Nov 29, 2013 at 07:39:45AM -0500, Peter Hurley wrote:
> On 11/28/2013 12:46 PM, Jason Gunthorpe wrote:
> > On Thu, Nov 28, 2013 at 10:26:57AM -0500, Peter Hurley wrote:
> >
> >> Please test out the attached patch and let me know if that solves
> >> the login whitespace problem.
> >
> > Yes, it certainly does:
> > Tested-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> >
> > My bisection search showed this should go into 3.12 stable and 3.13..
> 
> Greg,
> 
> Please let me know if you need me to resend the earlier patch with
> Jason's Tested-by and a proper Cc to stable.

That would be nice to have, thanks :)

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

* [PATCH] n_tty: Fix missing newline echo
  2013-11-29 17:32           ` Greg Kroah-Hartman
@ 2013-11-29 17:56               ` Peter Hurley
  0 siblings, 0 replies; 9+ messages in thread
From: Peter Hurley @ 2013-11-29 17:56 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Jason Gunthorpe, Jiri Slaby, linux-serial, linux-kernel,
	Peter Hurley, stable

When L_ECHONL is on, newlines are echoed regardless of the L_ECHO
state; if set, ensure accumulated echoes are flushed before finishing
the current input processing and before more output.

Cc: <stable@vger.kernel.org> # 3.12.x
Reported-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com
Tested-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/n_tty.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index ac8dfe6..3ab928f 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -855,7 +855,8 @@ static void process_echoes(struct tty_struct *tty)
 	struct n_tty_data *ldata = tty->disc_data;
 	size_t echoed;
 
-	if (!L_ECHO(tty) || ldata->echo_commit == ldata->echo_tail)
+	if ((!L_ECHO(tty) && !L_ECHONL(tty)) ||
+	    ldata->echo_commit == ldata->echo_tail)
 		return;
 
 	mutex_lock(&ldata->output_lock);
@@ -870,7 +871,8 @@ static void flush_echoes(struct tty_struct *tty)
 {
 	struct n_tty_data *ldata = tty->disc_data;
 
-	if (!L_ECHO(tty) || ldata->echo_commit == ldata->echo_head)
+	if ((!L_ECHO(tty) && !L_ECHONL(tty)) ||
+	    ldata->echo_commit == ldata->echo_head)
 		return;
 
 	mutex_lock(&ldata->output_lock);
-- 
1.8.1.2


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

* [PATCH] n_tty: Fix missing newline echo
@ 2013-11-29 17:56               ` Peter Hurley
  0 siblings, 0 replies; 9+ messages in thread
From: Peter Hurley @ 2013-11-29 17:56 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Jason Gunthorpe, Jiri Slaby, linux-serial, linux-kernel,
	Peter Hurley, stable

When L_ECHONL is on, newlines are echoed regardless of the L_ECHO
state; if set, ensure accumulated echoes are flushed before finishing
the current input processing and before more output.

Cc: <stable@vger.kernel.org> # 3.12.x
Reported-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com
Tested-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/n_tty.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index ac8dfe6..3ab928f 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -855,7 +855,8 @@ static void process_echoes(struct tty_struct *tty)
 	struct n_tty_data *ldata = tty->disc_data;
 	size_t echoed;
 
-	if (!L_ECHO(tty) || ldata->echo_commit == ldata->echo_tail)
+	if ((!L_ECHO(tty) && !L_ECHONL(tty)) ||
+	    ldata->echo_commit == ldata->echo_tail)
 		return;
 
 	mutex_lock(&ldata->output_lock);
@@ -870,7 +871,8 @@ static void flush_echoes(struct tty_struct *tty)
 {
 	struct n_tty_data *ldata = tty->disc_data;
 
-	if (!L_ECHO(tty) || ldata->echo_commit == ldata->echo_head)
+	if ((!L_ECHO(tty) && !L_ECHONL(tty)) ||
+	    ldata->echo_commit == ldata->echo_head)
 		return;
 
 	mutex_lock(&ldata->output_lock);
-- 
1.8.1.2


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

end of thread, other threads:[~2013-11-29 17:56 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-11-26 21:53 tty/serial eating \n regression in 3.13-rc1 Jason Gunthorpe
2013-11-27 12:13 ` Peter Hurley
2013-11-27 18:13   ` Jason Gunthorpe
2013-11-28 15:26     ` >> On 11/26/2013 04:53 PM, Jason Gunthorpe wrote: Peter Hurley
2013-11-28 17:46       ` tty/serial eating \n regression in 3.13-rc1 Jason Gunthorpe
2013-11-29 12:39         ` Peter Hurley
2013-11-29 17:32           ` Greg Kroah-Hartman
2013-11-29 17:56             ` [PATCH] n_tty: Fix missing newline echo Peter Hurley
2013-11-29 17:56               ` Peter Hurley

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.