* [PATCH 01/14] tty: serial - fix tty referencing in set_ldisc
2010-04-01 16:04 [RFC PATCH 00/14] Some BKL killing in the serial drivers Alan Cox
@ 2010-04-01 16:04 ` Alan Cox
2010-04-01 16:04 ` [PATCH 02/14] tty: serial - fix tty back references in termios Alan Cox
` (12 subsequent siblings)
13 siblings, 0 replies; 18+ messages in thread
From: Alan Cox @ 2010-04-01 16:04 UTC (permalink / raw)
To: linux-kernel
Pass down the ldisc number so that the drivers don't have to peek into the
tty object themselves. This lets us get rid of another case of back referencing
port to tty which we don't want (because of races versus hangup/close).
Signed-off-by: Alan Cox <alan@linux.intel.com>
---
drivers/serial/bfin_5xx.c | 7 ++-----
drivers/serial/serial_core.c | 2 +-
include/linux/serial_core.h | 2 +-
3 files changed, 4 insertions(+), 7 deletions(-)
diff --git a/drivers/serial/bfin_5xx.c b/drivers/serial/bfin_5xx.c
index 96f7e74..e0d25ca 100644
--- a/drivers/serial/bfin_5xx.c
+++ b/drivers/serial/bfin_5xx.c
@@ -952,15 +952,12 @@ bfin_serial_verify_port(struct uart_port *port, struct serial_struct *ser)
* Enable the IrDA function if tty->ldisc.num is N_IRDA.
* In other cases, disable IrDA function.
*/
-static void bfin_serial_set_ldisc(struct uart_port *port)
+static void bfin_serial_set_ldisc(struct uart_port *port, int ld)
{
int line = port->line;
unsigned short val;
- if (line >= port->state->port.tty->driver->num)
- return;
-
- switch (port->state->port.tty->termios->c_line) {
+ switch (ld) {
case N_IRDA:
val = UART_GET_GCTL(&bfin_serial_ports[line]);
val |= (IREN | RPOLC);
diff --git a/drivers/serial/serial_core.c b/drivers/serial/serial_core.c
index 7f28307..686ff6c 100644
--- a/drivers/serial/serial_core.c
+++ b/drivers/serial/serial_core.c
@@ -1194,7 +1194,7 @@ static void uart_set_ldisc(struct tty_struct *tty)
struct uart_port *uport = state->uart_port;
if (uport->ops->set_ldisc)
- uport->ops->set_ldisc(uport);
+ uport->ops->set_ldisc(uport, tty->termios->c_line);
}
static void uart_set_termios(struct tty_struct *tty,
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index ad83996..5c4cbe6 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -216,7 +216,7 @@ struct uart_ops {
void (*flush_buffer)(struct uart_port *);
void (*set_termios)(struct uart_port *, struct ktermios *new,
struct ktermios *old);
- void (*set_ldisc)(struct uart_port *);
+ void (*set_ldisc)(struct uart_port *, int new);
void (*pm)(struct uart_port *, unsigned int state,
unsigned int oldstate);
int (*set_wake)(struct uart_port *, unsigned int state);
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 02/14] tty: serial - fix tty back references in termios
2010-04-01 16:04 [RFC PATCH 00/14] Some BKL killing in the serial drivers Alan Cox
2010-04-01 16:04 ` [PATCH 01/14] tty: serial - fix tty referencing in set_ldisc Alan Cox
@ 2010-04-01 16:04 ` Alan Cox
2010-04-01 16:04 ` [PATCH 03/14] tty: serial - fix various misuses/mishandlings of port->tty Alan Cox
` (11 subsequent siblings)
13 siblings, 0 replies; 18+ messages in thread
From: Alan Cox @ 2010-04-01 16:04 UTC (permalink / raw)
To: linux-kernel
One or two drivers go poking back into the tty from the termios setting
routine in unsafe ways. We don't need to pass the tty down because the
[ab]users are just using it to get at things they can get at anyway.
This leaves low_latency setting to sort out along with set_ldisc use.
Signed-off-by: Alan Cox <alan@linux.intel.com>
---
drivers/serial/21285.c | 10 +++-------
drivers/serial/imx.c | 10 ++++------
drivers/serial/ioc3_serial.c | 9 +++++----
drivers/serial/ioc4_serial.c | 9 +++++----
drivers/serial/max3100.c | 3 +--
5 files changed, 18 insertions(+), 23 deletions(-)
diff --git a/drivers/serial/21285.c b/drivers/serial/21285.c
index 8681f13..d89aa38 100644
--- a/drivers/serial/21285.c
+++ b/drivers/serial/21285.c
@@ -216,7 +216,7 @@ serial21285_set_termios(struct uart_port *port, struct ktermios *termios,
struct ktermios *old)
{
unsigned long flags;
- unsigned int baud, quot, h_lcr;
+ unsigned int baud, quot, h_lcr, b;
/*
* We don't support modem control lines.
@@ -234,12 +234,8 @@ serial21285_set_termios(struct uart_port *port, struct ktermios *termios,
*/
baud = uart_get_baud_rate(port, termios, old, 0, port->uartclk/16);
quot = uart_get_divisor(port, baud);
-
- if (port->state && port->state->port.tty) {
- struct tty_struct *tty = port->state->port.tty;
- unsigned int b = port->uartclk / (16 * quot);
- tty_encode_baud_rate(tty, b, b);
- }
+ b = port->uartclk / (16 * quot);
+ tty_termios_encode_baud_rate(termios, b, b);
switch (termios->c_cflag & CSIZE) {
case CS5:
diff --git a/drivers/serial/imx.c b/drivers/serial/imx.c
index 4315b23..9504b44 100644
--- a/drivers/serial/imx.c
+++ b/drivers/serial/imx.c
@@ -901,13 +901,11 @@ imx_set_termios(struct uart_port *port, struct ktermios *termios,
rational_best_approximation(16 * div * baud, sport->port.uartclk,
1 << 16, 1 << 16, &num, &denom);
- if (port->state && port->state->port.tty) {
- tdiv64 = sport->port.uartclk;
- tdiv64 *= num;
- do_div(tdiv64, denom * 16 * div);
- tty_encode_baud_rate(sport->port.state->port.tty,
+ tdiv64 = sport->port.uartclk;
+ tdiv64 *= num;
+ do_div(tdiv64, denom * 16 * div);
+ tty_termios_encode_baud_rate(termios,
(speed_t)tdiv64, (speed_t)tdiv64);
- }
num -= 1;
denom -= 1;
diff --git a/drivers/serial/ioc3_serial.c b/drivers/serial/ioc3_serial.c
index f164ba4..bd2e9e6 100644
--- a/drivers/serial/ioc3_serial.c
+++ b/drivers/serial/ioc3_serial.c
@@ -954,12 +954,13 @@ ioc3_change_speed(struct uart_port *the_port,
struct ktermios *new_termios, struct ktermios *old_termios)
{
struct ioc3_port *port = get_ioc3_port(the_port);
- unsigned int cflag;
+ unsigned int cflag, iflag;
int baud;
int new_parity = 0, new_parity_enable = 0, new_stop = 0, new_data = 8;
struct uart_state *state = the_port->state;
cflag = new_termios->c_cflag;
+ iflag = new_termios->c_iflag;
switch (cflag & CSIZE) {
case CS5:
@@ -1000,12 +1001,12 @@ ioc3_change_speed(struct uart_port *the_port,
state->port.tty->low_latency = 1;
- if (I_IGNPAR(state->port.tty))
+ if (iflag & IGNPAR))
the_port->ignore_status_mask &= ~(N_PARITY_ERROR
| N_FRAMING_ERROR);
- if (I_IGNBRK(state->port.tty)) {
+ if (iflag & IGNBRK) {
the_port->ignore_status_mask &= ~N_BREAK;
- if (I_IGNPAR(state->port.tty))
+ if (iflag & IGNPAR)
the_port->ignore_status_mask &= ~N_OVERRUN_ERROR;
}
if (!(cflag & CREAD)) {
diff --git a/drivers/serial/ioc4_serial.c b/drivers/serial/ioc4_serial.c
index 8ad28fc..2c30023 100644
--- a/drivers/serial/ioc4_serial.c
+++ b/drivers/serial/ioc4_serial.c
@@ -1685,11 +1685,12 @@ ioc4_change_speed(struct uart_port *the_port,
{
struct ioc4_port *port = get_ioc4_port(the_port, 0);
int baud, bits;
- unsigned cflag;
+ unsigned cflag, iflag;
int new_parity = 0, new_parity_enable = 0, new_stop = 0, new_data = 8;
struct uart_state *state = the_port->state;
cflag = new_termios->c_cflag;
+ iflag = new_termios->c_iflag;
switch (cflag & CSIZE) {
case CS5:
@@ -1741,12 +1742,12 @@ ioc4_change_speed(struct uart_port *the_port,
state->port.tty->low_latency = 1;
- if (I_IGNPAR(state->port.tty))
+ if (iflag & IGNPAR)
the_port->ignore_status_mask &= ~(N_PARITY_ERROR
| N_FRAMING_ERROR);
- if (I_IGNBRK(state->port.tty)) {
+ if (iflag & IGNBRK) {
the_port->ignore_status_mask &= ~N_BREAK;
- if (I_IGNPAR(state->port.tty))
+ if (iflag & IGNPAR))
the_port->ignore_status_mask &= ~N_OVERRUN_ERROR;
}
if (!(cflag & CREAD)) {
diff --git a/drivers/serial/max3100.c b/drivers/serial/max3100.c
index 3351c3b..dfcabfe 100644
--- a/drivers/serial/max3100.c
+++ b/drivers/serial/max3100.c
@@ -430,7 +430,6 @@ max3100_set_termios(struct uart_port *port, struct ktermios *termios,
int baud = 0;
unsigned cflag;
u32 param_new, param_mask, parity = 0;
- struct tty_struct *tty = s->port.state->port.tty;
dev_dbg(&s->spi->dev, "%s\n", __func__);
if (!tty)
@@ -485,7 +484,7 @@ max3100_set_termios(struct uart_port *port, struct ktermios *termios,
default:
baud = s->baud;
}
- tty_encode_baud_rate(tty, baud, baud);
+ tty_termios_encode_baud_rate(termios, baud, baud);
s->baud = baud;
param_mask |= MAX3100_BAUD;
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 03/14] tty: serial - fix various misuses/mishandlings of port->tty
2010-04-01 16:04 [RFC PATCH 00/14] Some BKL killing in the serial drivers Alan Cox
2010-04-01 16:04 ` [PATCH 01/14] tty: serial - fix tty referencing in set_ldisc Alan Cox
2010-04-01 16:04 ` [PATCH 02/14] tty: serial - fix tty back references in termios Alan Cox
@ 2010-04-01 16:04 ` Alan Cox
2010-04-01 16:04 ` [PATCH 04/14] vc: Locking clean up Alan Cox
` (10 subsequent siblings)
13 siblings, 0 replies; 18+ messages in thread
From: Alan Cox @ 2010-04-01 16:04 UTC (permalink / raw)
To: linux-kernel
Make it robust against hang up events. In most cases we can do this simply
by passing the right things in the first place.
Signed-off-by: Alan Cox <alan@linux.intel.com>
---
drivers/serial/serial_core.c | 109 +++++++++++++++++++++++-------------------
1 files changed, 60 insertions(+), 49 deletions(-)
diff --git a/drivers/serial/serial_core.c b/drivers/serial/serial_core.c
index 686ff6c..570dca2 100644
--- a/drivers/serial/serial_core.c
+++ b/drivers/serial/serial_core.c
@@ -58,7 +58,7 @@ static struct lock_class_key port_lock_key;
#define uart_console(port) (0)
#endif
-static void uart_change_speed(struct uart_state *state,
+static void uart_change_speed(struct tty_struct *tty, struct uart_state *state,
struct ktermios *old_termios);
static void uart_wait_until_sent(struct tty_struct *tty, int timeout);
static void uart_change_pm(struct uart_state *state, int pm_state);
@@ -137,7 +137,7 @@ uart_update_mctrl(struct uart_port *port, unsigned int set, unsigned int clear)
* Startup the port. This will be called once per open. All calls
* will be serialised by the per-port mutex.
*/
-static int uart_startup(struct uart_state *state, int init_hw)
+static int uart_startup(struct tty_struct *tty, struct uart_state *state, int init_hw)
{
struct uart_port *uport = state->uart_port;
struct tty_port *port = &state->port;
@@ -152,7 +152,7 @@ static int uart_startup(struct uart_state *state, int init_hw)
* once we have successfully opened the port. Also set
* up the tty->alt_speed kludge
*/
- set_bit(TTY_IO_ERROR, &port->tty->flags);
+ set_bit(TTY_IO_ERROR, &tty->flags);
if (uport->type == PORT_UNKNOWN)
return 0;
@@ -177,26 +177,26 @@ static int uart_startup(struct uart_state *state, int init_hw)
/*
* Initialise the hardware port settings.
*/
- uart_change_speed(state, NULL);
+ uart_change_speed(tty, state, NULL);
/*
* Setup the RTS and DTR signals once the
* port is open and ready to respond.
*/
- if (port->tty->termios->c_cflag & CBAUD)
+ if (tty->termios->c_cflag & CBAUD)
uart_set_mctrl(uport, TIOCM_RTS | TIOCM_DTR);
}
if (port->flags & ASYNC_CTS_FLOW) {
spin_lock_irq(&uport->lock);
if (!(uport->ops->get_mctrl(uport) & TIOCM_CTS))
- port->tty->hw_stopped = 1;
+ tty->hw_stopped = 1;
spin_unlock_irq(&uport->lock);
}
set_bit(ASYNCB_INITIALIZED, &port->flags);
- clear_bit(TTY_IO_ERROR, &port->tty->flags);
+ clear_bit(TTY_IO_ERROR, &tty->flags);
}
if (retval && capable(CAP_SYS_ADMIN))
@@ -210,11 +210,10 @@ static int uart_startup(struct uart_state *state, int init_hw)
* DTR is dropped if the hangup on close termio flag is on. Calls to
* uart_shutdown are serialised by the per-port semaphore.
*/
-static void uart_shutdown(struct uart_state *state)
+static void uart_shutdown(struct tty_struct *tty, struct uart_state *state)
{
struct uart_port *uport = state->uart_port;
struct tty_port *port = &state->port;
- struct tty_struct *tty = port->tty;
/*
* Set the TTY IO error marker
@@ -430,11 +429,10 @@ uart_get_divisor(struct uart_port *port, unsigned int baud)
EXPORT_SYMBOL(uart_get_divisor);
/* FIXME: Consistent locking policy */
-static void
-uart_change_speed(struct uart_state *state, struct ktermios *old_termios)
+static void uart_change_speed(struct tty_struct *tty, struct uart_state *state,
+ struct ktermios *old_termios)
{
struct tty_port *port = &state->port;
- struct tty_struct *tty = port->tty;
struct uart_port *uport = state->uart_port;
struct ktermios *termios;
@@ -463,8 +461,8 @@ uart_change_speed(struct uart_state *state, struct ktermios *old_termios)
uport->ops->set_termios(uport, termios, old_termios);
}
-static inline int
-__uart_put_char(struct uart_port *port, struct circ_buf *circ, unsigned char c)
+static inline int __uart_put_char(struct uart_port *port,
+ struct circ_buf *circ, unsigned char c)
{
unsigned long flags;
int ret = 0;
@@ -494,8 +492,8 @@ static void uart_flush_chars(struct tty_struct *tty)
uart_start(tty);
}
-static int
-uart_write(struct tty_struct *tty, const unsigned char *buf, int count)
+static int uart_write(struct tty_struct *tty,
+ const unsigned char *buf, int count)
{
struct uart_state *state = tty->driver_data;
struct uart_port *port;
@@ -675,7 +673,7 @@ static int uart_get_info(struct uart_state *state,
return 0;
}
-static int uart_set_info(struct uart_state *state,
+static int uart_set_info(struct tty_struct *tty, struct uart_state *state,
struct serial_struct __user *newinfo)
{
struct serial_struct new_serial;
@@ -770,7 +768,7 @@ static int uart_set_info(struct uart_state *state,
* We need to shutdown the serial port at the old
* port/type/irq combination.
*/
- uart_shutdown(state);
+ uart_shutdown(tty, state);
}
if (change_port) {
@@ -869,25 +867,27 @@ static int uart_set_info(struct uart_state *state,
"is deprecated.\n", current->comm,
tty_name(port->tty, buf));
}
- uart_change_speed(state, NULL);
+ uart_change_speed(tty, state, NULL);
}
} else
- retval = uart_startup(state, 1);
+ retval = uart_startup(tty, state, 1);
exit:
mutex_unlock(&port->mutex);
return retval;
}
-
-/*
- * uart_get_lsr_info - get line status register info.
- * Note: uart_ioctl protects us against hangups.
+/**
+ * uart_get_lsr_info - get line status register info
+ * @tty: tty associated with the UART
+ * @state: UART being queried
+ * @value: returned modem value
+ *
+ * Note: uart_ioctl protects us against hangups.
*/
-static int uart_get_lsr_info(struct uart_state *state,
- unsigned int __user *value)
+static int uart_get_lsr_info(struct tty_struct *tty,
+ struct uart_state *state, unsigned int __user *value)
{
struct uart_port *uport = state->uart_port;
- struct tty_port *port = &state->port;
unsigned int result;
result = uport->ops->tx_empty(uport);
@@ -900,7 +900,7 @@ static int uart_get_lsr_info(struct uart_state *state,
*/
if (uport->x_char ||
((uart_circ_chars_pending(&state->xmit) > 0) &&
- !port->tty->stopped && !port->tty->hw_stopped))
+ !tty->stopped && !tty->hw_stopped))
result &= ~TIOCSER_TEMT;
return put_user(result, value);
@@ -961,7 +961,7 @@ static int uart_break_ctl(struct tty_struct *tty, int break_state)
return 0;
}
-static int uart_do_autoconfig(struct uart_state *state)
+static int uart_do_autoconfig(struct tty_struct *tty,struct uart_state *state)
{
struct uart_port *uport = state->uart_port;
struct tty_port *port = &state->port;
@@ -980,7 +980,7 @@ static int uart_do_autoconfig(struct uart_state *state)
ret = -EBUSY;
if (tty_port_users(port) == 1) {
- uart_shutdown(state);
+ uart_shutdown(tty, state);
/*
* If we already have a port type configured,
@@ -999,7 +999,7 @@ static int uart_do_autoconfig(struct uart_state *state)
*/
uport->ops->config_port(uport, flags);
- ret = uart_startup(state, 1);
+ ret = uart_startup(tty, state, 1);
}
mutex_unlock(&port->mutex);
return ret;
@@ -1122,11 +1122,11 @@ uart_ioctl(struct tty_struct *tty, struct file *filp, unsigned int cmd,
break;
case TIOCSSERIAL:
- ret = uart_set_info(state, uarg);
+ ret = uart_set_info(tty, state, uarg);
break;
case TIOCSERCONFIG:
- ret = uart_do_autoconfig(state);
+ ret = uart_do_autoconfig(tty, state);
break;
case TIOCSERGWILD: /* obsolete */
@@ -1172,7 +1172,7 @@ uart_ioctl(struct tty_struct *tty, struct file *filp, unsigned int cmd,
*/
switch (cmd) {
case TIOCSERGETLSR: /* Get line status register */
- ret = uart_get_lsr_info(state, uarg);
+ ret = uart_get_lsr_info(tty, state, uarg);
break;
default: {
@@ -1219,7 +1219,7 @@ static void uart_set_termios(struct tty_struct *tty,
return;
}
- uart_change_speed(state, old_termios);
+ uart_change_speed(tty, state, old_termios);
/* Handle transition to B0 status */
if ((old_termios->c_cflag & CBAUD) && !(cflag & CBAUD))
@@ -1335,7 +1335,7 @@ static void uart_close(struct tty_struct *tty, struct file *filp)
uart_wait_until_sent(tty, uport->timeout);
}
- uart_shutdown(state);
+ uart_shutdown(tty, state);
uart_flush_buffer(tty);
tty_ldisc_flush(tty);
@@ -1436,7 +1436,7 @@ static void uart_hangup(struct tty_struct *tty)
mutex_lock(&port->mutex);
if (port->flags & ASYNC_NORMAL_ACTIVE) {
uart_flush_buffer(tty);
- uart_shutdown(state);
+ uart_shutdown(tty, state);
port->count = 0;
clear_bit(ASYNCB_NORMAL_ACTIVE, &port->flags);
tty_port_tty_set(port, NULL);
@@ -1446,15 +1446,19 @@ static void uart_hangup(struct tty_struct *tty)
mutex_unlock(&port->mutex);
}
-/*
- * Copy across the serial console cflag setting into the termios settings
- * for the initial open of the port. This allows continuity between the
- * kernel settings, and the settings init adopts when it opens the port
- * for the first time.
+/**
+ * uart_update_termios - update the terminal hw settings
+ * @tty: tty associated with UART
+ * @state: UART to update
+ *
+ * Copy across the serial console cflag setting into the termios settings
+ * for the initial open of the port. This allows continuity between the
+ * kernel settings, and the settings init adopts when it opens the port
+ * for the first time.
*/
-static void uart_update_termios(struct uart_state *state)
+static void uart_update_termios(struct tty_struct *tty,
+ struct uart_state *state)
{
- struct tty_struct *tty = state->port.tty;
struct uart_port *port = state->uart_port;
if (uart_console(port) && port->cons->cflag) {
@@ -1471,7 +1475,7 @@ static void uart_update_termios(struct uart_state *state)
/*
* Make termios settings take effect.
*/
- uart_change_speed(state, NULL);
+ uart_change_speed(tty, state, NULL);
/*
* And finally enable the RTS and DTR signals.
@@ -1668,7 +1672,7 @@ static int uart_open(struct tty_struct *tty, struct file *filp)
/*
* Start up the serial port.
*/
- retval = uart_startup(state, 0);
+ retval = uart_startup(tty, state, 0);
/*
* If we succeeded, wait until the port is ready.
@@ -1683,7 +1687,7 @@ static int uart_open(struct tty_struct *tty, struct file *filp)
if (retval == 0 && !(port->flags & ASYNC_NORMAL_ACTIVE)) {
set_bit(ASYNCB_NORMAL_ACTIVE, &port->flags);
- uart_update_termios(state);
+ uart_update_termios(tty, state);
}
fail:
@@ -2010,9 +2014,13 @@ int uart_suspend_port(struct uart_driver *drv, struct uart_port *uport)
struct tty_port *port = &state->port;
struct device *tty_dev;
struct uart_match match = {uport, drv};
+ struct tty_struct *tty;
mutex_lock(&port->mutex);
+ /* Must be inside the mutex lock until we convert to tty_port */
+ tty = port->tty;
+
tty_dev = device_find_child(uport->dev, &match, serial_match_port);
if (device_may_wakeup(tty_dev)) {
enable_irq_wake(uport->irq);
@@ -2105,9 +2113,12 @@ int uart_resume_port(struct uart_driver *drv, struct uart_port *uport)
ops->set_mctrl(uport, 0);
spin_unlock_irq(&uport->lock);
if (console_suspend_enabled || !uart_console(uport)) {
+ /* Protected by port mutex for now */
+ struct tty_struct *tty = port->tty;
ret = ops->startup(uport);
if (ret == 0) {
- uart_change_speed(state, NULL);
+ if (tty)
+ uart_change_speed(tty, state, NULL);
spin_lock_irq(&uport->lock);
ops->set_mctrl(uport, uport->mctrl);
ops->start_tx(uport);
@@ -2119,7 +2130,7 @@ int uart_resume_port(struct uart_driver *drv, struct uart_port *uport)
* Clear the "initialized" flag so we won't try
* to call the low level drivers shutdown method.
*/
- uart_shutdown(state);
+ uart_shutdown(tty, state);
}
}
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 04/14] vc: Locking clean up
2010-04-01 16:04 [RFC PATCH 00/14] Some BKL killing in the serial drivers Alan Cox
` (2 preceding siblings ...)
2010-04-01 16:04 ` [PATCH 03/14] tty: serial - fix various misuses/mishandlings of port->tty Alan Cox
@ 2010-04-01 16:04 ` Alan Cox
2010-04-30 20:30 ` Arnd Bergmann
2010-04-01 16:05 ` [PATCH 05/14] stallion: prune lock_kernel calls Alan Cox
` (9 subsequent siblings)
13 siblings, 1 reply; 18+ messages in thread
From: Alan Cox @ 2010-04-01 16:04 UTC (permalink / raw)
To: linux-kernel
The virtual console layer uses the BKL for various things that don't really
need it. Clean them out.
Signed-off-by: Alan Cox <alan@linux.intel.com>
---
drivers/char/selection.c | 4 ++++
drivers/char/vt.c | 8 +++++---
2 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/char/selection.c b/drivers/char/selection.c
index f97b9e8..6e79340 100644
--- a/drivers/char/selection.c
+++ b/drivers/char/selection.c
@@ -26,6 +26,7 @@
#include <linux/selection.h>
#include <linux/tiocl.h>
#include <linux/console.h>
+#include <linux/smp_lock.h>
/* Don't take this from <ctype.h>: 011-015 on the screen aren't spaces */
#define isspace(c) ((c) == ' ')
@@ -312,6 +313,8 @@ int paste_selection(struct tty_struct *tty)
struct tty_ldisc *ld;
DECLARE_WAITQUEUE(wait, current);
+ lock_kernel();
+
acquire_console_sem();
poke_blanked_console();
release_console_sem();
@@ -335,5 +338,6 @@ int paste_selection(struct tty_struct *tty)
__set_current_state(TASK_RUNNING);
tty_ldisc_deref(ld);
+ unlock_kernel();
return 0;
}
diff --git a/drivers/char/vt.c b/drivers/char/vt.c
index bd1d116..501ca9e 100644
--- a/drivers/char/vt.c
+++ b/drivers/char/vt.c
@@ -282,7 +282,12 @@ static inline unsigned short *screenpos(struct vc_data *vc, int offset, int view
static inline void scrolldelta(int lines)
{
+ lock_kernel();
+ /* FIXME */
+ /* scrolldelta needs some kind of consistency lock, but the BKL was
+ and still is not protecting versus the scheduled back end */
scrollback_delta += lines;
+ unlock_kernel();
schedule_console_callback();
}
@@ -2604,8 +2609,6 @@ int tioclinux(struct tty_struct *tty, unsigned long arg)
return -EFAULT;
ret = 0;
- lock_kernel();
-
switch (type)
{
case TIOCL_SETSEL:
@@ -2680,7 +2683,6 @@ int tioclinux(struct tty_struct *tty, unsigned long arg)
ret = -EINVAL;
break;
}
- unlock_kernel();
return ret;
}
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH 04/14] vc: Locking clean up
2010-04-01 16:04 ` [PATCH 04/14] vc: Locking clean up Alan Cox
@ 2010-04-30 20:30 ` Arnd Bergmann
0 siblings, 0 replies; 18+ messages in thread
From: Arnd Bergmann @ 2010-04-30 20:30 UTC (permalink / raw)
To: Alan Cox; +Cc: linux-kernel
On Thursday 01 April 2010 18:04:59 Alan Cox wrote:
> @@ -282,7 +282,12 @@ static inline unsigned short *screenpos(struct vc_data *vc, int offset, int view
>
> static inline void scrolldelta(int lines)
> {
> + lock_kernel();
> + /* FIXME */
> + /* scrolldelta needs some kind of consistency lock, but the BKL was
> + and still is not protecting versus the scheduled back end */
> scrollback_delta += lines;
> + unlock_kernel();
> schedule_console_callback();
> }
>
This one triggered a might_sleep() warning in _lock_kernel(), because
it's called from the keyboard interrupt.
Arnd
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 05/14] stallion: prune lock_kernel calls
2010-04-01 16:04 [RFC PATCH 00/14] Some BKL killing in the serial drivers Alan Cox
` (3 preceding siblings ...)
2010-04-01 16:04 ` [PATCH 04/14] vc: Locking clean up Alan Cox
@ 2010-04-01 16:05 ` Alan Cox
2010-04-29 21:29 ` Arnd Bergmann
2010-04-01 16:05 ` [PATCH 06/14] istallion: use bit ops for the board flags Alan Cox
` (8 subsequent siblings)
13 siblings, 1 reply; 18+ messages in thread
From: Alan Cox @ 2010-04-01 16:05 UTC (permalink / raw)
To: linux-kernel
Remove unneeded tty layer lock kernel bits. Relock the needed bits using the
port mutex. The istallion still has brd state races but those are not new
or introduced by the removal of the lock_kernel logic.
Signed-off-by: Alan Cox <alan@linux.intel.com>
---
drivers/char/istallion.c | 24 +++++++++++++++---------
drivers/char/stallion.c | 20 ++++++++++++--------
2 files changed, 27 insertions(+), 17 deletions(-)
diff --git a/drivers/char/istallion.c b/drivers/char/istallion.c
index 4cd6c52..3699e30 100644
--- a/drivers/char/istallion.c
+++ b/drivers/char/istallion.c
@@ -14,6 +14,7 @@
* the Free Software Foundation; either version 2 of the License, or
* (at your option) any later version.
*
+ * FIXME: brdp->state needs proper locking.
*/
/*****************************************************************************/
@@ -4009,6 +4010,8 @@ static int stli_getbrdstats(combrd_t __user *bp)
return -ENODEV;
memset(&stli_brdstats, 0, sizeof(combrd_t));
+
+ mutex_lock(&portp->port.mutex);
stli_brdstats.brd = brdp->brdnr;
stli_brdstats.type = brdp->brdtype;
stli_brdstats.hwid = 0;
@@ -4022,6 +4025,7 @@ static int stli_getbrdstats(combrd_t __user *bp)
stli_brdstats.panels[i].hwid = brdp->panelids[i];
stli_brdstats.panels[i].nrports = brdp->panels[i];
}
+ mutex_unlock(&portp->port.mutex);
if (copy_to_user(bp, &stli_brdstats, sizeof(combrd_t)))
return -EFAULT;
@@ -4074,10 +4078,13 @@ static int stli_portcmdstats(struct tty_struct *tty, struct stliport *portp)
if (brdp == NULL)
return -ENODEV;
+ mutex_lock(&portp->port.mutex);
if (brdp->state & BST_STARTED) {
if ((rc = stli_cmdwait(brdp, portp, A_GETSTATS,
- &stli_cdkstats, sizeof(asystats_t), 1)) < 0)
+ &stli_cdkstats, sizeof(asystats_t), 1)) < 0) {
+ mutex_unlock(&portp->port.mutex);
return rc;
+ }
} else {
memset(&stli_cdkstats, 0, sizeof(asystats_t));
}
@@ -4122,6 +4129,7 @@ static int stli_portcmdstats(struct tty_struct *tty, struct stliport *portp)
stli_comstats.modem = stli_cdkstats.dcdcnt;
stli_comstats.hwid = stli_cdkstats.hwid;
stli_comstats.signals = stli_mktiocm(stli_cdkstats.signals);
+ mutex_unlock(&portp->port.mutex);
return 0;
}
@@ -4184,15 +4192,20 @@ static int stli_clrportstats(struct stliport *portp, comstats_t __user *cp)
if (!brdp)
return -ENODEV;
+ mutex_lock(&portp->port.mutex);
+
if (brdp->state & BST_STARTED) {
- if ((rc = stli_cmdwait(brdp, portp, A_CLEARSTATS, NULL, 0, 0)) < 0)
+ if ((rc = stli_cmdwait(brdp, portp, A_CLEARSTATS, NULL, 0, 0)) < 0) {
+ mutex_unlock(&portp->port.mutex);
return rc;
+ }
}
memset(&stli_comstats, 0, sizeof(comstats_t));
stli_comstats.brd = portp->brdnr;
stli_comstats.panel = portp->panelnr;
stli_comstats.port = portp->portnr;
+ mutex_unlock(&portp->port.mutex);
if (copy_to_user(cp, &stli_comstats, sizeof(comstats_t)))
return -EFAULT;
@@ -4264,8 +4277,6 @@ static long stli_memioctl(struct file *fp, unsigned int cmd, unsigned long arg)
done = 0;
rc = 0;
- lock_kernel();
-
switch (cmd) {
case COM_GETPORTSTATS:
rc = stli_getportstats(NULL, NULL, argp);
@@ -4288,8 +4299,6 @@ static long stli_memioctl(struct file *fp, unsigned int cmd, unsigned long arg)
done++;
break;
}
- unlock_kernel();
-
if (done)
return rc;
@@ -4306,8 +4315,6 @@ static long stli_memioctl(struct file *fp, unsigned int cmd, unsigned long arg)
if (brdp->state == 0)
return -ENODEV;
- lock_kernel();
-
switch (cmd) {
case STL_BINTR:
EBRDINTR(brdp);
@@ -4330,7 +4337,6 @@ static long stli_memioctl(struct file *fp, unsigned int cmd, unsigned long arg)
rc = -ENOIOCTLCMD;
break;
}
- unlock_kernel();
return rc;
}
diff --git a/drivers/char/stallion.c b/drivers/char/stallion.c
index 0e511d6..db5c418 100644
--- a/drivers/char/stallion.c
+++ b/drivers/char/stallion.c
@@ -807,7 +807,6 @@ static void stl_waituntilsent(struct tty_struct *tty, int timeout)
timeout = HZ;
tend = jiffies + timeout;
- lock_kernel();
while (stl_datastate(portp)) {
if (signal_pending(current))
break;
@@ -815,7 +814,6 @@ static void stl_waituntilsent(struct tty_struct *tty, int timeout)
if (time_after_eq(jiffies, tend))
break;
}
- unlock_kernel();
}
/*****************************************************************************/
@@ -1028,6 +1026,8 @@ static int stl_getserial(struct stlport *portp, struct serial_struct __user *sp)
pr_debug("stl_getserial(portp=%p,sp=%p)\n", portp, sp);
memset(&sio, 0, sizeof(struct serial_struct));
+
+ mutex_lock(&portp->port.mutex);
sio.line = portp->portnr;
sio.port = portp->ioaddr;
sio.flags = portp->port.flags;
@@ -1047,6 +1047,7 @@ static int stl_getserial(struct stlport *portp, struct serial_struct __user *sp)
brdp = stl_brds[portp->brdnr];
if (brdp != NULL)
sio.irq = brdp->irq;
+ mutex_unlock(&portp->port.mutex);
return copy_to_user(sp, &sio, sizeof(struct serial_struct)) ? -EFAULT : 0;
}
@@ -1068,12 +1069,15 @@ static int stl_setserial(struct tty_struct *tty, struct serial_struct __user *sp
if (copy_from_user(&sio, sp, sizeof(struct serial_struct)))
return -EFAULT;
+ mutex_lock(&portp->port.mutex);
if (!capable(CAP_SYS_ADMIN)) {
if ((sio.baud_base != portp->baud_base) ||
(sio.close_delay != portp->close_delay) ||
((sio.flags & ~ASYNC_USR_MASK) !=
- (portp->port.flags & ~ASYNC_USR_MASK)))
+ (portp->port.flags & ~ASYNC_USR_MASK))) {
+ mutex_unlock(&portp->port.mutex);
return -EPERM;
+ }
}
portp->port.flags = (portp->port.flags & ~ASYNC_USR_MASK) |
@@ -1082,6 +1086,7 @@ static int stl_setserial(struct tty_struct *tty, struct serial_struct __user *sp
portp->close_delay = sio.close_delay;
portp->closing_wait = sio.closing_wait;
portp->custom_divisor = sio.custom_divisor;
+ mutex_unlock(&portp->port.mutex);
stl_setport(portp, tty->termios);
return 0;
}
@@ -1146,8 +1151,6 @@ static int stl_ioctl(struct tty_struct *tty, struct file *file, unsigned int cmd
rc = 0;
- lock_kernel();
-
switch (cmd) {
case TIOCGSERIAL:
rc = stl_getserial(portp, argp);
@@ -1172,7 +1175,6 @@ static int stl_ioctl(struct tty_struct *tty, struct file *file, unsigned int cmd
rc = -ENOIOCTLCMD;
break;
}
- unlock_kernel();
return rc;
}
@@ -2326,6 +2328,7 @@ static int stl_getportstats(struct tty_struct *tty, struct stlport *portp, comst
return -ENODEV;
}
+ mutex_lock(&portp->port.mutex);
portp->stats.state = portp->istate;
portp->stats.flags = portp->port.flags;
portp->stats.hwid = portp->hwid;
@@ -2357,6 +2360,7 @@ static int stl_getportstats(struct tty_struct *tty, struct stlport *portp, comst
(STL_TXBUFSIZE - (tail - head));
portp->stats.signals = (unsigned long) stl_getsignals(portp);
+ mutex_unlock(&portp->port.mutex);
return copy_to_user(cp, &portp->stats,
sizeof(comstats_t)) ? -EFAULT : 0;
@@ -2381,10 +2385,12 @@ static int stl_clrportstats(struct stlport *portp, comstats_t __user *cp)
return -ENODEV;
}
+ mutex_lock(&portp->port.mutex);
memset(&portp->stats, 0, sizeof(comstats_t));
portp->stats.brd = portp->brdnr;
portp->stats.panel = portp->panelnr;
portp->stats.port = portp->portnr;
+ mutex_unlock(&portp->port.mutex);
return copy_to_user(cp, &portp->stats,
sizeof(comstats_t)) ? -EFAULT : 0;
}
@@ -2450,7 +2456,6 @@ static long stl_memioctl(struct file *fp, unsigned int cmd, unsigned long arg)
return -ENODEV;
rc = 0;
- lock_kernel();
switch (cmd) {
case COM_GETPORTSTATS:
rc = stl_getportstats(NULL, NULL, argp);
@@ -2471,7 +2476,6 @@ static long stl_memioctl(struct file *fp, unsigned int cmd, unsigned long arg)
rc = -ENOIOCTLCMD;
break;
}
- unlock_kernel();
return rc;
}
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH 05/14] stallion: prune lock_kernel calls
2010-04-01 16:05 ` [PATCH 05/14] stallion: prune lock_kernel calls Alan Cox
@ 2010-04-29 21:29 ` Arnd Bergmann
2010-04-30 19:32 ` Alan Cox
0 siblings, 1 reply; 18+ messages in thread
From: Arnd Bergmann @ 2010-04-29 21:29 UTC (permalink / raw)
To: Alan Cox; +Cc: linux-kernel
On Thursday 01 April 2010 18:05:19 Alan Cox wrote:
> @@ -4009,6 +4010,8 @@ static int stli_getbrdstats(combrd_t __user *bp)
> return -ENODEV;
>
> memset(&stli_brdstats, 0, sizeof(combrd_t));
> +
> + mutex_lock(&portp->port.mutex);
> stli_brdstats.brd = brdp->brdnr;
> stli_brdstats.type = brdp->brdtype;
> stli_brdstats.hwid = 0;
> @@ -4022,6 +4025,7 @@ static int stli_getbrdstats(combrd_t __user *bp)
> stli_brdstats.panels[i].hwid = brdp->panelids[i];
> stli_brdstats.panels[i].nrports = brdp->panels[i];
> }
> + mutex_unlock(&portp->port.mutex);
>
> if (copy_to_user(bp, &stli_brdstats, sizeof(combrd_t)))
> return -EFAULT;
I just tried porting my patches on top of yours (yes, it's been
some time). This one doesn't work, because there is no portp variable
in stli_getbrdstats.
On a related topic, in patch 06/14 "istallion: use bit ops for the board flags",
you have
> @@ -817,7 +817,7 @@ static int stli_open(struct tty_struct *tty, struct file *filp)
> brdp = stli_brds[brdnr];
> if (brdp == NULL)
> return -ENODEV;
> - if ((brdp->state & BST_STARTED) == 0)
> + if (!(test_bit(BST_STARTED, &brdp->state))
> return -ENODEV;
> portnr = MINOR2PORT(minordev);
> if (portnr > brdp->nrports)
This won't work because brdp->state is an 'unsigned int' variable, while
bitops only work on 'unsigned long'.
Maybe you sent out a wrong version of this file?
Arnd
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 05/14] stallion: prune lock_kernel calls
2010-04-29 21:29 ` Arnd Bergmann
@ 2010-04-30 19:32 ` Alan Cox
0 siblings, 0 replies; 18+ messages in thread
From: Alan Cox @ 2010-04-30 19:32 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: Alan Cox, linux-kernel
> This won't work because brdp->state is an 'unsigned int' variable, while
> bitops only work on 'unsigned long'.
32bit x86 only code - but I have now updated the patch to fix this bug
> Maybe you sent out a wrong version of this file?
No it seems I forgot to build the stallion driver when testing the
patches.
Alan
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 06/14] istallion: use bit ops for the board flags
2010-04-01 16:04 [RFC PATCH 00/14] Some BKL killing in the serial drivers Alan Cox
` (4 preceding siblings ...)
2010-04-01 16:05 ` [PATCH 05/14] stallion: prune lock_kernel calls Alan Cox
@ 2010-04-01 16:05 ` Alan Cox
2010-04-01 16:05 ` [PATCH 07/14] riscom8: kill use of lock_kernel Alan Cox
` (7 subsequent siblings)
13 siblings, 0 replies; 18+ messages in thread
From: Alan Cox @ 2010-04-01 16:05 UTC (permalink / raw)
To: linux-kernel
This lets us avoid problems with races on the flag changes
Signed-off-by: Alan Cox <alan@linux.intel.com>
---
drivers/char/istallion.c | 35 ++++++++++++++++++-----------------
1 files changed, 18 insertions(+), 17 deletions(-)
diff --git a/drivers/char/istallion.c b/drivers/char/istallion.c
index 3699e30..1937e61 100644
--- a/drivers/char/istallion.c
+++ b/drivers/char/istallion.c
@@ -204,9 +204,9 @@ static int stli_shared;
* the board has been detected, and whether it is actually running a slave
* or not.
*/
-#define BST_FOUND 0x1
-#define BST_STARTED 0x2
-#define BST_PROBED 0x4
+#define BST_FOUND 0
+#define BST_STARTED 1
+#define BST_PROBED 2
/*
* Define the set of port state flags. These are marked for internal
@@ -817,7 +817,7 @@ static int stli_open(struct tty_struct *tty, struct file *filp)
brdp = stli_brds[brdnr];
if (brdp == NULL)
return -ENODEV;
- if ((brdp->state & BST_STARTED) == 0)
+ if (!(test_bit(BST_STARTED, &brdp->state))
return -ENODEV;
portnr = MINOR2PORT(minordev);
if (portnr > brdp->nrports)
@@ -1845,7 +1845,7 @@ static void stli_portinfo(struct seq_file *m, struct stlibrd *brdp, struct stlip
rc = stli_portcmdstats(NULL, portp);
uart = "UNKNOWN";
- if (brdp->state & BST_STARTED) {
+ if (test_bit(BST_STARTED, &brdp->state)) {
switch (stli_comstats.hwid) {
case 0: uart = "2681"; break;
case 1: uart = "SC26198"; break;
@@ -1854,7 +1854,7 @@ static void stli_portinfo(struct seq_file *m, struct stlibrd *brdp, struct stlip
}
seq_printf(m, "%d: uart:%s ", portnr, uart);
- if ((brdp->state & BST_STARTED) && (rc >= 0)) {
+ if (test_bit(BST_STARTED, &brdp->state) && rc >= 0) {
char sep;
seq_printf(m, "tx:%d rx:%d", (int) stli_comstats.txtotal,
@@ -2354,7 +2354,7 @@ static void stli_poll(unsigned long arg)
brdp = stli_brds[brdnr];
if (brdp == NULL)
continue;
- if ((brdp->state & BST_STARTED) == 0)
+ if (!test_bit(BST_STARTED, &brdp->state))
continue;
spin_lock(&brd_lock);
@@ -3139,7 +3139,7 @@ static int stli_initecp(struct stlibrd *brdp)
}
- brdp->state |= BST_FOUND;
+ set_bit(BST_FOUND, &brdp->state);
return 0;
err_unmap:
iounmap(brdp->membase);
@@ -3296,7 +3296,7 @@ static int stli_initonb(struct stlibrd *brdp)
brdp->panels[0] = brdp->nrports;
- brdp->state |= BST_FOUND;
+ set_bit(BST_FOUND, &brdp->state);
return 0;
err_unmap:
iounmap(brdp->membase);
@@ -3406,7 +3406,7 @@ stli_donestartup:
spin_unlock_irqrestore(&brd_lock, flags);
if (rc == 0)
- brdp->state |= BST_STARTED;
+ set_bit(BST_STARTED, &brdp->state);
if (! stli_timeron) {
stli_timeron++;
@@ -3709,7 +3709,7 @@ static int __devinit stli_pciprobe(struct pci_dev *pdev,
if (retval)
goto err_null;
- brdp->state |= BST_PROBED;
+ set_bit(BST_PROBED, &brdp->state);
pci_set_drvdata(pdev, brdp);
EBRDENABLE(brdp);
@@ -3840,7 +3840,7 @@ static int __init stli_initbrds(void)
brdp = stli_brds[i];
if (brdp == NULL)
continue;
- if (brdp->state & BST_FOUND) {
+ if (test_bit(BST_FOUND, &brdp->state)) {
EBRDENABLE(brdp);
brdp->enable = NULL;
brdp->disable = NULL;
@@ -4079,7 +4079,7 @@ static int stli_portcmdstats(struct tty_struct *tty, struct stliport *portp)
return -ENODEV;
mutex_lock(&portp->port.mutex);
- if (brdp->state & BST_STARTED) {
+ if (test_bit(BST_STARTED, &brdp->state)) {
if ((rc = stli_cmdwait(brdp, portp, A_GETSTATS,
&stli_cdkstats, sizeof(asystats_t), 1)) < 0) {
mutex_unlock(&portp->port.mutex);
@@ -4194,7 +4194,7 @@ static int stli_clrportstats(struct stliport *portp, comstats_t __user *cp)
mutex_lock(&portp->port.mutex);
- if (brdp->state & BST_STARTED) {
+ if (test_bit(BST_STARTED, &brdp->state)) {
if ((rc = stli_cmdwait(brdp, portp, A_CLEARSTATS, NULL, 0, 0)) < 0) {
mutex_unlock(&portp->port.mutex);
return rc;
@@ -4323,10 +4323,10 @@ static long stli_memioctl(struct file *fp, unsigned int cmd, unsigned long arg)
rc = stli_startbrd(brdp);
break;
case STL_BSTOP:
- brdp->state &= ~BST_STARTED;
+ clear_bit(BST_STARTED, &brdp->state);
break;
case STL_BRESET:
- brdp->state &= ~BST_STARTED;
+ clear_bit(BST_STARTED, &brdp->state);
EBRDRESET(brdp);
if (stli_shared == 0) {
if (brdp->reenable != NULL)
@@ -4382,7 +4382,8 @@ static void istallion_cleanup_isa(void)
unsigned int j;
for (j = 0; (j < stli_nrbrds); j++) {
- if ((brdp = stli_brds[j]) == NULL || (brdp->state & BST_PROBED))
+ if ((brdp = stli_brds[j]) == NULL ||
+ test_bit(BST_PROBED, &brdp->state))
continue;
stli_cleanup_ports(brdp);
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 07/14] riscom8: kill use of lock_kernel
2010-04-01 16:04 [RFC PATCH 00/14] Some BKL killing in the serial drivers Alan Cox
` (5 preceding siblings ...)
2010-04-01 16:05 ` [PATCH 06/14] istallion: use bit ops for the board flags Alan Cox
@ 2010-04-01 16:05 ` Alan Cox
2010-04-01 16:05 ` [PATCH 08/14] isicom: kill off the BKL Alan Cox
` (6 subsequent siblings)
13 siblings, 0 replies; 18+ messages in thread
From: Alan Cox @ 2010-04-01 16:05 UTC (permalink / raw)
To: linux-kernel
The riscom8 board uses lock_kernel to protect bits of the port setting
ioctl logic. We can use the port mutex for this as the logic is internal
and will also lock set versus open (a locking property that has been lost
somewhere along the way)
Signed-off-by: Alan Cox <alan@linux.intel.com>
---
drivers/char/riscom8.c | 14 ++++++++------
1 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/char/riscom8.c b/drivers/char/riscom8.c
index 0a8d1e5..9c00b7d 100644
--- a/drivers/char/riscom8.c
+++ b/drivers/char/riscom8.c
@@ -47,7 +47,6 @@
#include <linux/init.h>
#include <linux/delay.h>
#include <linux/tty_flip.h>
-#include <linux/smp_lock.h>
#include <linux/spinlock.h>
#include <linux/device.h>
@@ -1183,6 +1182,7 @@ static int rc_set_serial_info(struct tty_struct *tty, struct riscom_port *port,
if (copy_from_user(&tmp, newinfo, sizeof(tmp)))
return -EFAULT;
+ mutex_lock(&port->port.mutex);
change_speed = ((port->port.flags & ASYNC_SPD_MASK) !=
(tmp.flags & ASYNC_SPD_MASK));
@@ -1190,8 +1190,10 @@ static int rc_set_serial_info(struct tty_struct *tty, struct riscom_port *port,
if ((tmp.close_delay != port->port.close_delay) ||
(tmp.closing_wait != port->port.closing_wait) ||
((tmp.flags & ~ASYNC_USR_MASK) !=
- (port->port.flags & ~ASYNC_USR_MASK)))
+ (port->port.flags & ~ASYNC_USR_MASK))) {
+ mutex_unlock(&port->port.mutex);
return -EPERM;
+ }
port->port.flags = ((port->port.flags & ~ASYNC_USR_MASK) |
(tmp.flags & ASYNC_USR_MASK));
} else {
@@ -1207,6 +1209,7 @@ static int rc_set_serial_info(struct tty_struct *tty, struct riscom_port *port,
rc_change_speed(tty, bp, port);
spin_unlock_irqrestore(&riscom_lock, flags);
}
+ mutex_unlock(&port->port.mutex);
return 0;
}
@@ -1219,12 +1222,15 @@ static int rc_get_serial_info(struct riscom_port *port,
memset(&tmp, 0, sizeof(tmp));
tmp.type = PORT_CIRRUS;
tmp.line = port - rc_port;
+
+ mutex_lock(&port->port.mutex);
tmp.port = bp->base;
tmp.irq = bp->irq;
tmp.flags = port->port.flags;
tmp.baud_base = (RC_OSCFREQ + CD180_TPC/2) / CD180_TPC;
tmp.close_delay = port->port.close_delay * HZ/100;
tmp.closing_wait = port->port.closing_wait * HZ/100;
+ mutex_unlock(&port->port.mutex);
tmp.xmit_fifo_size = CD180_NFIFO;
return copy_to_user(retinfo, &tmp, sizeof(tmp)) ? -EFAULT : 0;
}
@@ -1241,14 +1247,10 @@ static int rc_ioctl(struct tty_struct *tty, struct file *filp,
switch (cmd) {
case TIOCGSERIAL:
- lock_kernel();
retval = rc_get_serial_info(port, argp);
- unlock_kernel();
break;
case TIOCSSERIAL:
- lock_kernel();
retval = rc_set_serial_info(tty, port, argp);
- unlock_kernel();
break;
default:
retval = -ENOIOCTLCMD;
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 08/14] isicom: kill off the BKL
2010-04-01 16:04 [RFC PATCH 00/14] Some BKL killing in the serial drivers Alan Cox
` (6 preceding siblings ...)
2010-04-01 16:05 ` [PATCH 07/14] riscom8: kill use of lock_kernel Alan Cox
@ 2010-04-01 16:05 ` Alan Cox
2010-04-01 16:06 ` [PATCH 09/14] rocket: kill BKL Alan Cox
` (5 subsequent siblings)
13 siblings, 0 replies; 18+ messages in thread
From: Alan Cox @ 2010-04-01 16:05 UTC (permalink / raw)
To: linux-kernel
As with the others we can use the port mutex to get the needed locking
properties and fix the race with open.
Signed-off-by: Alan Cox <alan@linux.intel.com>
---
drivers/char/isicom.c | 12 +++++-------
1 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/drivers/char/isicom.c b/drivers/char/isicom.c
index 0fa2e4a..989dd4b 100644
--- a/drivers/char/isicom.c
+++ b/drivers/char/isicom.c
@@ -124,7 +124,6 @@
#include <linux/fs.h>
#include <linux/sched.h>
#include <linux/serial.h>
-#include <linux/smp_lock.h>
#include <linux/mm.h>
#include <linux/interrupt.h>
#include <linux/timer.h>
@@ -1113,8 +1112,7 @@ static int isicom_set_serial_info(struct tty_struct *tty,
if (copy_from_user(&newinfo, info, sizeof(newinfo)))
return -EFAULT;
- lock_kernel();
-
+ mutex_lock(&port->port.mutex);
reconfig_port = ((port->port.flags & ASYNC_SPD_MASK) !=
(newinfo.flags & ASYNC_SPD_MASK));
@@ -1123,7 +1121,7 @@ static int isicom_set_serial_info(struct tty_struct *tty,
(newinfo.closing_wait != port->port.closing_wait) ||
((newinfo.flags & ~ASYNC_USR_MASK) !=
(port->port.flags & ~ASYNC_USR_MASK))) {
- unlock_kernel();
+ mutex_unlock(&port->port.mutex);
return -EPERM;
}
port->port.flags = ((port->port.flags & ~ASYNC_USR_MASK) |
@@ -1140,7 +1138,7 @@ static int isicom_set_serial_info(struct tty_struct *tty,
isicom_config_port(tty);
spin_unlock_irqrestore(&port->card->card_lock, flags);
}
- unlock_kernel();
+ mutex_unlock(&port->port.mutex);
return 0;
}
@@ -1149,7 +1147,7 @@ static int isicom_get_serial_info(struct isi_port *port,
{
struct serial_struct out_info;
- lock_kernel();
+ mutex_lock(&port->port.mutex);
memset(&out_info, 0, sizeof(out_info));
/* out_info.type = ? */
out_info.line = port - isi_ports;
@@ -1159,7 +1157,7 @@ static int isicom_get_serial_info(struct isi_port *port,
/* out_info.baud_base = ? */
out_info.close_delay = port->port.close_delay;
out_info.closing_wait = port->port.closing_wait;
- unlock_kernel();
+ mutex_unlock(&port->port.mutex);
if (copy_to_user(info, &out_info, sizeof(out_info)))
return -EFAULT;
return 0;
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 09/14] rocket: kill BKL
2010-04-01 16:04 [RFC PATCH 00/14] Some BKL killing in the serial drivers Alan Cox
` (7 preceding siblings ...)
2010-04-01 16:05 ` [PATCH 08/14] isicom: kill off the BKL Alan Cox
@ 2010-04-01 16:06 ` Alan Cox
2010-04-01 16:06 ` [PATCH 10/14] synclink: kill the big kernel lock Alan Cox
` (4 subsequent siblings)
13 siblings, 0 replies; 18+ messages in thread
From: Alan Cox @ 2010-04-01 16:06 UTC (permalink / raw)
To: linux-kernel
We can use the port mutex for this and also for the hangup path so removing
the problematic use of the BKL in this driver. Fix up the locking
on the various port flags while we are at it.
Ultimately this driver needs to be using tty_port_ helpers which would sort
this out far better.
Signed-off-by: Alan Cox <alan@linux.intel.com>
---
drivers/char/rocket.c | 28 +++++++++++++++++++---------
1 files changed, 19 insertions(+), 9 deletions(-)
diff --git a/drivers/char/rocket.c b/drivers/char/rocket.c
index 0e29a23..79c3bc6 100644
--- a/drivers/char/rocket.c
+++ b/drivers/char/rocket.c
@@ -73,7 +73,6 @@
#include <linux/tty_driver.h>
#include <linux/tty_flip.h>
#include <linux/serial.h>
-#include <linux/smp_lock.h>
#include <linux/string.h>
#include <linux/fcntl.h>
#include <linux/ptrace.h>
@@ -1017,6 +1016,7 @@ static void rp_close(struct tty_struct *tty, struct file *filp)
if (tty_port_close_start(port, tty, filp) == 0)
return;
+ mutex_lock(&port->mutex);
cp = &info->channel;
/*
* Before we drop DTR, make sure the UART transmitter
@@ -1060,9 +1060,13 @@ static void rp_close(struct tty_struct *tty, struct file *filp)
info->xmit_buf = NULL;
}
}
+ spin_lock_irq(&port->lock);
info->port.flags &= ~(ASYNC_INITIALIZED | ASYNC_CLOSING | ASYNC_NORMAL_ACTIVE);
tty->closing = 0;
+ spin_unlock_irq(&port->lock);
+ mutex_unlock(&port->mutex);
tty_port_tty_set(port, NULL);
+
wake_up_interruptible(&port->close_wait);
complete_all(&info->close_wait);
atomic_dec(&rp_num_ports_open);
@@ -1210,11 +1214,13 @@ static int get_config(struct r_port *info, struct rocket_config __user *retinfo)
if (!retinfo)
return -EFAULT;
memset(&tmp, 0, sizeof (tmp));
+ mutex_lock(&info->port.mutex);
tmp.line = info->line;
tmp.flags = info->flags;
tmp.close_delay = info->port.close_delay;
tmp.closing_wait = info->port.closing_wait;
tmp.port = rcktpt_io_addr[(info->line >> 5) & 3];
+ mutex_unlock(&info->port.mutex);
if (copy_to_user(retinfo, &tmp, sizeof (*retinfo)))
return -EFAULT;
@@ -1229,10 +1235,13 @@ static int set_config(struct tty_struct *tty, struct r_port *info,
if (copy_from_user(&new_serial, new_info, sizeof (new_serial)))
return -EFAULT;
+ mutex_lock(&info->port.mutex);
if (!capable(CAP_SYS_ADMIN))
{
- if ((new_serial.flags & ~ROCKET_USR_MASK) != (info->flags & ~ROCKET_USR_MASK))
+ if ((new_serial.flags & ~ROCKET_USR_MASK) != (info->flags & ~ROCKET_USR_MASK)) {
+ mutex_unlock(&info->port.mutex);
return -EPERM;
+ }
info->flags = ((info->flags & ~ROCKET_USR_MASK) | (new_serial.flags & ROCKET_USR_MASK));
configure_r_port(tty, info, NULL);
return 0;
@@ -1250,6 +1259,7 @@ static int set_config(struct tty_struct *tty, struct r_port *info,
tty->alt_speed = 230400;
if ((info->flags & ROCKET_SPD_MASK) == ROCKET_SPD_WARP)
tty->alt_speed = 460800;
+ mutex_unlock(&info->port.mutex);
configure_r_port(tty, info, NULL);
return 0;
@@ -1325,8 +1335,6 @@ static int rp_ioctl(struct tty_struct *tty, struct file *file,
if (cmd != RCKP_GET_PORTS && rocket_paranoia_check(info, "rp_ioctl"))
return -ENXIO;
- lock_kernel();
-
switch (cmd) {
case RCKP_GET_STRUCT:
if (copy_to_user(argp, info, sizeof (struct r_port)))
@@ -1350,7 +1358,6 @@ static int rp_ioctl(struct tty_struct *tty, struct file *file,
default:
ret = -ENOIOCTLCMD;
}
- unlock_kernel();
return ret;
}
@@ -1471,7 +1478,6 @@ static void rp_wait_until_sent(struct tty_struct *tty, int timeout)
jiffies);
printk(KERN_INFO "cps=%d...\n", info->cps);
#endif
- lock_kernel();
while (1) {
txcnt = sGetTxCnt(cp);
if (!txcnt) {
@@ -1499,7 +1505,6 @@ static void rp_wait_until_sent(struct tty_struct *tty, int timeout)
break;
}
__set_current_state(TASK_RUNNING);
- unlock_kernel();
#ifdef ROCKET_DEBUG_WAIT_UNTIL_SENT
printk(KERN_INFO "txcnt = %d (jiff=%lu)...done\n", txcnt, jiffies);
#endif
@@ -1512,6 +1517,7 @@ static void rp_hangup(struct tty_struct *tty)
{
CHANNEL_t *cp;
struct r_port *info = tty->driver_data;
+ unsigned long flags;
if (rocket_paranoia_check(info, "rp_hangup"))
return;
@@ -1520,11 +1526,15 @@ static void rp_hangup(struct tty_struct *tty)
printk(KERN_INFO "rp_hangup of ttyR%d...\n", info->line);
#endif
rp_flush_buffer(tty);
- if (info->port.flags & ASYNC_CLOSING)
+ spin_lock_irqsave(&info->port.lock, flags);
+ if (info->port.flags & ASYNC_CLOSING) {
+ spin_unlock_irqrestore(&info->port.lock, flags);
return;
+ }
if (info->port.count)
atomic_dec(&rp_num_ports_open);
clear_bit((info->aiop * 8) + info->chan, (void *) &xmit_flags[info->board]);
+ spin_unlock_irqrestore(&info->port.lock, flags);
tty_port_hangup(&info->port);
@@ -1535,7 +1545,7 @@ static void rp_hangup(struct tty_struct *tty)
sDisCTSFlowCtl(cp);
sDisTxSoftFlowCtl(cp);
sClrTxXOFF(cp);
- info->port.flags &= ~ASYNC_INITIALIZED;
+ clear_bit(ASYNCB_INITIALIZED, &info->port.flags);
wake_up_interruptible(&info->port.open_wait);
}
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 10/14] synclink: kill the big kernel lock
2010-04-01 16:04 [RFC PATCH 00/14] Some BKL killing in the serial drivers Alan Cox
` (8 preceding siblings ...)
2010-04-01 16:06 ` [PATCH 09/14] rocket: kill BKL Alan Cox
@ 2010-04-01 16:06 ` Alan Cox
2010-04-01 16:06 ` [PATCH 11/14] cyclades: Kill off BKL usage Alan Cox
` (3 subsequent siblings)
13 siblings, 0 replies; 18+ messages in thread
From: Alan Cox @ 2010-04-01 16:06 UTC (permalink / raw)
To: linux-kernel
We don't need it while waiting and we can lock the ioctls using the port
mutex.
Signed-off-by: Alan Cox <alan@linux.intel.com>
---
drivers/char/synclink.c | 19 ++++++-----
drivers/char/synclink_gt.c | 78 +++++++++++++++++++-------------------------
drivers/char/synclinkmp.c | 32 +++++++-----------
3 files changed, 56 insertions(+), 73 deletions(-)
diff --git a/drivers/char/synclink.c b/drivers/char/synclink.c
index 0658fc5..4de1246 100644
--- a/drivers/char/synclink.c
+++ b/drivers/char/synclink.c
@@ -81,7 +81,6 @@
#include <linux/mm.h>
#include <linux/seq_file.h>
#include <linux/slab.h>
-#include <linux/smp_lock.h>
#include <linux/delay.h>
#include <linux/netdevice.h>
#include <linux/vmalloc.h>
@@ -2436,7 +2435,9 @@ static int mgsl_get_stats(struct mgsl_struct * info, struct mgsl_icount __user *
if (!user_icount) {
memset(&info->icount, 0, sizeof(info->icount));
} else {
+ mutex_lock(&info->port.mutex);
COPY_TO_USER(err, user_icount, &info->icount, sizeof(struct mgsl_icount));
+ mutex_unlock(&info->port.mutex);
if (err)
return -EFAULT;
}
@@ -2461,7 +2462,9 @@ static int mgsl_get_params(struct mgsl_struct * info, MGSL_PARAMS __user *user_p
printk("%s(%d):mgsl_get_params(%s)\n",
__FILE__,__LINE__, info->device_name);
+ mutex_lock(&info->port.mutex);
COPY_TO_USER(err,user_params, &info->params, sizeof(MGSL_PARAMS));
+ mutex_unlock(&info->port.mutex);
if (err) {
if ( debug_level >= DEBUG_LEVEL_INFO )
printk( "%s(%d):mgsl_get_params(%s) user buffer copy failed\n",
@@ -2501,11 +2504,13 @@ static int mgsl_set_params(struct mgsl_struct * info, MGSL_PARAMS __user *new_pa
return -EFAULT;
}
+ mutex_lock(&info->port.mutex);
spin_lock_irqsave(&info->irq_spinlock,flags);
memcpy(&info->params,&tmp_params,sizeof(MGSL_PARAMS));
spin_unlock_irqrestore(&info->irq_spinlock,flags);
mgsl_change_params(info);
+ mutex_unlock(&info->port.mutex);
return 0;
@@ -2935,7 +2940,6 @@ static int mgsl_ioctl(struct tty_struct *tty, struct file * file,
unsigned int cmd, unsigned long arg)
{
struct mgsl_struct * info = tty->driver_data;
- int ret;
if (debug_level >= DEBUG_LEVEL_INFO)
printk("%s(%d):mgsl_ioctl %s cmd=%08X\n", __FILE__,__LINE__,
@@ -2950,10 +2954,7 @@ static int mgsl_ioctl(struct tty_struct *tty, struct file * file,
return -EIO;
}
- lock_kernel();
- ret = mgsl_ioctl_common(info, cmd, arg);
- unlock_kernel();
- return ret;
+ return mgsl_ioctl_common(info, cmd, arg);
}
static int mgsl_ioctl_common(struct mgsl_struct *info, unsigned int cmd, unsigned long arg)
@@ -3109,12 +3110,14 @@ static void mgsl_close(struct tty_struct *tty, struct file * filp)
if (tty_port_close_start(&info->port, tty, filp) == 0)
goto cleanup;
-
+
+ mutex_lock(&info->port.mutex);
if (info->port.flags & ASYNC_INITIALIZED)
mgsl_wait_until_sent(tty, info->timeout);
mgsl_flush_buffer(tty);
tty_ldisc_flush(tty);
shutdown(info);
+ mutex_unlock(&info->port.mutex);
tty_port_close_end(&info->port, tty);
info->port.tty = NULL;
@@ -3162,7 +3165,6 @@ static void mgsl_wait_until_sent(struct tty_struct *tty, int timeout)
* Note: use tight timings here to satisfy the NIST-PCTS.
*/
- lock_kernel();
if ( info->params.data_rate ) {
char_time = info->timeout/(32 * 5);
if (!char_time)
@@ -3192,7 +3194,6 @@ static void mgsl_wait_until_sent(struct tty_struct *tty, int timeout)
break;
}
}
- unlock_kernel();
exit:
if (debug_level >= DEBUG_LEVEL_INFO)
diff --git a/drivers/char/synclink_gt.c b/drivers/char/synclink_gt.c
index 4561ce2..d0f81f5 100644
--- a/drivers/char/synclink_gt.c
+++ b/drivers/char/synclink_gt.c
@@ -40,8 +40,8 @@
#define DBGBH(fmt) if (debug_level >= DEBUG_LEVEL_BH) printk fmt
#define DBGISR(fmt) if (debug_level >= DEBUG_LEVEL_ISR) printk fmt
#define DBGDATA(info, buf, size, label) if (debug_level >= DEBUG_LEVEL_DATA) trace_block((info), (buf), (size), (label))
-//#define DBGTBUF(info) dump_tbufs(info)
-//#define DBGRBUF(info) dump_rbufs(info)
+/*#define DBGTBUF(info) dump_tbufs(info)*/
+/*#define DBGRBUF(info) dump_rbufs(info)*/
#include <linux/module.h>
@@ -62,7 +62,6 @@
#include <linux/mm.h>
#include <linux/seq_file.h>
#include <linux/slab.h>
-#include <linux/smp_lock.h>
#include <linux/netdevice.h>
#include <linux/vmalloc.h>
#include <linux/init.h>
@@ -901,8 +900,6 @@ static void wait_until_sent(struct tty_struct *tty, int timeout)
* Note: use tight timings here to satisfy the NIST-PCTS.
*/
- lock_kernel();
-
if (info->params.data_rate) {
char_time = info->timeout/(32 * 5);
if (!char_time)
@@ -920,8 +917,6 @@ static void wait_until_sent(struct tty_struct *tty, int timeout)
if (timeout && time_after(jiffies, orig_jiffies + timeout))
break;
}
- unlock_kernel();
-
exit:
DBGINFO(("%s wait_until_sent exit\n", info->device_name));
}
@@ -1041,8 +1036,37 @@ static int ioctl(struct tty_struct *tty, struct file *file,
return -EIO;
}
- lock_kernel();
-
+ switch (cmd) {
+ case MGSL_IOCWAITEVENT:
+ return wait_mgsl_event(info, argp);
+ case TIOCMIWAIT:
+ return modem_input_wait(info,(int)arg);
+ case TIOCGICOUNT:
+ spin_lock_irqsave(&info->lock,flags);
+ cnow = info->icount;
+ spin_unlock_irqrestore(&info->lock,flags);
+ p_cuser = argp;
+ if (put_user(cnow.cts, &p_cuser->cts) ||
+ put_user(cnow.dsr, &p_cuser->dsr) ||
+ put_user(cnow.rng, &p_cuser->rng) ||
+ put_user(cnow.dcd, &p_cuser->dcd) ||
+ put_user(cnow.rx, &p_cuser->rx) ||
+ put_user(cnow.tx, &p_cuser->tx) ||
+ put_user(cnow.frame, &p_cuser->frame) ||
+ put_user(cnow.overrun, &p_cuser->overrun) ||
+ put_user(cnow.parity, &p_cuser->parity) ||
+ put_user(cnow.brk, &p_cuser->brk) ||
+ put_user(cnow.buf_overrun, &p_cuser->buf_overrun))
+ return -EFAULT;
+ return 0;
+ case MGSL_IOCSGPIO:
+ return set_gpio(info, argp);
+ case MGSL_IOCGGPIO:
+ return get_gpio(info, argp);
+ case MGSL_IOCWAITGPIO:
+ return wait_gpio(info, argp);
+ }
+ mutex_lock(&info->port.mutex);
switch (cmd) {
case MGSL_IOCGPARAMS:
ret = get_params(info, argp);
@@ -1068,50 +1092,16 @@ static int ioctl(struct tty_struct *tty, struct file *file,
case MGSL_IOCGSTATS:
ret = get_stats(info, argp);
break;
- case MGSL_IOCWAITEVENT:
- ret = wait_mgsl_event(info, argp);
- break;
- case TIOCMIWAIT:
- ret = modem_input_wait(info,(int)arg);
- break;
case MGSL_IOCGIF:
ret = get_interface(info, argp);
break;
case MGSL_IOCSIF:
ret = set_interface(info,(int)arg);
break;
- case MGSL_IOCSGPIO:
- ret = set_gpio(info, argp);
- break;
- case MGSL_IOCGGPIO:
- ret = get_gpio(info, argp);
- break;
- case MGSL_IOCWAITGPIO:
- ret = wait_gpio(info, argp);
- break;
- case TIOCGICOUNT:
- spin_lock_irqsave(&info->lock,flags);
- cnow = info->icount;
- spin_unlock_irqrestore(&info->lock,flags);
- p_cuser = argp;
- if (put_user(cnow.cts, &p_cuser->cts) ||
- put_user(cnow.dsr, &p_cuser->dsr) ||
- put_user(cnow.rng, &p_cuser->rng) ||
- put_user(cnow.dcd, &p_cuser->dcd) ||
- put_user(cnow.rx, &p_cuser->rx) ||
- put_user(cnow.tx, &p_cuser->tx) ||
- put_user(cnow.frame, &p_cuser->frame) ||
- put_user(cnow.overrun, &p_cuser->overrun) ||
- put_user(cnow.parity, &p_cuser->parity) ||
- put_user(cnow.brk, &p_cuser->brk) ||
- put_user(cnow.buf_overrun, &p_cuser->buf_overrun))
- ret = -EFAULT;
- ret = 0;
- break;
default:
ret = -ENOIOCTLCMD;
}
- unlock_kernel();
+ mutex_unlock(&info->port.mutex);
return ret;
}
diff --git a/drivers/char/synclinkmp.c b/drivers/char/synclinkmp.c
index 2b18adc..8da976b 100644
--- a/drivers/char/synclinkmp.c
+++ b/drivers/char/synclinkmp.c
@@ -52,7 +52,6 @@
#include <linux/mm.h>
#include <linux/seq_file.h>
#include <linux/slab.h>
-#include <linux/smp_lock.h>
#include <linux/netdevice.h>
#include <linux/vmalloc.h>
#include <linux/init.h>
@@ -1062,9 +1061,7 @@ static void wait_until_sent(struct tty_struct *tty, int timeout)
if (sanity_check(info, tty->name, "wait_until_sent"))
return;
- lock_kernel();
-
- if (!(info->port.flags & ASYNC_INITIALIZED))
+ if (!test_bit(ASYNCB_INITIALIZED, &info->port.flags))
goto exit;
orig_jiffies = jiffies;
@@ -1094,8 +1091,10 @@ static void wait_until_sent(struct tty_struct *tty, int timeout)
break;
}
} else {
- //TODO: determine if there is something similar to USC16C32
- // TXSTATUS_ALL_SENT status
+ /*
+ * TODO: determine if there is something similar to USC16C32
+ * TXSTATUS_ALL_SENT status
+ */
while ( info->tx_active && info->tx_enabled) {
msleep_interruptible(jiffies_to_msecs(char_time));
if (signal_pending(current))
@@ -1106,7 +1105,6 @@ static void wait_until_sent(struct tty_struct *tty, int timeout)
}
exit:
- unlock_kernel();
if (debug_level >= DEBUG_LEVEL_INFO)
printk("%s(%d):%s wait_until_sent() exit\n",
__FILE__,__LINE__, info->device_name );
@@ -1122,7 +1120,6 @@ static int write_room(struct tty_struct *tty)
if (sanity_check(info, tty->name, "write_room"))
return 0;
- lock_kernel();
if (info->params.mode == MGSL_MODE_HDLC) {
ret = (info->tx_active) ? 0 : HDLC_MAX_FRAME_SIZE;
} else {
@@ -1130,7 +1127,6 @@ static int write_room(struct tty_struct *tty)
if (ret < 0)
ret = 0;
}
- unlock_kernel();
if (debug_level >= DEBUG_LEVEL_INFO)
printk("%s(%d):%s write_room()=%d\n",
@@ -1251,7 +1247,7 @@ static void tx_release(struct tty_struct *tty)
*
* Return Value: 0 if success, otherwise error code
*/
-static int do_ioctl(struct tty_struct *tty, struct file *file,
+static int ioctl(struct tty_struct *tty, struct file *file,
unsigned int cmd, unsigned long arg)
{
SLMP_INFO *info = tty->driver_data;
@@ -1341,16 +1337,6 @@ static int do_ioctl(struct tty_struct *tty, struct file *file,
return 0;
}
-static int ioctl(struct tty_struct *tty, struct file *file,
- unsigned int cmd, unsigned long arg)
-{
- int ret;
- lock_kernel();
- ret = do_ioctl(tty, file, cmd, arg);
- unlock_kernel();
- return ret;
-}
-
/*
* /proc fs routines....
*/
@@ -2883,7 +2869,9 @@ static int get_stats(SLMP_INFO * info, struct mgsl_icount __user *user_icount)
if (!user_icount) {
memset(&info->icount, 0, sizeof(info->icount));
} else {
+ mutex_lock(&info->port.mutex);
COPY_TO_USER(err, user_icount, &info->icount, sizeof(struct mgsl_icount));
+ mutex_unlock(&info->port.mutex);
if (err)
return -EFAULT;
}
@@ -2898,7 +2886,9 @@ static int get_params(SLMP_INFO * info, MGSL_PARAMS __user *user_params)
printk("%s(%d):%s get_params()\n",
__FILE__,__LINE__, info->device_name);
+ mutex_lock(&info->port.mutex);
COPY_TO_USER(err,user_params, &info->params, sizeof(MGSL_PARAMS));
+ mutex_unlock(&info->port.mutex);
if (err) {
if ( debug_level >= DEBUG_LEVEL_INFO )
printk( "%s(%d):%s get_params() user buffer copy failed\n",
@@ -2926,11 +2916,13 @@ static int set_params(SLMP_INFO * info, MGSL_PARAMS __user *new_params)
return -EFAULT;
}
+ mutex_lock(&info->port.mutex);
spin_lock_irqsave(&info->lock,flags);
memcpy(&info->params,&tmp_params,sizeof(MGSL_PARAMS));
spin_unlock_irqrestore(&info->lock,flags);
change_params(info);
+ mutex_unlock(&info->port.mutex);
return 0;
}
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 11/14] cyclades: Kill off BKL usage
2010-04-01 16:04 [RFC PATCH 00/14] Some BKL killing in the serial drivers Alan Cox
` (9 preceding siblings ...)
2010-04-01 16:06 ` [PATCH 10/14] synclink: kill the big kernel lock Alan Cox
@ 2010-04-01 16:06 ` Alan Cox
2010-04-01 16:06 ` [PATCH 12/14] epca: Kill the big kernel lock Alan Cox
` (2 subsequent siblings)
13 siblings, 0 replies; 18+ messages in thread
From: Alan Cox @ 2010-04-01 16:06 UTC (permalink / raw)
To: linux-kernel
Use the port mutext for config setting, the rest is locked sufficiently
anyway that the BKL makes no odds.
Signed-off-by: Alan Cox <alan@linux.intel.com>
---
drivers/char/cyclades.c | 20 +++++++++-----------
1 files changed, 9 insertions(+), 11 deletions(-)
diff --git a/drivers/char/cyclades.c b/drivers/char/cyclades.c
index 9824b41..51acfe3 100644
--- a/drivers/char/cyclades.c
+++ b/drivers/char/cyclades.c
@@ -65,7 +65,6 @@
#include <linux/tty.h>
#include <linux/tty_flip.h>
#include <linux/serial.h>
-#include <linux/smp_lock.h>
#include <linux/major.h>
#include <linux/string.h>
#include <linux/fcntl.h>
@@ -1655,7 +1654,6 @@ static void cy_wait_until_sent(struct tty_struct *tty, int timeout)
return; /* Just in case.... */
orig_jiffies = jiffies;
- lock_kernel();
/*
* Set the check interval to be 1/5 of the estimated time to
* send a single character, and make it at least 1. The check
@@ -1702,7 +1700,6 @@ static void cy_wait_until_sent(struct tty_struct *tty, int timeout)
}
/* Run one more char cycle */
msleep_interruptible(jiffies_to_msecs(char_time * 5));
- unlock_kernel();
#ifdef CY_DEBUG_WAIT_UNTIL_SENT
printk(KERN_DEBUG "Clean (jiff=%lu)...done\n", jiffies);
#endif
@@ -1959,7 +1956,6 @@ static int cy_chars_in_buffer(struct tty_struct *tty)
int char_count;
__u32 tx_put, tx_get, tx_bufsize;
- lock_kernel();
tx_get = readl(&buf_ctrl->tx_get);
tx_put = readl(&buf_ctrl->tx_put);
tx_bufsize = readl(&buf_ctrl->tx_bufsize);
@@ -1971,7 +1967,6 @@ static int cy_chars_in_buffer(struct tty_struct *tty)
printk(KERN_DEBUG "cyc:cy_chars_in_buffer ttyC%d %d\n",
info->line, info->xmit_cnt + char_count);
#endif
- unlock_kernel();
return info->xmit_cnt + char_count;
}
#endif /* Z_EXT_CHARS_IN_BUFFER */
@@ -2359,17 +2354,22 @@ cy_set_serial_info(struct cyclades_port *info, struct tty_struct *tty,
struct serial_struct __user *new_info)
{
struct serial_struct new_serial;
+ int ret;
if (copy_from_user(&new_serial, new_info, sizeof(new_serial)))
return -EFAULT;
+ mutex_lock(&info->port.mutex);
if (!capable(CAP_SYS_ADMIN)) {
if (new_serial.close_delay != info->port.close_delay ||
new_serial.baud_base != info->baud ||
(new_serial.flags & ASYNC_FLAGS &
~ASYNC_USR_MASK) !=
(info->port.flags & ASYNC_FLAGS & ~ASYNC_USR_MASK))
+ {
+ mutex_unlock(&info->port.mutex);
return -EPERM;
+ }
info->port.flags = (info->port.flags & ~ASYNC_USR_MASK) |
(new_serial.flags & ASYNC_USR_MASK);
info->baud = new_serial.baud_base;
@@ -2392,10 +2392,12 @@ cy_set_serial_info(struct cyclades_port *info, struct tty_struct *tty,
check_and_exit:
if (info->port.flags & ASYNC_INITIALIZED) {
cy_set_line_char(info, tty);
- return 0;
+ ret = 0;
} else {
- return cy_startup(info, tty);
+ ret = cy_startup(info, tty);
}
+ mutex_unlock(&info->port.mutex);
+ return ret;
} /* set_serial_info */
/*
@@ -2438,7 +2440,6 @@ static int cy_tiocmget(struct tty_struct *tty, struct file *file)
card = info->card;
- lock_kernel();
if (!cy_is_Z(card)) {
unsigned long flags;
int channel = info->line - card->first_line;
@@ -2478,7 +2479,6 @@ static int cy_tiocmget(struct tty_struct *tty, struct file *file)
((lstatus & C_RS_CTS) ? TIOCM_CTS : 0);
}
end:
- unlock_kernel();
return result;
} /* cy_tiomget */
@@ -2696,7 +2696,6 @@ cy_ioctl(struct tty_struct *tty, struct file *file,
printk(KERN_DEBUG "cyc:cy_ioctl ttyC%d, cmd = %x arg = %lx\n",
info->line, cmd, arg);
#endif
- lock_kernel();
switch (cmd) {
case CYGETMON:
@@ -2817,7 +2816,6 @@ cy_ioctl(struct tty_struct *tty, struct file *file,
default:
ret_val = -ENOIOCTLCMD;
}
- unlock_kernel();
#ifdef CY_DEBUG_OTHER
printk(KERN_DEBUG "cyc:cy_ioctl done\n");
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 12/14] epca: Kill the big kernel lock
2010-04-01 16:04 [RFC PATCH 00/14] Some BKL killing in the serial drivers Alan Cox
` (10 preceding siblings ...)
2010-04-01 16:06 ` [PATCH 11/14] cyclades: Kill off BKL usage Alan Cox
@ 2010-04-01 16:06 ` Alan Cox
2010-04-01 16:07 ` [PATCH 13/14] specialix; Kill the BKL Alan Cox
2010-04-01 16:07 ` [PATCH 14/14] synclink: reworking locking a bit Alan Cox
13 siblings, 0 replies; 18+ messages in thread
From: Alan Cox @ 2010-04-01 16:06 UTC (permalink / raw)
To: linux-kernel
The lock is no longer needed for wait until sent paths so this can go
Signed-off-by: Alan Cox <alan@linux.intel.com>
---
drivers/char/epca.c | 4 +---
1 files changed, 1 insertions(+), 3 deletions(-)
diff --git a/drivers/char/epca.c b/drivers/char/epca.c
index 6f5ffe1..d9df46a 100644
--- a/drivers/char/epca.c
+++ b/drivers/char/epca.c
@@ -36,7 +36,7 @@
#include <linux/ctype.h>
#include <linux/tty.h>
#include <linux/tty_flip.h>
-#include <linux/smp_lock.h>
+#include <linux/slab.h>
#include <linux/ioport.h>
#include <linux/interrupt.h>
#include <linux/uaccess.h>
@@ -2105,7 +2105,6 @@ static int pc_ioctl(struct tty_struct *tty, struct file *file,
break;
case DIGI_SETAW:
case DIGI_SETAF:
- lock_kernel();
if (cmd == DIGI_SETAW) {
/* Setup an event to indicate when the transmit
buffer empties */
@@ -2118,7 +2117,6 @@ static int pc_ioctl(struct tty_struct *tty, struct file *file,
if (tty->ldisc->ops->flush_buffer)
tty->ldisc->ops->flush_buffer(tty);
}
- unlock_kernel();
/* Fall Thru */
case DIGI_SETA:
if (copy_from_user(&ch->digiext, argp, sizeof(digi_t)))
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 13/14] specialix; Kill the BKL
2010-04-01 16:04 [RFC PATCH 00/14] Some BKL killing in the serial drivers Alan Cox
` (11 preceding siblings ...)
2010-04-01 16:06 ` [PATCH 12/14] epca: Kill the big kernel lock Alan Cox
@ 2010-04-01 16:07 ` Alan Cox
2010-04-01 16:07 ` [PATCH 14/14] synclink: reworking locking a bit Alan Cox
13 siblings, 0 replies; 18+ messages in thread
From: Alan Cox @ 2010-04-01 16:07 UTC (permalink / raw)
To: linux-kernel
Use the port mutex instead
Signed-off-by: Alan Cox <alan@linux.intel.com>
---
drivers/char/specialix.c | 11 +++++------
1 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/char/specialix.c b/drivers/char/specialix.c
index 2c24fcd..7be456f 100644
--- a/drivers/char/specialix.c
+++ b/drivers/char/specialix.c
@@ -1863,8 +1863,7 @@ static int sx_set_serial_info(struct specialix_port *port,
return -EFAULT;
}
- lock_kernel();
-
+ mutex_lock(&port->port.mutex);
change_speed = ((port->port.flags & ASYNC_SPD_MASK) !=
(tmp.flags & ASYNC_SPD_MASK));
change_speed |= (tmp.custom_divisor != port->custom_divisor);
@@ -1875,7 +1874,7 @@ static int sx_set_serial_info(struct specialix_port *port,
((tmp.flags & ~ASYNC_USR_MASK) !=
(port->port.flags & ~ASYNC_USR_MASK))) {
func_exit();
- unlock_kernel();
+ mutex_unlock(&port->port.mutex);
return -EPERM;
}
port->port.flags = ((port->port.flags & ~ASYNC_USR_MASK) |
@@ -1892,7 +1891,7 @@ static int sx_set_serial_info(struct specialix_port *port,
sx_change_speed(bp, port);
func_exit();
- unlock_kernel();
+ mutex_unlock(&port->port.mutex);
return 0;
}
@@ -1906,7 +1905,7 @@ static int sx_get_serial_info(struct specialix_port *port,
func_enter();
memset(&tmp, 0, sizeof(tmp));
- lock_kernel();
+ mutex_lock(&port->port.mutex);
tmp.type = PORT_CIRRUS;
tmp.line = port - sx_port;
tmp.port = bp->base;
@@ -1917,7 +1916,7 @@ static int sx_get_serial_info(struct specialix_port *port,
tmp.closing_wait = port->port.closing_wait * HZ/100;
tmp.custom_divisor = port->custom_divisor;
tmp.xmit_fifo_size = CD186x_NFIFO;
- unlock_kernel();
+ mutex_unlock(&port->port.mutex);
if (copy_to_user(retinfo, &tmp, sizeof(tmp))) {
func_exit();
return -EFAULT;
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 14/14] synclink: reworking locking a bit
2010-04-01 16:04 [RFC PATCH 00/14] Some BKL killing in the serial drivers Alan Cox
` (12 preceding siblings ...)
2010-04-01 16:07 ` [PATCH 13/14] specialix; Kill the BKL Alan Cox
@ 2010-04-01 16:07 ` Alan Cox
13 siblings, 0 replies; 18+ messages in thread
From: Alan Cox @ 2010-04-01 16:07 UTC (permalink / raw)
To: linux-kernel
Use the port mutex and port lock to fix the various races. The locking
still isn't totally consistent but its better than before. Wants switching
to the port helpers.
Signed-off-by: Alan Cox <alan@linux.intel.com>
---
drivers/char/synclink_gt.c | 12 +++++++++++-
drivers/char/synclinkmp.c | 9 ++++++++-
2 files changed, 19 insertions(+), 2 deletions(-)
diff --git a/drivers/char/synclink_gt.c b/drivers/char/synclink_gt.c
index d0f81f5..c56b70a 100644
--- a/drivers/char/synclink_gt.c
+++ b/drivers/char/synclink_gt.c
@@ -675,12 +675,14 @@ static int open(struct tty_struct *tty, struct file *filp)
goto cleanup;
}
+ mutex_lock(&info->port.mutex);
info->port.tty->low_latency = (info->port.flags & ASYNC_LOW_LATENCY) ? 1 : 0;
spin_lock_irqsave(&info->netlock, flags);
if (info->netcount) {
retval = -EBUSY;
spin_unlock_irqrestore(&info->netlock, flags);
+ mutex_unlock(&info->port.mutex);
goto cleanup;
}
info->port.count++;
@@ -692,7 +694,7 @@ static int open(struct tty_struct *tty, struct file *filp)
if (retval < 0)
goto cleanup;
}
-
+ mutex_unlock(&info->port.mutex);
retval = block_til_ready(tty, filp, info);
if (retval) {
DBGINFO(("%s block_til_ready rc=%d\n", info->device_name, retval));
@@ -724,12 +726,14 @@ static void close(struct tty_struct *tty, struct file *filp)
if (tty_port_close_start(&info->port, tty, filp) == 0)
goto cleanup;
+ mutex_lock(&info->port.mutex);
if (info->port.flags & ASYNC_INITIALIZED)
wait_until_sent(tty, info->timeout);
flush_buffer(tty);
tty_ldisc_flush(tty);
shutdown(info);
+ mutex_unlock(&info->port.mutex);
tty_port_close_end(&info->port, tty);
info->port.tty = NULL;
@@ -740,17 +744,23 @@ cleanup:
static void hangup(struct tty_struct *tty)
{
struct slgt_info *info = tty->driver_data;
+ unsigned long flags;
if (sanity_check(info, tty->name, "hangup"))
return;
DBGINFO(("%s hangup\n", info->device_name));
flush_buffer(tty);
+
+ mutex_lock(&info->port.mutex);
shutdown(info);
+ spin_lock_irqsave(&info->port.lock, flags);
info->port.count = 0;
info->port.flags &= ~ASYNC_NORMAL_ACTIVE;
info->port.tty = NULL;
+ spin_unlock_irqrestore(&info->port.lock, flags);
+ mutex_unlock(&info->port.mutex);
wake_up_interruptible(&info->port.open_wait);
}
diff --git a/drivers/char/synclinkmp.c b/drivers/char/synclinkmp.c
index 8da976b..cfa581e 100644
--- a/drivers/char/synclinkmp.c
+++ b/drivers/char/synclinkmp.c
@@ -812,13 +812,15 @@ static void close(struct tty_struct *tty, struct file *filp)
if (tty_port_close_start(&info->port, tty, filp) == 0)
goto cleanup;
-
+
+ mutex_lock(&info->port.mutex);
if (info->port.flags & ASYNC_INITIALIZED)
wait_until_sent(tty, info->timeout);
flush_buffer(tty);
tty_ldisc_flush(tty);
shutdown(info);
+ mutex_unlock(&info->port.mutex);
tty_port_close_end(&info->port, tty);
info->port.tty = NULL;
@@ -834,6 +836,7 @@ cleanup:
static void hangup(struct tty_struct *tty)
{
SLMP_INFO *info = tty->driver_data;
+ unsigned long flags;
if (debug_level >= DEBUG_LEVEL_INFO)
printk("%s(%d):%s hangup()\n",
@@ -842,12 +845,16 @@ static void hangup(struct tty_struct *tty)
if (sanity_check(info, tty->name, "hangup"))
return;
+ mutex_lock(&info->port.mutex);
flush_buffer(tty);
shutdown(info);
+ spin_lock_irqsave(&info->port.lock, flags);
info->port.count = 0;
info->port.flags &= ~ASYNC_NORMAL_ACTIVE;
info->port.tty = NULL;
+ spin_unlock_irqrestore(&info->port.lock, flags);
+ mutex_unlock(&info->port.mutex);
wake_up_interruptible(&info->port.open_wait);
}
^ permalink raw reply related [flat|nested] 18+ messages in thread