Commit Graph

76 Commits

Author SHA1 Message Date
Peter Hurley 17a6921971 tty: Fix direct use of tty buffer work
Recent abstraction of tty buffer work introduced api to manage
tty input kworker; use it.

Fixes: e176058f0d ("tty: Abstract tty buffer work")
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2015-11-20 16:19:54 -08:00
Guillaume Gomez 52772ea615 tty: remove unneeded return statement
Signed-off-by: Guillaume Gomez <guillaume1.gomez@gmail.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2015-10-17 21:29:21 -07:00
Peter Hurley fb6edc9115 tty: core: Improve ldisc debug messages
Add debug messages for ldisc open and close, and remove
"closing ldisc" message from tty_ldisc_release(), because a
close message is now printed for both ldiscs; always print ldisc
pointer first so ldisc changes are easier to identify.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2015-07-23 18:37:31 -07:00
Peter Hurley 0a6adc131c tty: Use tty_debug() for tty_ldisc_debug()
Replace tty_ldisc_debug() macro definition; substitute with equivalent
tty_debug() invocation.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2015-07-23 18:37:31 -07:00
Rasmus Villemoes 429b474990 tty: remove buf parameter from tty_name()
tty_name no longer uses the buf parameter, so remove it along with all
the 64 byte stack buffers that used to be passed in.

Mostly generated by the coccinelle script

@depends on patch@
identifier buf;
constant C;
expression tty;
@@
- char buf[C];
  <+...
- tty_name(tty, buf)
+ tty_name(tty)
  ...+>

allmodconfig compiles, so I'm fairly confident the stack buffers
weren't used for other purposes as well.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Reviewed-by: Peter Hurley <peter@hurleysoftware.com>
Acked-by: Jesper Nilsson <jesper.nilsson@axis.com>
Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2015-05-06 22:26:57 +02:00
Peter Hurley 9191aaaa82 tty: Allow safe access to termios for set_ldisc() handlers
Allow a tty driver to safely access termios settings while handling
the set_ldisc() notification. UART drivers use the set_ldisc()
notification to check if the N_PPS line discipline is being enabled;
if so, modem status interrupts may also need to be enabled. Conversely,
modem status interrupts may need to be disabled if switching away
from the N_PPS line discipline.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2014-11-06 14:57:27 -08:00
Peter Hurley 86c80a8e2a tty: Flush ldisc buffer atomically with tty flip buffers
tty_ldisc_flush() first clears the line discipline input buffer,
then clears the tty flip buffers. However, this allows for existing
data in the tty flip buffers to be added after the ldisc input
buffer has been cleared, but before the flip buffers have been cleared.

Add an optional ldisc parameter to tty_buffer_flush() to allow
tty_ldisc_flush() to pass the ldisc to clear.

NB: Initially, the plan was to do this automatically in
tty_buffer_flush(). However, an audit of the behavior of existing
line disciplines showed that performing a ldisc buffer flush on
ioctl(TCFLSH) was not always the outcome. For example, some line
disciplines have flush_buffer() methods but not ioctl() methods,
so a ->flush_buffer() command would be unexpected.

Reviewed-by: Alan Cox <alan@linux.intel.com>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2014-11-05 18:50:43 -08:00
Peter Hurley 276a661a4d tty: Fix timeout on pty set ldisc
When changing the ldisc on one end of a pty pair, there may be
waiting readers/writers on the other end which may not exit from
the ldisc i/o loop, preventing tty_ldisc_lock_pair_timeout() from
acquiring the other side's ldisc lock.

Only acquire this side's ldisc lock; although this will no longer
prevent the other side from writing new input, that input will not
be processed until after the ldisc change completes. This has no
effect on normal ttys; new input from the driver was never disabled.

Remove tty_ldisc_enable_pair().

Reviewed-by: Alan Cox <alan@linux.intel.com>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2014-11-05 18:50:43 -08:00
Peter Hurley fae76e9adf tty: Fix hung task on pty hangup
When hanging up one end of a pty pair, there may be waiting
readers/writers on the other end which may not exit, preventing
tty_ldisc_lock_pair() from acquiring the other side's ldisc lock.

Only acquire this side's ldisc lock; although this will no longer
prevent the other side from writing new input, that input will not
be processing until after the ldisc hangup is complete.

