On 09/02/2014 05:39 PM, Peter Hurley wrote:
> tty->hw_stopped is not used by the tty core and is thread-unsafe;
> hw_stopped is a member of a bitfield whose fields are updated
> non-atomically and no lock is suitable for serializing updates.
>
> Replace serial core usage of tty->hw_stopped with uport->hw_stopped.
>
> Signed-off-by: Peter Hurley <peter@xxxxxxxxxxxxxxxxxx>
> ---
> drivers/tty/serial/bfin_uart.c | 14 +++++++-------
> drivers/tty/serial/serial_core.c | 28 +++++++++++++---------------
> include/linux/serial_core.h | 3 ++-
> 3 files changed, 22 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/tty/serial/bfin_uart.c b/drivers/tty/serial/bfin_uart.c
> index dec0fd7..fc9fbf3 100644
> --- a/drivers/tty/serial/bfin_uart.c
> +++ b/drivers/tty/serial/bfin_uart.c
> @@ -108,22 +108,22 @@ static void bfin_serial_set_mctrl(struct uart_port *port, unsigned int mctrl)
> static irqreturn_t bfin_serial_mctrl_cts_int(int irq, void *dev_id)
> {
> struct bfin_serial_port *uart = dev_id;
> - unsigned int status = bfin_serial_get_mctrl(&uart->port);
> + struct uart_port *uport = &uart->port;
> + unsigned int status = bfin_serial_get_mctrl(uport);
> #ifdef CONFIG_SERIAL_BFIN_HARD_CTSRTS
> - struct tty_struct *tty = uart->port.state->port.tty;
>
> UART_CLEAR_SCTS(uart);
> - if (tty->hw_stopped) {
> + if (uport->hw_stopped) {
> if (status) {
> - tty->hw_stopped = 0;
> - uart_write_wakeup(&uart->port);
> + uport->hw_stopped = 0;
> + uart_write_wakeup(uport);
> }
> } else {
> if (!status)
> - tty->hw_stopped = 1;
> + uport->hw_stopped = 1;
> }
> #endif
> - uart_handle_cts_change(&uart->port, status & TIOCM_CTS);
> + uart_handle_cts_change(uport, status & TIOCM_CTS);
>
> return IRQ_HANDLED;
> }
> diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
> index 288dc2e..95277a2 100644
> --- a/drivers/tty/serial/serial_core.c
> +++ b/drivers/tty/serial/serial_core.c
> @@ -94,7 +94,7 @@ static void __uart_start(struct tty_struct *tty)
> struct uart_state *state = tty->driver_data;
> struct uart_port *port = state->uart_port;
>
> - if (!tty->stopped && !tty->hw_stopped)
> + if (!uart_tx_stopped(port))
> port->ops->start_tx(port);
> }
>
> @@ -180,10 +180,11 @@ static int uart_port_startup(struct tty_struct *tty, struct uart_state *state,
> }
>
> spin_lock_irq(&uport->lock);
> - if (uart_cts_enabled(uport)) {
> - if (!(uport->ops->get_mctrl(uport) & TIOCM_CTS))
> - tty->hw_stopped = 1;
> - }
> + if (uart_cts_enabled(uport) &&
> + !(uport->ops->get_mctrl(uport) & TIOCM_CTS))
> + uport->hw_stopped = 1;
> + else
> + uport->hw_stopped = 0;
> spin_unlock_irq(&uport->lock);
> }
>
> @@ -944,7 +945,7 @@ static int uart_get_lsr_info(struct tty_struct *tty,
> */
> if (uport->x_char ||
> ((uart_circ_chars_pending(&state->xmit) > 0) &&
> - !tty->stopped && !tty->hw_stopped))
> + !uart_tx_stopped(uport)))
> result &= ~TIOCSER_TEMT;
>
> return put_user(result, value);
> @@ -1290,7 +1291,7 @@ static void uart_set_termios(struct tty_struct *tty,
>
> /*
> * If the port is doing h/w assisted flow control, do nothing.
> - * We assume that tty->hw_stopped has never been set.
> + * We assume that port->hw_stopped has never been set.
> */
> if (uport->flags & UPF_HARD_FLOW)
> return;
> @@ -1298,7 +1299,7 @@ static void uart_set_termios(struct tty_struct *tty,
> /* Handle turning off CRTSCTS */
> if ((old_termios->c_cflag & CRTSCTS) && !(cflag & CRTSCTS)) {
> spin_lock_irqsave(&uport->lock, flags);
> - tty->hw_stopped = 0;
> + uport->hw_stopped = 0;
> __uart_start(tty);
> spin_unlock_irqrestore(&uport->lock, flags);
> }
> @@ -1306,7 +1307,7 @@ static void uart_set_termios(struct tty_struct *tty,
> else if (!(old_termios->c_cflag & CRTSCTS) && (cflag & CRTSCTS)) {
> spin_lock_irqsave(&uport->lock, flags);
> if (!(uport->ops->get_mctrl(uport) & TIOCM_CTS)) {
> - tty->hw_stopped = 1;
> + uport->hw_stopped = 1;
> uport->ops->stop_tx(uport);
> }
> spin_unlock_irqrestore(&uport->lock, flags);
> @@ -2776,23 +2777,20 @@ EXPORT_SYMBOL_GPL(uart_handle_dcd_change);
> */
> void uart_handle_cts_change(struct uart_port *uport, unsigned int status)
> {
> - struct tty_port *port = &uport->state->port;
> - struct tty_struct *tty = port->tty;
> -
> warn_not_spin_locked(&uport->lock);
>
> uport->icount.cts++;
>
> if (uart_cts_enabled(uport)) {
> - if (tty->hw_stopped) {
> + if (uport->hw_stopped) {
> if (status) {
> - tty->hw_stopped = 0;
> + uport->hw_stopped = 0;
> uport->ops->start_tx(uport);
> uart_write_wakeup(uport);
> }
> } else {
> if (!status) {
> - tty->hw_stopped = 1;
> + uport->hw_stopped = 1;
> uport->ops->stop_tx(uport);
> }
> }
> diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
> index bc70048..c87aaf8 100644
> --- a/include/linux/serial_core.h
> +++ b/include/linux/serial_core.h
> @@ -195,6 +195,7 @@ struct uart_port {
> #define UPSTAT_CTS_ENABLE ((__force upstat_t) (1 << 0))
> #define UPSTAT_DCD_ENABLE ((__force upstat_t) (1 << 1))
>
> + bool hw_stopped; /* sw-assisted CTS flow state */
This is fragile on Alpha as well (byte storage) in structure with
mixed cpu contexts.
Regards,
Peter Hurley
> unsigned int mctrl; /* current modem ctrl settings */
> unsigned int timeout; /* character-based timeout */
> unsigned int type; /* port type */
> @@ -353,7 +354,7 @@ int uart_resume_port(struct uart_driver *reg, struct uart_port *port);
> static inline int uart_tx_stopped(struct uart_port *port)
> {
> struct tty_struct *tty = port->state->port.tty;
> - if(tty->stopped || tty->hw_stopped)
> + if (tty->stopped || port->hw_stopped)
> return 1;
> return 0;
> }
>
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
