Commit Graph

99 Commits

Author SHA1 Message Date
Peter Hurley 93a8d4163e n_tty: Fix EOF push index when termios changes
Commit 40d5e0905a,
'n_tty: Fix EOF push handling' introduced a subtle state
change error wrt EOF push handling when the termios is
changed from non-canonical to canonical mode.

Reset line_start to the current read_tail index, not 0.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2013-09-17 10:10:51 -07:00
Peter Hurley aefceaf453 n_tty: Fix termios_rwsem lockdep false positive
Lockdep reports a circular lock dependency between
atomic_read_lock and termios_rwsem [1]. However, a lock
order deadlock is not possible since CPU1 only holds a
read lock which cannot prevent CPU0 from also acquiring
a read lock on the same r/w semaphore.

Unfortunately, lockdep cannot currently distinguish whether
the locks are read or write for any particular lock graph,
merely that the locks _were_ previously read and/or write.

Until lockdep is fixed, re-order atomic_read_lock so
termios_rwsem can be dropped and reacquired without
triggering lockdep.

Patch based on original posted here https://lkml.org/lkml/2013/8/1/510
by Sergey Senozhatsky <sergey.senozhatsky@gmail.com>

[1] Initial lockdep report from Artem Savkov <artem.savkov@gmail.com>

 ======================================================
 [ INFO: possible circular locking dependency detected ]
 3.11.0-rc3-next-20130730+ #140 Tainted: G        W
 -------------------------------------------------------
 bash/1198 is trying to acquire lock:
  (&tty->termios_rwsem){++++..}, at: [<ffffffff816aa3bb>] n_tty_read+0x49b/0x660

 but task is already holding lock:
  (&ldata->atomic_read_lock){+.+...}, at: [<ffffffff816aa0f0>] n_tty_read+0x1d0/0x660

 which lock already depends on the new lock.

 the existing dependency chain (in reverse order) is:

 -> #1 (&ldata->atomic_read_lock){+.+...}:
        [<ffffffff811111cc>] validate_chain+0x73c/0x850
        [<ffffffff811117e0>] __lock_acquire+0x500/0x5d0
        [<ffffffff81111a29>] lock_acquire+0x179/0x1d0
        [<ffffffff81d34b9c>] mutex_lock_interruptible_nested+0x7c/0x540
        [<ffffffff816aa0f0>] n_tty_read+0x1d0/0x660
        [<ffffffff816a3bb6>] tty_read+0x86/0xf0
        [<ffffffff811f21d3>] vfs_read+0xc3/0x130
        [<ffffffff811f2702>] SyS_read+0x62/0xa0
        [<ffffffff81d45259>] system_call_fastpath+0x16/0x1b

 -> #0 (&tty->termios_rwsem){++++..}:
        [<ffffffff8111064f>] check_prev_add+0x14f/0x590
        [<ffffffff811111cc>] validate_chain+0x73c/0x850
        [<ffffffff811117e0>] __lock_acquire+0x500/0x5d0
        [<ffffffff81111a29>] lock_acquire+0x179/0x1d0
        [<ffffffff81d372c1>] down_read+0x51/0xa0
        [<ffffffff816aa3bb>] n_tty_read+0x49b/0x660
        [<ffffffff816a3bb6>] tty_read+0x86/0xf0
        [<ffffffff811f21d3>] vfs_read+0xc3/0x130
        [<ffffffff811f2702>] SyS_read+0x62/0xa0
        [<ffffffff81d45259>] system_call_fastpath+0x16/0x1b

 other info that might help us debug this:

  Possible unsafe locking scenario:

        CPU0                    CPU1
        ----                    ----
   lock(&ldata->atomic_read_lock);
                                lock(&tty->termios_rwsem);
                                lock(&ldata->atomic_read_lock);
   lock(&tty->termios_rwsem);

  *** DEADLOCK ***

 2 locks held by bash/1198:
  #0:  (&tty->ldisc_sem){.+.+.+}, at: [<ffffffff816ade04>] tty_ldisc_ref_wait+0x24/0x60
  #1:  (&ldata->atomic_read_lock){+.+...}, at: [<ffffffff816aa0f0>] n_tty_read+0x1d0/0x660

 stack backtrace:
 CPU: 1 PID: 1198 Comm: bash Tainted: G        W    3.11.0-rc3-next-20130730+ #140
 Hardware name: Bochs Bochs, BIOS Bochs 01/01/2007
  0000000000000000 ffff880019acdb28 ffffffff81d34074 0000000000000002
  0000000000000000 ffff880019acdb78 ffffffff8110ed75 ffff880019acdb98
  ffff880019fd0000 ffff880019acdb78 ffff880019fd0638 ffff880019fd0670
 Call Trace:
  [<ffffffff81d34074>] dump_stack+0x59/0x7d
  [<ffffffff8110ed75>] print_circular_bug+0x105/0x120
  [<ffffffff8111064f>] check_prev_add+0x14f/0x590
  [<ffffffff81d3ab5f>] ? _raw_spin_unlock_irq+0x4f/0x70
  [<ffffffff811111cc>] validate_chain+0x73c/0x850
  [<ffffffff8110ae0f>] ? trace_hardirqs_off_caller+0x1f/0x190
  [<ffffffff811117e0>] __lock_acquire+0x500/0x5d0
  [<ffffffff81111a29>] lock_acquire+0x179/0x1d0
  [<ffffffff816aa3bb>] ? n_tty_read+0x49b/0x660
  [<ffffffff81d372c1>] down_read+0x51/0xa0
  [<ffffffff816aa3bb>] ? n_tty_read+0x49b/0x660
  [<ffffffff816aa3bb>] n_tty_read+0x49b/0x660
  [<ffffffff810e4130>] ? try_to_wake_up+0x210/0x210
  [<ffffffff816a3bb6>] tty_read+0x86/0xf0
  [<ffffffff811f21d3>] vfs_read+0xc3/0x130
  [<ffffffff811f2702>] SyS_read+0x62/0xa0
  [<ffffffff815e24ee>] ? trace_hardirqs_on_thunk+0x3a/0x3f
  [<ffffffff81d45259>] system_call_fastpath+0x16/0x1b