Reported-by: Sasha Levin <sasha.levin@oracle.com>
Reviewed-by: Alan Cox <alan@linux.intel.com>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2014-11-05 18:50:43 -08:00
Peter Hurley e80a10ee4d tty: Prefix tty_ldisc_{lock,lock_nested,unlock} functions
tty_ldisc_lock(), tty_ldisc_unlock(), and tty_ldisc_lock_nested()
are low-level aliases for the underlying lock mechanism. Rename
with double underscore to allow for new, higher level functions
with those names.

Reviewed-by: Alan Cox <alan@linux.intel.com>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2014-11-05 18:50:42 -08:00
Peter Hurley 62462aefeb tty: Simplify tty_ldisc_release() interface
Passing the 'other' tty to tty_ldisc_release() only makes sense
for a pty pair; make o_tty function local instead.

Reviewed-by: Alan Cox <alan@linux.intel.com>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2014-11-05 18:42:46 -08:00
Peter Hurley 3ff51a199f tty: Remove TTY_HUPPING
Now that tty_ldisc_hangup() does not drop the tty lock, it is no
longer possible to observe TTY_HUPPING while holding the tty lock
on another cpu.

Remove TTY_HUPPING bit definition.

Reviewed-by: Alan Cox <alan@linux.intel.com>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2014-11-05 18:24:10 -08:00
Peter Hurley c8483bc9de tty: Invert tty_lock/ldisc_sem lock order
Dropping the tty lock to acquire the tty->ldisc_sem allows several
race conditions (such as hangup while changing the ldisc) which requires
extra states and testing. The ldisc_sem->tty_lock lock order has
not been required since tty buffer ownership was moved to tty_port.

Reviewed-by: Alan Cox <alan@linux.intel.com>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2014-11-05 18:24:10 -08:00
Peter Hurley 3ee175d9d9 tty: Don't hold tty_lock for ldisc release
The tty->ldisc_sem write lock is sufficient for serializing changes
to tty->ldisc; holding the tty lock is not required.

Reviewed-by: Alan Cox <alan@linux.intel.com>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2014-11-05 18:24:10 -08:00
Paul Gortmaker f8e87cb4a1 tty: delete non-required instances of include <linux/init.h>
None of these files are actually using any __init type directives
and hence don't need to include <linux/init.h>.  Most are just a
left over from __devinit and __cpuinit removal, or simply due to
code getting copied from one driver to the next.

Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2014-01-07 17:05:21 -08:00
Peter Hurley 6a1c0680cf tty: Convert termios_mutex to termios_rwsem
termios is commonly accessed unsafely (especially by N_TTY)
because the existing mutex forces exclusive access.
Convert existing usage.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2013-07-23 16:43:01 -07:00
Peter Hurley 3b80df7ca7 tty: Don't change receive_room for ioctl(TIOCSETD)
tty_set_ldisc() is guaranteed exclusive use of the line discipline
by tty_ldisc_lock_pair_timeout(); shutting off input by resetting
receive_room is unnecessary.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2013-07-23 16:42:59 -07:00
Peter Hurley b0e9585823 tty: Clarify multiple-references comment in TIOCSETD ioctl
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2013-07-23 16:38:35 -07:00
Peter Hurley e97733ca67 tty: Fix hangup race with TIOCSETD ioctl
The hangup may already have happened; check for that state also.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2013-07-23 16:38:35 -07:00
Peter Hurley 9fbfa34c18 tty: Clarify ldisc variable
Rename o_ldisc to avoid confusion with the ldisc of the
'other' tty.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2013-07-23 16:38:35 -07:00
Peter Hurley 36697529b5 tty: Replace ldisc locking with ldisc_sem
Line discipline locking was performed with a combination of
a mutex, a status bit, a count, and a waitqueue -- basically,
a rw semaphore.

Replace the existing combination with an ld_semaphore.

Fixes:
 1) the 'reference acquire after ldisc locked' bug
 2) the over-complicated halt mechanism
 3) lock order wrt. tty_lock()
 4) dropping locks while changing ldisc
 5) previously unidentified deadlock while locking ldisc from
    both linked ttys concurrently
 6) previously unidentified recursive deadlocks

