Use container_of instead of casting first structure member.
Signed-off-by: Fabian Frederick <fabf@skynet.be>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Use container_of instead of casting first structure member.
Signed-off-by: Fabian Frederick <fabf@skynet.be>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Use container_of instead of casting first structure member.
Signed-off-by: Fabian Frederick <fabf@skynet.be>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Use container_of instead of casting first structure member.
Signed-off-by: Fabian Frederick <fabf@skynet.be>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Use container_of instead of casting first structure member.
Signed-off-by: Fabian Frederick <fabf@skynet.be>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This patch adds the required pieces to 8250-OMAP UART driver for DMA
support. The TX burst size is set to 1 so we can send an arbitrary
amount of bytes.
The RX burst is currently set to 48 which means we receive an DMA
interrupt every 48 bytes and have to reprogram everything. Less bytes in
the RX-FIFO mean that no DMA transfer will happen and the UART will send a
RX-timeout _or_ RDI event at which point the FIFO will be manually purged.
There is a workaround for TX-DMA on AM33xx where we put the first byte
into the FIFO to kick start the DMA process. Haven't seen this problem on
OMAP36xx (beagle board xm) or DRA7xx.
On AM375x there is "Usage Note 2.7: UART: Cannot Acknowledge Idle
Requests in Smartidle Mode When Configured for DMA Operations" in the
errata document. This problem persists even after disabling DMA in the
UART and will be addressed in the HWMOD.
v10:
- delay update_registers() from set_termios() until TX-DMA is
done. It has been reported / proved that invoking
update_registers() while TX-DMA is in progress may stall the
DMA operation and it won't finish.
- use the new omap DMA-TX-RX hooks and DMA only interrupt
routine.
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
We have (or will have) custom DMA callbacks in the omap driver due to
the different behaviour in the RX and TX case. To make this work
we need a few changes in the IRQ handler to invoke the rx_handler again
after the "manual" mode or retry the tx_handler again before falling
back to the manual mode.
Heikki didn't want to see the extra hacks in the generic / default irq
handler and Peter wasn't too happy about an OMAP-only IRQ handler. The
way I planned it is to use this extra IRQ routine only in DMA case. If
Peter dislike this approach then I hope Heikki doesn't block changes in
the default IRQ handler :)
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Reviewed-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The omap needs a DMA request pending right away. If it is
enqueued once the bytes are in the FIFO then nothing will happen
and the FIFO will be later purged via RX-timeout interrupt.
This patch enqueues RX-DMA request on completion but not if it
was aborted on error. The first enqueue will happen in the driver
in startup.
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Reviewed-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This patch provides mostly a copy of serial8250_tx_dma() +
__dma_tx_complete() with the following extensions:
- DMA bug
At least on AM335x the following problem exists: Even if the TX FIFO is
empty and a TX transfer is programmed (and started) the UART does not
trigger the DMA transfer.
After $TRESHOLD number of bytes have been written to the FIFO manually the
UART reevaluates the whole situation and decides that now there is enough
room in the FIFO and so the transfer begins.
This problem has not been seen on DRA7 or beagle board xm (OMAP3). I am not
sure if this is UART-IP core specific or DMA engine.
The workaround is to use a threshold of one byte, program the DMA
transfer minus one byte and then to put the first byte into the FIFO to
kick start the transfer.
- support for runtime PM
RPM is enabled on start_tx(). We can't disable RPM on DMA complete callback
because there is still data in the FIFO which is being sent. We have to wait
until the FIFO is empty before we disable it.
For this to happen we fake a TX sent error and enable THRI. Once the
FIFO is empty we receive an interrupt and since the TTY-buffer is still
empty we "put RPM" via __stop_tx(). Should it been filed then in the
start_tx() path we should program the DMA transfer and remove the error
flag and the THRI bit.
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Reviewed-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The OMAP has a few corner cases where it needs a share of kindness of
affection to do the right thing. Heikki Krogerus suggested that instead
adding the quirks into the default DMA implementation, OMAP could get
its own copy of the function. And Alan suggested the same thing so here
we go.
This patch provides callbacks for custom TX/RX DMA implementation. If
there are not setup / used, then the default (current) implementation is
used.
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Reviewed-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
After dmaengine_terminate_all() has been invoked then both DMA drivers
(edma and omap-dma) do not invoke dma_cookie_complete() to mark the
transfer as complete. This dma_cookie_complete() is performed by the
Synopsys DesignWare driver which is probably the only one that is used
by omap8250-dma and hence don't see following problem…
…which is that once a RX transfer has been terminated then following
query of channel status reports DMA_IN_PROGRESS (again: the actual
transfer has been canceled, there is nothing going on anymore).
This means that serial8250_rx_dma() never enqueues another DMA transfer
because it (wrongly) assumes that there is a transer already pending.
Vinod Koul refuses to accept a patch which adds this
dma_cookie_complete() to both drivers and so dmaengine_tx_status() would
report DMA_COMPLETE instead (and behave like the Synopsys DesignWare
driver already does). He argues that I am not allowed to use the cookie
to query the status and that the driver already cleaned everything up after
the invokation of dmaengine_terminate_all().
To end this I add a bookkeeping whether or not a RX-transfer has been
started to the 8250-dma code. It has already been done for the TX side.
*Now* we learn about the RX status based on our bookkeeping and don't
need dmaengine_tx_status() for this anymore.
Cc: vinod.koul@intel.com
Reviewed-by: Tony Lindgren <tony@atomide.com>
Tested-by: Tony Lindgren <tony@atomide.com>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Reviewed-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Right now it is possible that serial8250_tx_dma() fails and returns
-EBUSY. The caller (serial8250_start_tx()) will then enable
UART_IER_THRI which will generate an interrupt once the TX FIFO is
empty.
In serial8250_handle_irq() nothing will happen because up->dma is set
and so serial8250_tx_chars() won't be invoked. We end up with plenty of
interrupts and some "too much work for irq" output.
This patch introduces dma_tx_err in struct uart_8250_port to signal that
the last invocation of serial8250_tx_dma() failed so we can fill the TX
FIFO manually. Should the next invocation of serial8250_start_tx()
succeed then the dma_tx_err flag along with the THRI bit is removed and
DMA only usage may continue.
Reviewed-by: Tony Lindgren <tony@atomide.com>
Tested-by: Tony Lindgren <tony@atomide.com>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Reviewed-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This patch provides a 8250-core based UART driver for the internal OMAP
UART. The long term goal is to provide the same functionality as the
current OMAP uart driver and DMA support.
I tried to merge omap-serial code together with the 8250-core code.
There should should be hardly a noticable difference. The trigger levels
are different compared to omap-serial:
- omap serial
TX: Interrupt comes after TX FIFO has room for 16 bytes.
TX of 4096 bytes in one go results in 256 interrupts
RX: Interrupt comes after there is on byte in the FIFO.
RX of 4096 bytes results in 4096 interrupts.
- this driver
TX: Interrupt comes once the TX FIFO is empty.
TX of 4096 bytes results in 65 interrupts. That means there will
be gaps on the line while the driver reloads the FIFO.
RX: Interrupt comes once there are 48 bytes in the FIFO or less over
"longer" time frame. We have
1 / 11520 * 10^3 * 16 => 1.38… ms
1.38ms to react and purge the FIFO on 115200,8N1. Since the other
driver fired after each byte it had ~5.47ms time to react. This
_may_ cause problems if one relies on no missing bytes and has no
flow control. On the other hand we get only 85 interrupts for the
same amount of data.
It has been only tested as console UART on am335x-evm, dra7-evm and
beagle bone. I also did some longer raw-transfers to meassure the load.
The device name is ttyS based instead of ttyO. If a ttyO based node name
is required please ask udev for it. If both driver are activated (this
and omap-serial) then this serial driver will take control over the
device due to the link order
v9…v10:
- Tony noticed that omap3 won't show anything after waking up
from core off. In v9 I reworked the register restore and set
IER to 0 by accident. This went unnoticed because start_tx
usually sets ier (either due to DMA bug or due to TX-complete
IRQ).
- dropped EFR and SLEEP from capabilities. We do have both but
nobody should touch it. We already handle SLEEP ourself.
- make the private copy of the registers (like EFR) u8 instead
u32
- drop MDR1 & DL[ML] reset in restore registers. Does not look
required it is set to the required value later.
- update MDR1 & SCR only if changed.
- set MDR1 as the last thing. The errata says that we should
setup everything before MDR1 set.
- avoid div by 0 in omap_8250_get_divisor() if baud rate gets
very large (Frans Klaver fixed the same thing omap-serial)
- drop "is in early stage" from Kconfig.
v8…v9:
- less on a file seems to hang the am335x after a while. I
believe I introduce this bug a while ago since I can reproduce
this prior to v8. Fixed by redoing the omap8250_restore_regs()
v7…v8:
- redo the register write. There is now one function for that
which is used from set_termios() and runtime-resume.
- drop PORT_OMAP_16750 and move the setup to the omap file. We
have our own set termios function anyway (Heikki Krogerus)
- use MEM instead of MEM32. TRM of AM/DM37x says that 32bit
access on THR might result in data abort. We only need 32bit
access in the errata function which is before we use 8250's
read function so it doesn't matter.
v4…v7:
- change trigger levels after some tests with raw transfers.
v3…v4:
- drop RS485 support
- wire up ->throttle / ->unthrottle
v2…v3:
- wire up startup & shutdown for wakeup-irq handling.
- RS485 handling (well the core does).
v1…v2:
- added runtime PM. Could somebody could please double check
this?
- added omap_8250_set_termios()
Reviewed-by: Tony Lindgren <tony@atomide.com>
Tested-by: Tony Lindgren <tony@atomide.com>
Tested-by: Frans Klaver <frans.klaver@xsens.com>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Reviewed-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
if I boot with console=ttyS0 and the omap driver is module I end up with
| console [ttyS0] disabled
| omap8250 44e09000.serial: ttyS0 at MMIO 0x44e09000 (irq = 88, base_baud = 3000000) is a 8250
| Unable to handle kernel paging request at virtual address c07a9de0
| Modules linked in: 8250_omap(+)
| CPU: 0 PID: 908 Comm: modprobe Not tainted 3.17.0-rc5+ #1593
| PC is at serial8250_console_setup+0x0/0xc8
| LR is at register_console+0x13c/0x3a4
| [<c0078788>] (register_console) from [<c02d0340>] (uart_add_one_port+0x3cc/0x420)
| [<c02d0340>] (uart_add_one_port) from [<c02d38a4>] (serial8250_register_8250_port+0x298/0x39c)
| [<c02d38a4>] (serial8250_register_8250_port) from [<bf006274>] (omap8250_probe+0x218/0x3dc [8250_omap])
| [<bf006274>] (omap8250_probe [8250_omap]) from [<c02e3424>] (platform_drv_probe+0x2c/0x5c)
| [<c02e3424>] (platform_drv_probe) from [<c02e1eac>] (driver_probe_device+0x104/0x228)
…
| [<c009fa48>] (SyS_init_module) from [<c000e6e0>] (ret_fast_syscall+0x0/0x30)
| Code: 7823603b f8314620 051b3013 491ed416 (44792204)
because serial8250_console_setup() is already gone.
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Reviewed-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Frans reworded the two comments with better English for better
understanding. His review hit the mailing list after the patch got
applied so here is an incremental update.
Reported-by: Frans Klaver <frans.klaver@xsens.com>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Reviewed-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
When using mxs-auart based console, sometime we need the sysrq function
to help debugging kernel. The sysrq code is basically there,
this patch just simply enable it.
Signed-off-by: Janusz Uzycki <j.uzycki@elproma.com.pl>
Reviewed-by: Fabio Estevam <fabio.estevam@freescale.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This error message is not necessary. The driver core code will print all
probe error messages. It also resolves some error codes to proper error
messages. For example -EPROBE_DEFER will only be printed as an info message.
This patch removes the error message as the core prints the same
information.
Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
Acked-by: Felipe Balbi <balbi@ti.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
A port count mismatch occurs if mutex_lock_interruptible()
exits uart_open() and the port has already been opened. This may
prematurely close a port on an open tty. Since uart_close() is _always_
called if uart_open() fails, the port count must be corrected if errors
occur.
Always increment the port count in uart_open(), regardless of errors;
always decrement the port count in uart_close(). Note that
tty_port_close_start() decrements the port count when uart_open()
was successful.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
uart_start() only claims the port->lock to call __uart_start(),
which does the actual processing. Eliminate the extra acquire/release
in uart_write(); call __uart_start() directly with port->lock already
held.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The key function of uart_add_one_port() is to cross-reference the
UART driver's port structure with the serial core's state table;
keep the assignments together and document this crucial association.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
tty_port_init() initializes close_delay and closing_wait to these
same values; remove.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The wrapped line looks wrong and out-of-place; leave it as
>80 char line.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Note the serial_struct flags for which the kernel ignores and performs
no action. The flags cannot be removed since they form part of the
userspace interface via the TIOCSSERIAL/TIOCGSERIAL ioctls.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The userspace-defined ASYNC_* flags in include/uapi/linux/tty_flags.h
are the authoritative bit definitions for the serial_struct flags,
and thus for any derivative values or fields.
Although the serial core provides the TIOCSSERIAL and TIOCGSERIAL
ioctls to set and retrieve these flags from userspace, it defines these
bits independently, as UPF_* macros.
Define the UPF_* macros which are userspace-modifiable directly from
the ASYNC_* symbolic constants. Add compile-time test to ensure the
bits changeable by TIOCSSERIAL match the defined range in the uapi
header.
Add ASYNCB_MAGIC_MULTIPLIER to the uapi header since this bit is
programmable by userspace.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The low-level uart driver may modify termios settings to override
settings that are not compatible with the uart, such as CRTSCTS.
Thus, callers of the low-level uart driver's set_termios() method must
hold termios_rwsem write lock to prevent concurrent access to termios,
in case such override occurs.
The termios_rwsem lock requirement does not extend to console setup
(ie., uart_set_options), as console setup cannot race with tty
operations. Nor does this lock requirement extend to functions which
cannot be concurrent with tty ioctls (ie., uart_port_startup() and
uart_resume_port()).
Further, always claim the port mutex to protect hardware
re-reprogramming in the set_termios() uart driver method. Note this
is unnecessary for console initialization in uart_set_options()
which cannot be concurrent with other uart operations.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The tty buffers (and any line discipline buffers) must be flushed after
the UART hardware has shutdown; otherwise, a racing open on the same
tty may receive data from the previous session, which is a security
hazard. However, holding the port mutex while flushing the line
discipline buffers creates a lock inversion if the set_termios()
handler takes the port mutex (as it does in the followup patch,
'serial: Fix locking for uart driver set_termios method'.
Flush the ldisc buffers after dropping the port mutex; the tty lock
is still held which prevents a concurrent open() from advancing while
flushing. Since no new rx data is possible after uart_shutdown() until
a new open reinitializes the port, the later flush has no impact on
what data is being discarded.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
In the context of the final tty & port close, flushing the tx
ring buffer after the hardware has already been shutdown and
the ring buffer freed is neither required nor desirable.
uart_flush_buffer() performs 3 operations:
1. Resets tx ring buffer indices, but the tx ring buffer has
already been freed and the indices are reset if the port is
re-opened.
2. Calls uart driver's flush_buffer() method
5 in-tree uart drivers define flush_buffer() methods:
amba-pl011, atmel-serial, imx, serial-tegra, timbuart
These have been refactored into the shutdown() method, if
required.
3. Kicks the ldisc for more writing, but this is undesirable.
The file handle is being released; any waiting writer will
will be kicked out by tty_release() with a warning. Further,
the N_TTY ldisc may generate SIGIO for a file handle which
is no longer valid.
Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
Cc: Russell King <linux@arm.linux.org.uk>
Cc: Laxman Dewangan <ldewangan@nvidia.com>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
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>
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>
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>
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>
When releasing one end of a pty pair, that end may just have written
to the other, which the input processing worker, flush_to_ldisc(), is
still working on but has not completed the copy to the other end's
read buffer. So input may not appear to be available to a waiting
reader but yet TTY_OTHER_CLOSED is now observed. The n_tty line
discipline has worked around this by waiting for input processing
to complete and then re-checking if input is available before
exiting with -EIO.
Since the tty/ldisc lock reordering, the wait for input processing
to complete can now occur during final close before setting
TTY_OTHER_CLOSED. In this way, a waiting reader is guaranteed to
see input available (if any) before observing TTY_OTHER_CLOSED.
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>
With the revised tty lock order and lockdep annotation, claiming
the pty slave lock is now safe while still holding the pty master lock.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The tty_unhangup() function is not defined.
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>
Eliminate the requirement of specifying the tty lock nesting at
lock time; instead, set the lock subclass for slave ptys at pty
install (normal ttys and master ptys use subclass 0).
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
When releasing the master pty, the slave pty also needs to be locked
to prevent concurrent tty count changes for the slave pty and to
ensure that only one parallel master and slave release observe the
final close, and proceed to destruct the pty pair. Conversely, when
releasing the slave pty, locking the master pty is not necessary
(since the master's state can be inferred by the slave tty count).
Introduce tty_lock_slave()/tty_unlock_slave() which acquires/releases
the tty lock of the slave pty. Remove tty_lock_pair()/tty_unlock_pair().
Dropping the tty_lock is no longer required to re-establish a stable
lock order.
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>
The local o_tty variable in tty_release() is now accessed only
when closing the pty master.
Set o_tty to slave pty when closing pty master, otherwise NULL;
use o_tty != NULL as replacement for pty_master.
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>
Passing the 'other' tty to tty_release_checks() only makes sense
for a pty pair; make o_tty scope 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>
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>
Perform work flush for both ends of a pty pair within tty_flush_works(),
rather than calling twice.
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>
When the slave side closes and its tty count is 0, the pty
pair can be destroyed; the master side must have already
closed for the slave side tty count to be 0. Thus, only the
pty master close must check if the slave side has closed by
checking the slave tty count.
Remove the pre-computed closing flags and check the actual count(s).
Regular ttys are unaffected by this change.
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>
Holding the tty_lock() is necessary to prevent concurrent changes
to the tty count that may cause it to differ from the open file
list count. The tty_lock() is already held at all call sites.
NB: Note that the check for the pty master tty count is safe because
the slave's tty_lock() is held while decrementing the pty master
tty count.
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>
Releasing the tty locks while waiting for the tty wait queues to
be empty is no longer necessary nor desirable. Prior to
"tty: Don't take tty_mutex for tty count changes", dropping the
tty locks was necessary to reestablish the correct lock order between
tty_mutex and the tty locks. Dropping the global tty_mutex was necessary;
otherwise new ttys could not have been opened while waiting.
However, without needing the global tty_mutex held, the tty locks for
the releasing tty can now be held through the sleep. The sanity check
is for abnormal conditions caused by kernel bugs, not for recoverable
errors caused by misbehaving userspace; dropping the tty locks only
allows the tty state to get more sideways.
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>
Holding tty_mutex is no longer required to serialize changes to
the tty_count or to prevent concurrent opens of closing ttys;
tty_lock() is sufficient.
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>
Now that re-open is not permitted for a legacy BSD pty master,
using TTY_CLOSING to indicate when a tty can be torn-down is
no longer necessary.
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>
Holding tty_mutex for a tty re-open is no longer necessary since
"tty: Clarify re-open behavior of master ptys". Because the
slave tty count is no longer accessed by tty_reopen(), holding
tty_mutex to prevent concurrent final tty_release() of the slave
pty is not required.
As with "tty: Re-open /dev/tty without tty_mutex", holding a
tty kref until the tty_lock is acquired is sufficient to ensure
the tty has not been freed, which, in turn, is sufficient to
ensure the tty_lock can be safely acquired and the tty count
can be safely retrieved. A non-zero tty count with the tty lock
held guarantees that release_tty() has not run and cannot
run concurrently with tty_reopen().
Change tty_driver_lookup_tty() to acquire the tty kref, which
allows the tty_mutex to be dropped before acquiring the tty lock.
Dropping the tty_mutex before attempting the tty_lock allows
other ttys to be opened and released, without needing this
tty_reopen() to complete.
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>
Opening /dev/tty (ie., the controlling tty for the current task)
is always a re-open of the underlying tty. Because holding the
tty_lock is sufficient for safely re-opening a tty, and because
having a tty kref is sufficient for safely acquiring the tty_lock [1],
tty_open_current_tty() does not require holding tty_mutex.
Repurpose tty_open_current_tty() to perform the re-open itself and
refactor tty_open().
[1] Analysis of safely re-opening the current tty w/o tty_mutex
get_current_tty() gets a tty kref from the already kref'ed tty value of
current->signal->tty while holding the sighand lock for the current
task. This guarantees that the tty pointer returned from
get_current_tty() points to a tty which remains referenceable
while holding the kref.
Although release_tty() may run concurrently, and thus the driver
reference may be removed, release_one_tty() cannot have run, and
won't while holding the tty kref.
This, in turn, guarantees the tty_lock() can safely be acquired
(since tty->magic and tty->legacy_mutex are still a valid dereferences).
The tty_lock() also gets a tty kref to prevent the tty_unlock() from
dereferencing a released tty. Thus, the kref returned from
get_current_tty() can be released.
Lastly, the first operation of tty_reopen() is to check the tty count.
If non-zero, this ensures release_tty() is not running concurrently,
and the driver references have not been removed.
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>
Opening the slave BSD pty first already returns -EIO from the slave
pty_open(), which in turn causes the newly installed tty pair to be
released before returning from tty_open(). However, this can also
cause a parallel master BSD pty open to fail because the pty pair
destruction may already been taking place in tty_release().
Failing at driver->install() if the slave pty is opened first ensures
that a pty master open cannot fail, because the driver tables will
not have been updated so tty_driver_lookup_tty() won't find the
master pty (and attempt to "re-open" it).
In turn, this guarantees that any tty with a tty->count == 0 is
in final close (rather than never opened).
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>
Although perhaps not obvious, the TTY_CLOSING bit is set when the
tty count has been decremented to 0 (which occurs while holding
tty_lock). The only other case when tty count is 0 during a re-open
is when a legacy BSD pty master has been opened in parallel but
after the pty slave, which is unsupported and returns an error.
Thus !tty->count contains the complete set of degenerate conditions
under which a tty open fails.
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>
Re-opening master ptys is not allowed. Once opened and for the remaining
lifetime of the master pty, its tty count is 1. If its tty count has
dropped to 0, then the master pty was closed and TTY_CLOSING was set,
and destruction may begin imminently.
Besides the normal case of a legacy BSD pty master being re-opened
(which always returns -EIO), this code is only reachable in 2 degenerate
cases:
1. The pty master is the controlling terminal (this is possible through
the TIOCSCTTY ioctl). pty masters are not designed to be controlling
terminals and it's an oversight that tiocsctty() ever let that happen.
The attempted open of /dev/tty will always fail. No known program does
this.
2. The legacy BSD pty slave was opened first. The slave open will fail
in pty_open() and tty_release() will commence. But before tty_release()
claims the tty_mutex, there is a very small window where a parallel
master open might succeed. In a test of racing legacy BSD slave and
master parallel opens, where:
slave open attempts: 10000 success:4527 failure:5473
master open attempts: 11728 success:5789 failure:5939
only 8 master open attempts would have succeeded reaching this code and
successfully opened the master pty. This case is not possible with
SysV ptys.
Always return -EIO if a master pty is re-opened or the slave is opened
first and the master opened in parallel (for legacy BSD ptys).
Furthermore, now that changing the slave's count is not required,
the tty_lock is sufficient for preventing concurrent changes to the
tty being re-opened (or failing re-opening).
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>
Fabian Frederick