Reported-by: Artem Savkov <artem.savkov@gmail.com>
Reported-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2013-08-12 11:10:17 -07:00
Peter Hurley e60d27c4d8 n_tty: Factor LNEXT processing from per-char i/o path
LNEXT processing accounts for ~15% of total cpu time in end-to-end
tty i/o; factor the lnext test/clear from the per-char i/o path.

Instead, attempt to immediately handle the literal next char if not
at the end of this received buffer; otherwise, handle the first char
of the next received buffer as the literal next char, then continue
with normal i/o.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2013-07-24 09:29:32 -07:00
Peter Hurley 4b293492ae n_tty: Un-inline single-use functions
gcc will likely inline these single-use functions anyway; remove
inline modifier.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2013-07-24 09:29:12 -07:00
Peter Hurley 19e2ad6a09 n_tty: Remove overflow tests from receive_buf() path
Always pre-figure the space available in the read_buf and limit
the inbound receive request to that amount.

For compatibility reasons with the non-flow-controlled interface,
n_tty_receive_buf() will continue filling read_buf until all data
has been received or receive_room() returns 0.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2013-07-24 09:28:52 -07:00
Peter Hurley 7de971b050 n_tty: Factor PARMRK from normal per-char i/o
Handle PARMRK processing on the slow per-char i/o path.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2013-07-24 09:28:16 -07:00
Peter Hurley 6baad00867 n_tty: Factor ISTRIP and IUCLC receive_buf into separate fn
Convert to modal receive_buf processing; factor char receive
processing for unusual termios settings out of normal per-char
i/o path.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2013-07-24 09:27:55 -07:00
Peter Hurley 4b1f79c2d7 n_tty: Split n_tty_receive_char()
Factor 'special' per-char processing into standalone fn,
n_tty_receive_char_special(), which handles processing for chars
marked in the char_map.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2013-07-24 09:27:22 -07:00
Peter Hurley 855df3c089 n_tty: Eliminate char tests from IXANY restart test
Relocate the IXANY restart tty test to code paths where the
the received char is not START_CHAR, STOP_CHAR, INTR_CHAR,
QUIT_CHAR or SUSP_CHAR.

Fixes the condition when ISIG if off and one of INTR_CHAR,
QUIT_CHAR or SUSP_CHAR does not restart i/o.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2013-07-24 09:26:51 -07:00
Peter Hurley 7d88d637a3 n_tty: Factor standard per-char i/o into separate fn
Simplify __receive_buf() into a dispatch function; perform per-char
processing for all other modes not already handled.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2013-07-24 09:26:51 -07:00
Peter Hurley 86e35aea47 n_tty: Fix build breakage on ppc64
Commit 20bafb3d23
  'n_tty: Move buffers into n_tty_data'