Adds much-needed lockdep diagnostics.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2013-07-23 16:38:34 -07:00
Peter Hurley d2c438905f tty: Add lock/unlock ldisc pair functions
Just as the tty pair must be locked in a stable sequence
(ie, independent of which is consider the 'other' tty), so must
the ldisc pair be locked in a stable sequence as well.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2013-07-23 16:38:34 -07:00
Peter Hurley 137084bbad tty: Fix tty_ldisc_lock name collision
The file scope spinlock identifier, tty_ldisc_lock, will collide
with the file scope lock function tty_ldisc_lock() so rename it.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2013-07-23 16:38:34 -07:00
Peter Hurley be3971166d tty: Remove redundant tty_wait_until_sent()
tty_ioctl() already waits until sent.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2013-03-18 16:48:42 -07:00
Peter Hurley 734de249fb tty: Locate get/put ldisc functions together
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2013-03-18 16:48:42 -07:00
Peter Hurley f48070457c tty: Fold one-line assign function into callers
Now that tty_ldisc_assign() is a one-line file-scoped function,
remove it and perform the simple assignment at its call sites.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2013-03-18 16:48:42 -07:00
Peter Hurley 16759f6cd8 tty: Document unsafe ldisc reference acquire
Merge get_ldisc() into its only call site.
Note how, after merging, the unsafe acquire of an ldisc reference
is obvious.

   CPU 0 in tty_ldisc_try()         |  CPU 1 in tty_ldisc_halt()
                                    |
test_bit(TTY_LDISC, &tty_flags)     |
if (true)                           |  clear_bit(TTY_LDISC, &tty_flags)
  tty->ldisc != 0?                  |  atomic_read(&tty->ldisc->users)
  if (true)                         |  ret_val == 1?
    atomic_inc(&tty->ldisc->users)  |  if (false)
                                    |    wait
                                    |
<goes on assuming safe ldisc use>   |  <doesn't wait - proceeds w/ close>
                                    |

The spin lock in tty_ldisc_try() does nothing wrt synchronizing
the ldisc halt since it's not acquired as part of halting.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2013-03-18 16:48:42 -07:00
Peter Hurley ebc9baed42 tty: Separate release semantics of ldisc reference
tty_ldisc_ref()/tty_ldisc_unref() have usage semantics
equivalent to down_read_trylock()/up_read(). Only
callers of tty_ldisc_put() are performing the additional
operations necessary for proper ldisc teardown, and then only
after ensuring no outstanding 'read lock' remains.