broke the ppc64 build.

Include vmalloc.h for the required function declarations.

Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2013-07-24 09:20:21 -07:00
Peter Hurley ad0cc7bafe n_tty: Factor tty->closing receive_buf() into separate fn
Convert to modal receive_buf() processing; factor receive char
processing when tty->closing into n_tty_receive_buf_closing().

Note that EXTPROC when ISTRIP or IUCLC is set continues to be
handled by n_tty_receive_char().

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2013-07-23 17:11:02 -07:00
Peter Hurley a1dd30e9b4 n_tty: Special case EXTPROC receive_buf() as raw mode
When EXTPROC is set without ISTRIP or IUCLC, processing is
identical to raw mode; handle this receiving mode as a special-case
of raw mode.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2013-07-23 17:11:02 -07:00
Peter Hurley 554117bdc8 n_tty: Factor raw mode receive_buf() into separate fn
Convert to modal receive_buf() processing; factor raw mode
per-char i/o into n_tty_receive_buf_raw().

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2013-07-23 17:10:17 -07:00
Peter Hurley d2f8d7abd1 n_tty: Factor flagged char handling into separate fn
Prepare for modal receive_buf() handling; factor handling for
TTY_BREAK, TTY_PARITY, TTY_FRAME and TTY_OVERRUN into
n_tty_receive_char_flagged().

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2013-07-23 17:10:17 -07:00
Peter Hurley b0ac50be1f n_tty: Factor signal char handling into separate fn
Reduce the monolithic n_tty_receive_char() complexity; factor the
handling of INTR_CHAR, QUIT_CHAR and SUSP_CHAR into
n_tty_receive_signal_char().

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2013-07-23 17:10:17 -07:00
Peter Hurley 4a23a4df50 n_tty: Factor 'real raw' receive_buf into standalone fn
Convert to modal receive_buf() processing; factor real_raw
receive_buf() into n_tty_receive_buf_real_raw().

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2013-07-23 17:10:17 -07:00
Peter Hurley 781ad1c793 n_tty: Simplify __receive_buf loop count
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2013-07-23 17:08:40 -07:00
Peter Hurley 1bb9d56285 n_tty: Rename process_char_map to char_map
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2013-07-23 17:08:40 -07:00
Peter Hurley 20bafb3d23 n_tty: Move buffers into n_tty_data
Reduce pointer reloading and improve locality-of-reference;
allocate read_buf and echo_buf within struct n_tty_data.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2013-07-23 17:08:40 -07:00
Peter Hurley 8cb06c9838 n_tty: Remove alias ptrs in __receive_buf()
The char and flag buffer local alias pointers, p and f, are
unnecessary; remove them.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2013-07-23 17:08:40 -07:00
Peter Hurley 40d5e0905a n_tty: Fix EOF push handling
In canonical mode, an EOF which is not the first character of the line
causes read() to complete and return the number of characters read so
far (commonly referred to as EOF push). However, if the previous read()
returned because the user buffer was full _and_ the next character
is an EOF not at the beginning of the line, read() must not return 0,
thus mistakenly indicating the end-of-file condition.

The TTY_PUSH flag is used to indicate an EOF was received which is not
at the beginning of the line. Because the EOF push condition is
evaluated by a thread other than the read(), multiple EOF pushes can
cause a premature end-of-file to be indicated.

Instead, discover the 'EOF push as first read character' condition
from the read() thread itself, and restart the i/o loop if detected.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2013-07-23 17:08:40 -07:00
Peter Hurley 9dfd16ddea n_tty: Avoid false-sharing echo buffer indices
Separate the head & commit indices from the tail index to avoid
cache-line contention (so called 'false-sharing') between concurrent
threads.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2013-07-23 17:02:22 -07:00
Peter Hurley 29c7c5ca36 n_tty: Eliminate counter in __process_echoes
Since neither echo_commit nor echo_tail can change for the duration
of __process_echoes loop, substitute index comparison for the
snapshot counter.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2013-07-23 17:02:22 -07:00
Peter Hurley bc5b1ec586 n_tty: Only flush echo output if actually output
Don't have the driver flush received echoes if no echoes were
actually output.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2013-07-23 17:02:22 -07:00