Thus, tty_ldisc_unref() should never be the last reference;
WARN if it is. Conversely, tty_ldisc_put() should never be
destructing if the use count != 1.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2013-03-18 16:48:42 -07:00
Peter Hurley fc575ee6ea tty: Add ldisc hangup debug messages
Expected typical debug log:
[  582.721965] tty_open: opening pts3...
[  582.721970] tty_open: opening pts3...
[  582.721977] tty_release: pts3 (tty count=3)...
[  582.721980] tty_release: ptm3 (tty count=1)...
[  582.722015] pts3 vhangup...
[  582.722020] tty_ldisc_hangup: pts3: closing ldisc: ffff88007a920540
[  582.724128] tty_release: pts3 (tty count=2)...
[  582.724217] tty_ldisc_hangup: pts3: re-opened ldisc: ffff88007a920580
[  582.724221] tty_release: ptm3: final close
[  582.724234] tty_ldisc_release: ptm3: closing ldisc: ffff88007a920a80
[  582.724238] tty_ldisc_release: ptm3: ldisc closed
[  582.724241] tty_release: ptm3: freeing structure...
[  582.724741] tty_open: opening pts3...

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2013-03-18 16:46:30 -07:00
Peter Hurley c878524174 tty: Fix 'deferred reopen' ldisc comment
This comment is a victim of code migration from
"tty: Fix the ldisc hangup race"; re-parent it.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2013-03-18 16:44:02 -07:00
Peter Hurley a2965b7bee tty: Make core responsible for synchronizing its work
The tty core relies on the ldisc layer for synchronizing destruction
of the tty. Instead, the final tty release must wait for any pending tty
work to complete prior to tty destruction.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2013-03-18 16:44:02 -07:00
Peter Hurley 4f98d46751 tty: Complete ownership transfer of flip buffers
Waiting for buffer work to complete is not required for safely
performing changes to the line discipline, once the line discipline
is halted. The buffer work routine, flush_to_ldisc(), will be
unable to acquire an ldisc ref and all existing references were
waited until released (so it can't already have one).

Ensure running buffer work which may reference the soon-to-be-gone
tty completes and any buffer work running after this point retrieves
a NULL tty.

Also, ensure all buffer work is cancelled on port destruction.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2013-03-18 16:44:02 -07:00
Peter Hurley d912156605 tty: Don't reenable already enabled ldisc
tty_ldisc_hangup() guarantees the ldisc is enabled (or that there
is no ldisc). Since __tty_hangup() was the only user, re-define
tty_ldisc_enable() in file-scope.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2013-03-18 16:44:02 -07:00
Peter Hurley 977066e758 tty: Wait for SAK work before waiting for hangup work
SAK work may schedule hangup work (if TTY_SOFT_SAK is defined), thus
SAK work must be flushed before hangup work.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2013-03-18 16:38:58 -07:00
Peter Hurley f4cf7a3845 tty: Halt both ldiscs concurrently
The pty driver does not obtain an ldisc reference to the linked
tty when writing. When the ldiscs are sequentially halted, it
is possible for one ldisc to be halted, and before the second
ldisc can be halted, a concurrent write schedules buffer work on
the first ldisc. This can lead to an access-after-free error when
the scheduled buffer work starts on the closed ldisc.

Prevent subsequent use after halt by performing each stage
of the halt on both ttys.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2013-03-18 16:38:58 -07:00
Peter Hurley cf52847658 tty: Strengthen no-subsequent-use guarantee of tty_ldisc_halt()
In preparation for destructing and freeing the tty, the line discipline
must first be brought to an inactive state before it can be destructed.
This line discipline shutdown must:
 - disallow new users of the ldisc
 - wait for existing ldisc users to finish
 - only then, cancel/flush their pending/running work

Factor tty_ldisc_wait_idle() from tty_set_ldisc() and tty_ldisc_kill()
to ensure this shutdown order.

Failure to provide this guarantee can result in scheduled work
running after the tty has already been freed, as indicated in the
following log message:

[   88.331234] WARNING: at drivers/tty/tty_buffer.c:435 flush_to_ldisc+0x194/0x1d0()
[   88.334505] Hardware name: Bochs
[   88.335618] tty is bad=-1
[   88.335703] Modules linked in: netconsole configfs bnep rfcomm bluetooth ......
[   88.345272] Pid: 39, comm: kworker/1:1 Tainted: G        W    3.7.0-next-20121129+ttydebug-xeon #20121129+ttydebug
[   88.347736] Call Trace:
[   88.349024]  [<ffffffff81058aff>] warn_slowpath_common+0x7f/0xc0
[   88.350383]  [<ffffffff81058bf6>] warn_slowpath_fmt+0x46/0x50
[   88.351745]  [<ffffffff81432bd4>] flush_to_ldisc+0x194/0x1d0
[   88.353047]  [<ffffffff816f7fe1>] ? _raw_spin_unlock_irq+0x21/0x50
[   88.354190]  [<ffffffff8108a809>] ? finish_task_switch+0x49/0xe0
[   88.355436]  [<ffffffff81077ad1>] process_one_work+0x121/0x490
[   88.357674]  [<ffffffff81432a40>] ? __tty_buffer_flush+0x90/0x90
[   88.358954]  [<ffffffff81078c84>] worker_thread+0x164/0x3e0
[   88.360247]  [<ffffffff81078b20>] ? manage_workers+0x120/0x120
[   88.361282]  [<ffffffff8107e230>] kthread+0xc0/0xd0
[   88.362284]  [<ffffffff816f0000>] ? cmos_do_probe+0x2eb/0x3bf
[   88.363391]  [<ffffffff8107e170>] ? flush_kthread_worker+0xb0/0xb0
[   88.364797]  [<ffffffff816fff6c>] ret_from_fork+0x7c/0xb0
[   88.366087]  [<ffffffff8107e170>] ? flush_kthread_worker+0xb0/0xb0
[   88.367266] ---[ end trace 453a7c9f38fbfec0 ]---

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2013-03-18 16:38:58 -07:00
Peter Hurley 11cf48eab2 tty: Relocate tty_ldisc_halt() to avoid forward declaration
tty_ldisc_halt() will use the file-scoped function, tty_ldisc_wait_idle(),
in the following patch.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2013-03-18 16:38:58 -07:00
Peter Hurley 76bc35e78f tty: Fix ldisc halt sequence on hangup
Flip buffer work cannot be cancelled until all outstanding ldisc
references have been released. Convert the ldisc ref wait into
a full ldisc halt with buffer work cancellation.

Note that the legacy mutex is not held while cancelling.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2013-03-18 16:32:46 -07:00
Peter Hurley 2276ad9765 tty: Remove unnecessary re-test of ldisc ref count
Since the tty->ldisc is prevented from being changed by tty_set_ldisc()
when a tty is being hung up, re-testing the ldisc user count is
unnecessary -- ie, it cannot be a different ldisc and the user count
cannot have increased (assuming the caller meets the precondition that
TTY_LDISC flag is cleared)

Removal of the 'early-out' locking optimization is necessary for
the subsequent patch 'tty: Fix ldisc halt sequence on hangup'.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2013-03-18 16:32:46 -07:00
Peter Hurley 168942c9fa tty: Refactor wait for ldisc refs out of tty_ldisc_hangup()
Refactor tty_ldisc_hangup() to extract standalone function,
tty_ldisc_hangup_wait_idle(), to wait for ldisc references
to be released.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2013-03-18 16:32:46 -07:00
Peter Hurley 21622939fc tty: Add diagnostic for halted line discipline
Flip buffer work must not be scheduled by the line discipline
after the line discipline has been halted; issue warning.

Note: drivers can still schedule flip buffer work.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2013-03-18 16:32:46 -07:00
Sebastian Andrzej Siewior 852e4a8152 tty: don't deadlock while flushing workqueue
Since commit 89c8d91e31 ("tty: localise the lock") I see a dead lock
in one of my dummy_hcd + g_nokia test cases. The first run was usually
okay, the second often resulted in a splat by lockdep and the third was
usually a dead lock.
Lockdep complained about tty->hangup_work and tty->legacy_mutex taken
both ways:
| ======================================================
| [ INFO: possible circular locking dependency detected ]
| 3.7.0-rc6+ #204 Not tainted
| -------------------------------------------------------
| kworker/2:1/35 is trying to acquire lock:
|  (&tty->legacy_mutex){+.+.+.}, at: [<c14051e6>] tty_lock_nested+0x36/0x80
|
| but task is already holding lock:
|  ((&tty->hangup_work)){+.+...}, at: [<c104f6e4>] process_one_work+0x124/0x5e0
|
| which lock already depends on the new lock.
|
| the existing dependency chain (in reverse order) is:
|
| -> #2 ((&tty->hangup_work)){+.+...}:
|        [<c107fe74>] lock_acquire+0x84/0x190
|        [<c104d82d>] flush_work+0x3d/0x240
|        [<c12e6986>] tty_ldisc_flush_works+0x16/0x30
|        [<c12e7861>] tty_ldisc_release+0x21/0x70
|        [<c12e0dfc>] tty_release+0x35c/0x470
|        [<c1105e28>] __fput+0xd8/0x270
|        [<c1105fcd>] ____fput+0xd/0x10
|        [<c1051dd9>] task_work_run+0xb9/0xf0
|        [<c1002a51>] do_notify_resume+0x51/0x80
|        [<c140550a>] work_notifysig+0x35/0x3b
|
| -> #1 (&tty->legacy_mutex/1){+.+...}:
|        [<c107fe74>] lock_acquire+0x84/0x190
|        [<c140276c>] mutex_lock_nested+0x6c/0x2f0
|        [<c14051e6>] tty_lock_nested+0x36/0x80
|        [<c1405279>] tty_lock_pair+0x29/0x70
|        [<c12e0bb8>] tty_release+0x118/0x470
|        [<c1105e28>] __fput+0xd8/0x270
|        [<c1105fcd>] ____fput+0xd/0x10
|        [<c1051dd9>] task_work_run+0xb9/0xf0
|        [<c1002a51>] do_notify_resume+0x51/0x80
|        [<c140550a>] work_notifysig+0x35/0x3b
|
| -> #0 (&tty->legacy_mutex){+.+.+.}:
|        [<c107f3c9>] __lock_acquire+0x1189/0x16a0
|        [<c107fe74>] lock_acquire+0x84/0x190
|        [<c140276c>] mutex_lock_nested+0x6c/0x2f0
|        [<c14051e6>] tty_lock_nested+0x36/0x80
|        [<c140523f>] tty_lock+0xf/0x20
|        [<c12df8e4>] __tty_hangup+0x54/0x410
|        [<c12dfcb2>] do_tty_hangup+0x12/0x20
|        [<c104f763>] process_one_work+0x1a3/0x5e0
|        [<c104fec9>] worker_thread+0x119/0x3a0
|        [<c1055084>] kthread+0x94/0xa0
|        [<c140ca37>] ret_from_kernel_thread+0x1b/0x28
|
|other info that might help us debug this:
|
|Chain exists of:
|  &tty->legacy_mutex --> &tty->legacy_mutex/1 --> (&tty->hangup_work)
|
| Possible unsafe locking scenario:
|
|       CPU0                    CPU1
|       ----                    ----
|  lock((&tty->hangup_work));
|                               lock(&tty->legacy_mutex/1);
|                               lock((&tty->hangup_work));
|  lock(&tty->legacy_mutex);
|
| *** DEADLOCK ***

Before the path mentioned tty_ldisc_release() look like this:

|	tty_ldisc_halt(tty);
|	tty_ldisc_flush_works(tty);
|	tty_lock();

As it can be seen, it first flushes the workqueue and then grabs the
tty_lock. Now we grab the lock first:

|	tty_lock_pair(tty, o_tty);
|	tty_ldisc_halt(tty);
|	tty_ldisc_flush_works(tty);

so lockdep's complaint seems valid.

The earlier version of this patch took the ldisc_mutex since the other
user of tty_ldisc_flush_works() (tty_set_ldisc()) did this.
Peter Hurley then said that it is should not be requried. Since it
wasn't done earlier, I dropped this part.
The code under tty_ldisc_kill() was executed earlier with the tty lock
taken so it is taken again.

I was able to reproduce the deadlock on v3.8-rc1, this patch fixes the
problem in my testcase. I didn't notice any problems so far.

Cc: Alan Cox <alan@linux.intel.com>
Cc: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2013-01-15 23:07:15 -08:00
Ivo Sieben bd5d7ce9af tty: Only wakeup the line discipline idle queue when queue is active
Before waking up the tty line discipline idle queue first check if the queue is
active (non empty). This prevents unnecessary entering the critical section in
the wake_up() function and therefore avoid needless scheduling overhead on a
PREEMPT_RT system caused by two processes being in the same critical section.

Signed-off-by: Ivo Sieben <meltedpianoman@gmail.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2013-01-15 23:02:22 -08:00
Ivo Sieben c97399418a tty: Use raw spin lock to protect TTY ldisc administration
The global "normal" spin lock that guards the line discipline
administration is replaced by a raw spin lock. On a PREEMPT_RT system this
prevents unwanted scheduling overhead around the line discipline administration.

On a 200 MHz AT91SAM9261 processor setup this fixes about 100us of scheduling
overhead on a TTY read or write call.

Signed-off-by: Ivo Sieben <meltedpianoman@gmail.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2012-10-24 11:37:02 -07:00
Jiri Slaby ecbbfd44a0 TTY: move tty buffers to tty_port
So this is it. The big step why we did all the work over the past
kernel releases. Now everything is prepared, so nothing protects us
from doing that big step.

           |  |            \  \ nnnn/^l      |  |
           |  |             \  /     /       |  |
           |  '-,.__   =>    \/   ,-`    =>  |  '-,.__
           | O __.´´)        (  .`           | O __.´´)
            ~~~   ~~          ``              ~~~   ~~
The buffers are now in the tty_port structure and we can start
teaching the buffer helpers (insert char/string, flip etc.) to use
tty_port instead of tty_struct all around.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Acked-by: Alan Cox <alan@linux.intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2012-10-22 16:58:28 -07:00
Jiri Slaby 31e121284f TTY: ldisc, wait for idle ldisc in release
We reintroduced tty_ldisc_wait_idle in 100eeae2c5 (TTY: restore
tty_ldisc_wait_idle) and used in set_ldisc. Then we added it also to
the hangup path in 92f6fa09bd (TTY: ldisc, do not close until there
are readers). And we noted that there is one more path:
~   Before 65b770468e tty_ldisc_wait_idle was called also from
~   tty_ldisc_release. It is called from tty_release, so I don't think
~   we need to restore that one.

Well, I was wrong. There might still be holders of an ldisc
reference. Not from userspace, but drivers. If they take a reference
and a user closes the device immediately after that, we have a
problem. ldisc is halted and closed by TTY, but the driver still may
call some ldisc's operation and cause a crash.

So restore the tty_ldisc_wait_idle call also to the third location
where it was before 65b770468e (tty-ldisc: turn ldisc user count
into a proper refcount). Now we should be safe with respect to the
ldisc reference counting as all* tty_ldisc_close paths are safely
called with reference count of one.

* Not the one in tty_ldisc_setup's fail path. But that is called
  before the first open finishes. So userspace does not see it yet.
  Even thought the driver is given the TTY already via ->install, it
  should not take a reference to the ldisc yet. If some driver is to
  do this, we should put one tty_ldisc_wait_idle also in the setup.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Acked-by: Alan Cox <alan@linux.intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2012-10-22 16:50:53 -07:00
Linus Torvalds 033d9959ed Merge branch 'for-3.7' of git://git.kernel.org/pub/scm/linux/kernel/git/tj/wq
Pull workqueue changes from Tejun Heo:
 "This is workqueue updates for v3.7-rc1.  A lot of activities this
  round including considerable API and behavior cleanups.

   * delayed_work combines a timer and a work item.  The handling of the
     timer part has always been a bit clunky leading to confusing
     cancelation API with weird corner-case behaviors.  delayed_work is
     updated to use new IRQ safe timer and cancelation now works as
     expected.

   * Another deficiency of delayed_work was lack of the counterpart of
     mod_timer() which led to cancel+queue combinations or open-coded
     timer+work usages.  mod_delayed_work[_on]() are added.

     These two delayed_work changes make delayed_work provide interface
     and behave like timer which is executed with process context.

   * A work item could be executed concurrently on multiple CPUs, which
     is rather unintuitive and made flush_work() behavior confusing and
     half-broken under certain circumstances.  This problem doesn't
     exist for non-reentrant workqueues.  While non-reentrancy check
     isn't free, the overhead is incurred only when a work item bounces
     across different CPUs and even in simulated pathological scenario
     the overhead isn't too high.

     All workqueues are made non-reentrant.  This removes the
     distinction between flush_[delayed_]work() and
     flush_[delayed_]_work_sync().  The former is now as strong as the
     latter and the specified work item is guaranteed to have finished
     execution of any previous queueing on return.

   * In addition to the various bug fixes, Lai redid and simplified CPU
     hotplug handling significantly.

   * Joonsoo introduced system_highpri_wq and used it during CPU
     hotplug.

  There are two merge commits - one to pull in IRQ safe timer from
  tip/timers/core and the other to pull in CPU hotplug fixes from
  wq/for-3.6-fixes as Lai's hotplug restructuring depended on them."

Fixed a number of trivial conflicts, but the more interesting conflicts
were silent ones where the deprecated interfaces had been used by new
code in the merge window, and thus didn't cause any real data conflicts.

Tejun pointed out a few of them, I fixed a couple more.

* 'for-3.7' of git://git.kernel.org/pub/scm/linux/kernel/git/tj/wq: (46 commits)
  workqueue: remove spurious WARN_ON_ONCE(in_irq()) from try_to_grab_pending()
  workqueue: use cwq_set_max_active() helper for workqueue_set_max_active()
  workqueue: introduce cwq_set_max_active() helper for thaw_workqueues()
  workqueue: remove @delayed from cwq_dec_nr_in_flight()
  workqueue: fix possible stall on try_to_grab_pending() of a delayed work item
  workqueue: use hotcpu_notifier() for workqueue_cpu_down_callback()
  workqueue: use __cpuinit instead of __devinit for cpu callbacks
  workqueue: rename manager_mutex to assoc_mutex
  workqueue: WORKER_REBIND is no longer necessary for idle rebinding
  workqueue: WORKER_REBIND is no longer necessary for busy rebinding
  workqueue: reimplement idle worker rebinding
  workqueue: deprecate __cancel_delayed_work()
  workqueue: reimplement cancel_delayed_work() using try_to_grab_pending()
  workqueue: use mod_delayed_work() instead of __cancel + queue
  workqueue: use irqsafe timer for delayed_work
  workqueue: clean up delayed_work initializers and add missing one
  workqueue: make deferrable delayed_work initializer names consistent
  workqueue: cosmetic whitespace updates for macro definitions
  workqueue: deprecate system_nrt[_freezable]_wq
  workqueue: deprecate flush[_delayed]_work_sync()
  ...
2012-10-02 09:54:49 -07:00
Tejun Heo 43829731dd workqueue: deprecate flush[_delayed]_work_sync()
flush[_delayed]_work_sync() are now spurious.  Mark them deprecated
and convert all users to flush[_delayed]_work().

If you're cc'd and wondering what's going on: Now all workqueues are
non-reentrant and the regular flushes guarantee that the work item is
not pending or running on any CPU on return, so there's no reason to
use the sync flushes at all and they're going away.

This patch doesn't make any functional difference.

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Russell King <linux@arm.linux.org.uk>
Cc: Paul Mundt <lethal@linux-sh.org>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Jens Axboe <axboe@kernel.dk>
Cc: Mattia Dongili <malattia@linux.it>
Cc: Kent Yoder <key@linux.vnet.ibm.com>
Cc: David Airlie <airlied@linux.ie>
Cc: Jiri Kosina <jkosina@suse.cz>
Cc: Karsten Keil <isdn@linux-pingi.de>
Cc: Bryan Wu <bryan.wu@canonical.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Alasdair Kergon <agk@redhat.com>
Cc: Mauro Carvalho Chehab <mchehab@infradead.org>
Cc: Florian Tobias Schandinat <FlorianSchandinat@gmx.de>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: linux-wireless@vger.kernel.org
Cc: Anton Vorontsov <cbou@mail.ru>
Cc: Sangbeom Kim <sbkim73@samsung.com>
Cc: "James E.J. Bottomley" <James.Bottomley@HansenPartnership.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Eric Van Hensbergen <ericvh@gmail.com>
Cc: Takashi Iwai <tiwai@suse.de>
Cc: Steven Whitehouse <swhiteho@redhat.com>
Cc: Petr Vandrovec <petr@vandrovec.name>
Cc: Mark Fasheh <mfasheh@suse.com>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Avi Kivity <avi@redhat.com>
2012-08-20 14:51:24 -07:00
Alan Cox 89c8d91e31 tty: localise the lock
The termios and other changes mean the other protections needed on the driver
tty arrays should be adequate. Turn it all back on.

This contains pieces folded in from the fixes made to the original patches

| From: Geert Uytterhoeven <geert@linux-m68k.org>	(fix m68k)
| From: Paul Gortmaker <paul.gortmaker@windriver.com>	(fix cris)
| From: Jiri Kosina <jkosina@suze.cz>			(lockdep)
| From: Eric Dumazet <eric.dumazet@gmail.com>		(lockdep)

Signed-off-by: Alan Cox <alan@linux.intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2012-08-10 12:55:47 -07:00
Alan Cox d155255a34 tty: Fix race in tty release
Ian Abbott found that the tty layer would explode with the right set of
parallel open and close operations. This is because we race in the
handling of tty->drivers->termios[].

Correct this by
	Making tty_ldisc_release behave like nromal code (takes the lock,
			does stuff, drops the lock)
	Drop the tty lock earlier in tty_ldisc_release
	Taking the tty mutex around the driver->termios update in all cases
	Adding a WARN_ON to catch future screwups.

I also forgot to clean up the pty resources properly. With a pty pair we
need to pull both halves out of the tables.

Signed-off-by: Alan Cox <alan@linux.intel.com>
Tested-by: Ian Abbott <abbotti@mev.co.uk>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2012-07-27 11:55:59 -07:00