* [PATCH 01/13] tty: replace BKL with a new tty_lock
2010-05-04 22:33 [PATCH v2 00/13] BKL conversion in tty layer Arnd Bergmann
@ 2010-05-04 22:33 ` Arnd Bergmann
2010-05-04 22:33 ` [PATCH 02/13] tty: make atomic_write_lock release tty_lock Arnd Bergmann
` (12 subsequent siblings)
13 siblings, 0 replies; 25+ messages in thread
From: Arnd Bergmann @ 2010-05-04 22:33 UTC (permalink / raw)
To: linux-kernel
Cc: Arnd Bergmann, Alan Cox, Greg KH, Frederic Weisbecker,
Thomas Gleixner, Andrew Morton, John Kacur, Al Viro, Ingo Molnar
As a preparation for replacing the big kernel lock
in the TTY layer, wrap all the callers in new
macros tty_lock, tty_lock_nested and tty_unlock.
Every user of tty_lock_nested should come with
a comment explaining why we could get the lock
recursively. This will help turning the BTM
into a nonrecursive lock.
We also need to deal with lock order problems that
are currently solved by the BKL autorelease semantics.
For this, we get new macros that can be used to
replace sleeping calls like mutex_lock() or wait_event().
We start using these in the next patches.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/char/amiserial.c | 16 ++--
drivers/char/briq_panel.c | 6 +-
drivers/char/n_hdlc.c | 16 ++--
drivers/char/n_r3964.c | 8 +-
drivers/char/pty.c | 4 +-
drivers/char/selection.c | 4 +-
drivers/char/serial167.c | 4 +-
drivers/char/sx.c | 12 ++--
drivers/char/tty_io.c | 113 +++++++++++++++------------
drivers/char/tty_ldisc.c | 24 +++---
drivers/char/vc_screen.c | 4 +-
drivers/char/vt_ioctl.c | 10 +-
drivers/isdn/i4l/isdn_common.c | 20 +++---
drivers/isdn/i4l/isdn_tty.c | 8 +-
drivers/serial/68360serial.c | 4 +-
drivers/serial/crisv10.c | 4 +-
drivers/serial/serial_core.c | 10 +-
drivers/usb/serial/usb-serial.c | 18 ++--
drivers/video/console/vgacon.c | 4 +-
include/linux/tty.h | 169 +++++++++++++++++++++++++++++++++++++++
20 files changed, 321 insertions(+), 137 deletions(-)
diff --git a/drivers/char/amiserial.c b/drivers/char/amiserial.c
index 56b2767..5bd382e 100644
--- a/drivers/char/amiserial.c
+++ b/drivers/char/amiserial.c
@@ -1071,7 +1071,7 @@ static int get_serial_info(struct async_struct * info,
if (!retinfo)
return -EFAULT;
memset(&tmp, 0, sizeof(tmp));
- lock_kernel();
+ tty_lock();
tmp.type = state->type;
tmp.line = state->line;
tmp.port = state->port;
@@ -1082,7 +1082,7 @@ static int get_serial_info(struct async_struct * info,
tmp.close_delay = state->close_delay;
tmp.closing_wait = state->closing_wait;
tmp.custom_divisor = state->custom_divisor;
- unlock_kernel();
+ tty_unlock();
if (copy_to_user(retinfo,&tmp,sizeof(*retinfo)))
return -EFAULT;
return 0;
@@ -1099,14 +1099,14 @@ static int set_serial_info(struct async_struct * info,
if (copy_from_user(&new_serial,new_info,sizeof(new_serial)))
return -EFAULT;
- lock_kernel();
+ tty_lock();
state = info->state;
old_state = *state;
change_irq = new_serial.irq != state->irq;
change_port = (new_serial.port != state->port);
if(change_irq || change_port || (new_serial.xmit_fifo_size != state->xmit_fifo_size)) {
- unlock_kernel();
+ tty_unlock();
return -EINVAL;
}
@@ -1126,7 +1126,7 @@ static int set_serial_info(struct async_struct * info,
}
if (new_serial.baud_base < 9600) {
- unlock_kernel();
+ tty_unlock();
return -EINVAL;
}
@@ -1162,7 +1162,7 @@ check_and_exit:
}
} else
retval = startup(info);
- unlock_kernel();
+ tty_unlock();
return retval;
}
@@ -1537,7 +1537,7 @@ static void rs_wait_until_sent(struct tty_struct *tty, int timeout)
orig_jiffies = jiffies;
- lock_kernel();
+ tty_lock_nested(); /* tty_wait_until_sent is called from lots of places */
/*
* 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
@@ -1578,7 +1578,7 @@ static void rs_wait_until_sent(struct tty_struct *tty, int timeout)
break;
}
__set_current_state(TASK_RUNNING);
- unlock_kernel();
+ tty_unlock();
#ifdef SERIAL_DEBUG_RS_WAIT_UNTIL_SENT
printk("lsr = %d (jiff=%lu)...done\n", lsr, jiffies);
#endif
diff --git a/drivers/char/briq_panel.c b/drivers/char/briq_panel.c
index 555cd93..d5fa113 100644
--- a/drivers/char/briq_panel.c
+++ b/drivers/char/briq_panel.c
@@ -67,15 +67,15 @@ static void set_led(char state)
static int briq_panel_open(struct inode *ino, struct file *filep)
{
- lock_kernel();
+ tty_lock();
/* enforce single access, vfd_is_open is protected by BKL */
if (vfd_is_open) {
- unlock_kernel();
+ tty_unlock();
return -EBUSY;
}
vfd_is_open = 1;
- unlock_kernel();
+ tty_unlock();
return 0;
}
diff --git a/drivers/char/n_hdlc.c b/drivers/char/n_hdlc.c
index c68118e..47d3228 100644
--- a/drivers/char/n_hdlc.c
+++ b/drivers/char/n_hdlc.c
@@ -598,18 +598,18 @@ static ssize_t n_hdlc_tty_read(struct tty_struct *tty, struct file *file,
return -EFAULT;
}
- lock_kernel();
+ tty_lock();
for (;;) {
if (test_bit(TTY_OTHER_CLOSED, &tty->flags)) {
- unlock_kernel();
+ tty_unlock();
return -EIO;
}
n_hdlc = tty2n_hdlc (tty);
if (!n_hdlc || n_hdlc->magic != HDLC_MAGIC ||
tty != n_hdlc->tty) {
- unlock_kernel();
+ tty_unlock();
return 0;
}
@@ -619,13 +619,13 @@ static ssize_t n_hdlc_tty_read(struct tty_struct *tty, struct file *file,
/* no data */
if (file->f_flags & O_NONBLOCK) {
- unlock_kernel();
+ tty_unlock();
return -EAGAIN;
}
interruptible_sleep_on (&tty->read_wait);
if (signal_pending(current)) {
- unlock_kernel();
+ tty_unlock();
return -EINTR;
}
}
@@ -648,7 +648,7 @@ static ssize_t n_hdlc_tty_read(struct tty_struct *tty, struct file *file,
kfree(rbuf);
else
n_hdlc_buf_put(&n_hdlc->rx_free_buf_list,rbuf);
- unlock_kernel();
+ tty_unlock();
return ret;
} /* end of n_hdlc_tty_read() */
@@ -691,7 +691,7 @@ static ssize_t n_hdlc_tty_write(struct tty_struct *tty, struct file *file,
count = maxframe;
}
- lock_kernel();
+ tty_lock();
add_wait_queue(&tty->write_wait, &wait);
set_current_state(TASK_INTERRUPTIBLE);
@@ -731,7 +731,7 @@ static ssize_t n_hdlc_tty_write(struct tty_struct *tty, struct file *file,
n_hdlc_buf_put(&n_hdlc->tx_buf_list,tbuf);
n_hdlc_send_frames(n_hdlc,tty);
}
- unlock_kernel();
+ tty_unlock();
return error;
} /* end of n_hdlc_tty_write() */
diff --git a/drivers/char/n_r3964.c b/drivers/char/n_r3964.c
index c1d8b54..f4bd259 100644
--- a/drivers/char/n_r3964.c
+++ b/drivers/char/n_r3964.c
@@ -1067,7 +1067,7 @@ static ssize_t r3964_read(struct tty_struct *tty, struct file *file,
TRACE_L("read()");
- lock_kernel();
+ tty_lock();
pClient = findClient(pInfo, task_pid(current));
if (pClient) {
@@ -1109,7 +1109,7 @@ static ssize_t r3964_read(struct tty_struct *tty, struct file *file,
}
ret = -EPERM;
unlock:
- unlock_kernel();
+ tty_unlock();
return ret;
}
@@ -1158,7 +1158,7 @@ static ssize_t r3964_write(struct tty_struct *tty, struct file *file,
pHeader->locks = 0;
pHeader->owner = NULL;
- lock_kernel();
+ tty_lock();
pClient = findClient(pInfo, task_pid(current));
if (pClient) {
@@ -1177,7 +1177,7 @@ static ssize_t r3964_write(struct tty_struct *tty, struct file *file,
add_tx_queue(pInfo, pHeader);
trigger_transmit(pInfo);
- unlock_kernel();
+ tty_unlock();
return 0;
}
diff --git a/drivers/char/pty.c b/drivers/char/pty.c
index d83a431..384e79f 100644
--- a/drivers/char/pty.c
+++ b/drivers/char/pty.c
@@ -671,9 +671,9 @@ static int ptmx_open(struct inode *inode, struct file *filp)
{
int ret;
- lock_kernel();
+ tty_lock();
ret = __ptmx_open(inode, filp);
- unlock_kernel();
+ tty_unlock();
return ret;
}
diff --git a/drivers/char/selection.c b/drivers/char/selection.c
index 6e79340..85211a3 100644
--- a/drivers/char/selection.c
+++ b/drivers/char/selection.c
@@ -313,7 +313,7 @@ int paste_selection(struct tty_struct *tty)
struct tty_ldisc *ld;
DECLARE_WAITQUEUE(wait, current);
- lock_kernel();
+ tty_lock_nested(); /* always called with BTM from vt_ioctl */
acquire_console_sem();
poke_blanked_console();
@@ -338,6 +338,6 @@ int paste_selection(struct tty_struct *tty)
__set_current_state(TASK_RUNNING);
tty_ldisc_deref(ld);
- unlock_kernel();
+ tty_unlock();
return 0;
}
diff --git a/drivers/char/serial167.c b/drivers/char/serial167.c
index 8dfd247..26a7089 100644
--- a/drivers/char/serial167.c
+++ b/drivers/char/serial167.c
@@ -1537,7 +1537,7 @@ cy_ioctl(struct tty_struct *tty, struct file *file,
printk("cy_ioctl %s, cmd = %x arg = %lx\n", tty->name, cmd, arg); /* */
#endif
- lock_kernel();
+ tty_lock();
switch (cmd) {
case CYGETMON:
@@ -1593,7 +1593,7 @@ cy_ioctl(struct tty_struct *tty, struct file *file,
default:
ret_val = -ENOIOCTLCMD;
}
- unlock_kernel();
+ tty_unlock();
#ifdef SERIAL_DEBUG_OTHER
printk("cy_ioctl done\n");
diff --git a/drivers/char/sx.c b/drivers/char/sx.c
index a81ec4f..5b24db4 100644
--- a/drivers/char/sx.c
+++ b/drivers/char/sx.c
@@ -1699,7 +1699,7 @@ static long sx_fw_ioctl(struct file *filp, unsigned int cmd,
if (!capable(CAP_SYS_RAWIO))
return -EPERM;
- lock_kernel();
+ tty_lock();
sx_dprintk(SX_DEBUG_FIRMWARE, "IOCTL %x: %lx\n", cmd, arg);
@@ -1848,7 +1848,7 @@ static long sx_fw_ioctl(struct file *filp, unsigned int cmd,
break;
}
out:
- unlock_kernel();
+ tty_unlock();
func_exit();
return rc;
}
@@ -1859,7 +1859,7 @@ static int sx_break(struct tty_struct *tty, int flag)
int rv;
func_enter();
- lock_kernel();
+ tty_lock();
if (flag)
rv = sx_send_command(port, HS_START, -1, HS_IDLE_BREAK);
@@ -1868,7 +1868,7 @@ static int sx_break(struct tty_struct *tty, int flag)
if (rv != 1)
printk(KERN_ERR "sx: couldn't send break (%x).\n",
read_sx_byte(port->board, CHAN_OFFSET(port, hi_hstat)));
- unlock_kernel();
+ tty_unlock();
func_exit();
return 0;
}
@@ -1909,7 +1909,7 @@ static int sx_ioctl(struct tty_struct *tty, struct file *filp,
/* func_enter2(); */
rc = 0;
- lock_kernel();
+ tty_lock();
switch (cmd) {
case TIOCGSERIAL:
rc = gs_getserial(&port->gs, argp);
@@ -1921,7 +1921,7 @@ static int sx_ioctl(struct tty_struct *tty, struct file *filp,
rc = -ENOIOCTLCMD;
break;
}
- unlock_kernel();
+ tty_unlock();
/* func_exit(); */
return rc;
diff --git a/drivers/char/tty_io.c b/drivers/char/tty_io.c
index 6da962c..3bf2c75 100644
--- a/drivers/char/tty_io.c
+++ b/drivers/char/tty_io.c
@@ -149,6 +149,7 @@ static long tty_compat_ioctl(struct file *file, unsigned int cmd,
#else
#define tty_compat_ioctl NULL
#endif
+static int __tty_fasync(int fd, struct file *filp, int on);
static int tty_fasync(int fd, struct file *filp, int on);
static void release_tty(struct tty_struct *tty, int idx);
static void __proc_set_tty(struct task_struct *tsk, struct tty_struct *tty);
@@ -483,7 +484,7 @@ EXPORT_SYMBOL_GPL(tty_wakeup);
* remains intact.
*
* Locking:
- * BKL
+ * BTM
* redirect lock for undoing redirection
* file list lock for manipulating list of ttys
* tty_ldisc_lock from called functions
@@ -513,8 +514,11 @@ static void do_tty_hangup(struct work_struct *work)
}
spin_unlock(&redirect_lock);
- /* inuse_filps is protected by the single kernel lock */
- lock_kernel();
+ /* inuse_filps is protected by the single tty lock,
+ this really needs to change if we want to flush the
+ workqueue with the lock held */
+ tty_lock_nested(); /* called with BTM held from pty_close and
+ others */
check_tty_count(tty, "do_tty_hangup");
file_list_lock();
@@ -525,7 +529,7 @@ static void do_tty_hangup(struct work_struct *work)
if (filp->f_op->write != tty_write)
continue;
closecount++;
- tty_fasync(-1, filp, 0); /* can't block */
+ __tty_fasync(-1, filp, 0); /* can't block */
filp->f_op = &hung_up_tty_fops;
}
file_list_unlock();
@@ -594,7 +598,7 @@ static void do_tty_hangup(struct work_struct *work)
*/
set_bit(TTY_HUPPED, &tty->flags);
tty_ldisc_enable(tty);
- unlock_kernel();
+ tty_unlock();
if (f)
fput(f);
}
@@ -696,7 +700,8 @@ static void session_clear_tty(struct pid *session)
* exiting; it is 0 if called by the ioctl TIOCNOTTY.
*
* Locking:
- * BKL is taken for hysterical raisins
+ * BTM is taken for hysterical raisins, and held when
+ * called from no_tty().
* tty_mutex is taken to protect tty
* ->siglock is taken to protect ->signal/->sighand
* tasklist_lock is taken to walk process list for sessions
@@ -714,10 +719,10 @@ void disassociate_ctty(int on_exit)
tty = get_current_tty();
if (tty) {
tty_pgrp = get_pid(tty->pgrp);
- lock_kernel();
+ tty_lock_nested(); /* see above */
if (on_exit && tty->driver->type != TTY_DRIVER_TYPE_PTY)
tty_vhangup(tty);
- unlock_kernel();
+ tty_unlock();
tty_kref_put(tty);
} else if (on_exit) {
struct pid *old_pgrp;
@@ -774,9 +779,9 @@ void disassociate_ctty(int on_exit)
void no_tty(void)
{
struct task_struct *tsk = current;
- lock_kernel();
+ tty_lock();
disassociate_ctty(0);
- unlock_kernel();
+ tty_unlock();
proc_clear_tty(tsk);
}
@@ -1013,19 +1018,19 @@ out:
* We don't put it into the syslog queue right now maybe in the future if
* really needed.
*
- * We must still hold the BKL and test the CLOSING flag for the moment.
+ * We must still hold the BTM and test the CLOSING flag for the moment.
*/
void tty_write_message(struct tty_struct *tty, char *msg)
{
if (tty) {
mutex_lock(&tty->atomic_write_lock);
- lock_kernel();
+ tty_lock();
if (tty->ops->write && !test_bit(TTY_CLOSING, &tty->flags)) {
- unlock_kernel();
+ tty_unlock();
tty->ops->write(tty, msg, strlen(msg));
} else
- unlock_kernel();
+ tty_unlock();
tty_write_unlock(tty);
}
return;
@@ -1208,18 +1213,18 @@ static int tty_driver_install_tty(struct tty_driver *driver,
int ret;
if (driver->ops->install) {
- lock_kernel();
+ tty_lock_nested(); /* already called with BTM held */
ret = driver->ops->install(driver, tty);
- unlock_kernel();
+ tty_unlock();
return ret;
}
if (tty_init_termios(tty) == 0) {
- lock_kernel();
+ tty_lock_nested();
tty_driver_kref_get(driver);
tty->count++;
driver->ttys[idx] = tty;
- unlock_kernel();
+ tty_unlock();
return 0;
}
return -ENOMEM;
@@ -1312,14 +1317,15 @@ struct tty_struct *tty_init_dev(struct tty_driver *driver, int idx,
struct tty_struct *tty;
int retval;
- lock_kernel();
+ tty_lock_nested(); /* always called with tty lock held already */
+
/* Check if pty master is being opened multiple times */
if (driver->subtype == PTY_TYPE_MASTER &&
(driver->flags & TTY_DRIVER_DEVPTS_MEM) && !first_ok) {
- unlock_kernel();
+ tty_unlock();
return ERR_PTR(-EIO);
}
- unlock_kernel();
+ tty_unlock();
/*
* First time open is complex, especially for PTY devices.
@@ -1363,9 +1369,9 @@ release_mem_out:
if (printk_ratelimit())
printk(KERN_INFO "tty_init_dev: ldisc open failed, "
"clearing slot %d\n", idx);
- lock_kernel();
+ tty_lock_nested();
release_tty(tty, idx);
- unlock_kernel();
+ tty_unlock();
return ERR_PTR(retval);
}
@@ -1512,10 +1518,10 @@ int tty_release(struct inode *inode, struct file *filp)
if (tty_paranoia_check(tty, inode, "tty_release_dev"))
return 0;
- lock_kernel();
+ tty_lock();
check_tty_count(tty, "tty_release_dev");
- tty_fasync(-1, filp, 0);
+ __tty_fasync(-1, filp, 0);
idx = tty->index;
pty_master = (tty->driver->type == TTY_DRIVER_TYPE_PTY &&
@@ -1527,18 +1533,18 @@ int tty_release(struct inode *inode, struct file *filp)
if (idx < 0 || idx >= tty->driver->num) {
printk(KERN_DEBUG "tty_release_dev: bad idx when trying to "
"free (%s)\n", tty->name);
- unlock_kernel();
+ tty_unlock();
return 0;
}
if (!devpts) {
if (tty != tty->driver->ttys[idx]) {
- unlock_kernel();
+ tty_unlock();
printk(KERN_DEBUG "tty_release_dev: driver.table[%d] not tty "
"for (%s)\n", idx, tty->name);
return 0;
}
if (tty->termios != tty->driver->termios[idx]) {
- unlock_kernel();
+ tty_unlock();
printk(KERN_DEBUG "tty_release_dev: driver.termios[%d] not termios "
"for (%s)\n",
idx, tty->name);
@@ -1556,21 +1562,21 @@ int tty_release(struct inode *inode, struct file *filp)
if (tty->driver->other &&
!(tty->driver->flags & TTY_DRIVER_DEVPTS_MEM)) {
if (o_tty != tty->driver->other->ttys[idx]) {
- unlock_kernel();
+ tty_unlock();
printk(KERN_DEBUG "tty_release_dev: other->table[%d] "
"not o_tty for (%s)\n",
idx, tty->name);
return 0 ;
}
if (o_tty->termios != tty->driver->other->termios[idx]) {
- unlock_kernel();
+ tty_unlock();
printk(KERN_DEBUG "tty_release_dev: other->termios[%d] "
"not o_termios for (%s)\n",
idx, tty->name);
return 0;
}
if (o_tty->link != tty) {
- unlock_kernel();
+ tty_unlock();
printk(KERN_DEBUG "tty_release_dev: bad pty pointers\n");
return 0;
}
@@ -1579,7 +1585,7 @@ int tty_release(struct inode *inode, struct file *filp)
if (tty->ops->close)
tty->ops->close(tty, filp);
- unlock_kernel();
+ tty_unlock();
/*
* Sanity check: if tty->count is going to zero, there shouldn't be
* any waiters on tty->read_wait or tty->write_wait. We test the
@@ -1602,7 +1608,7 @@ int tty_release(struct inode *inode, struct file *filp)
opens on /dev/tty */
mutex_lock(&tty_mutex);
- lock_kernel();
+ tty_lock();
tty_closing = tty->count <= 1;
o_tty_closing = o_tty &&
(o_tty->count <= (pty_master ? 1 : 0));
@@ -1633,7 +1639,7 @@ int tty_release(struct inode *inode, struct file *filp)
printk(KERN_WARNING "tty_release_dev: %s: read/write wait queue "
"active!\n", tty_name(tty, buf));
- unlock_kernel();
+ tty_unlock();
mutex_unlock(&tty_mutex);
schedule();
}
@@ -1698,7 +1704,7 @@ int tty_release(struct inode *inode, struct file *filp)
/* check whether both sides are closing ... */
if (!tty_closing || (o_tty && !o_tty_closing)) {
- unlock_kernel();
+ tty_unlock();
return 0;
}
@@ -1718,7 +1724,7 @@ int tty_release(struct inode *inode, struct file *filp)
/* Make this pty number available for reallocation */
if (devpts)
devpts_kill_index(inode, idx);
- unlock_kernel();
+ tty_unlock();
return 0;
}
@@ -1760,12 +1766,12 @@ retry_open:
retval = 0;
mutex_lock(&tty_mutex);
- lock_kernel();
+ tty_lock();
if (device == MKDEV(TTYAUX_MAJOR, 0)) {
tty = get_current_tty();
if (!tty) {
- unlock_kernel();
+ tty_unlock();
mutex_unlock(&tty_mutex);
return -ENXIO;
}
@@ -1797,14 +1803,14 @@ retry_open:
goto got_driver;
}
}
- unlock_kernel();
+ tty_unlock();
mutex_unlock(&tty_mutex);
return -ENODEV;
}
driver = get_tty_driver(device, &index);
if (!driver) {
- unlock_kernel();
+ tty_unlock();
mutex_unlock(&tty_mutex);
return -ENODEV;
}
@@ -1814,7 +1820,7 @@ got_driver:
tty = tty_driver_lookup_tty(driver, inode, index);
if (IS_ERR(tty)) {
- unlock_kernel();
+ tty_unlock();
mutex_unlock(&tty_mutex);
return PTR_ERR(tty);
}
@@ -1830,7 +1836,7 @@ got_driver:
mutex_unlock(&tty_mutex);
tty_driver_kref_put(driver);
if (IS_ERR(tty)) {
- unlock_kernel();
+ tty_unlock();
return PTR_ERR(tty);
}
@@ -1862,11 +1868,11 @@ got_driver:
#endif
tty_release(inode, filp);
if (retval != -ERESTARTSYS) {
- unlock_kernel();
+ tty_unlock();
return retval;
}
if (signal_pending(current)) {
- unlock_kernel();
+ tty_unlock();
return retval;
}
schedule();
@@ -1877,11 +1883,11 @@ got_driver:
filp->f_op = &tty_fops;
goto retry_open;
}
- unlock_kernel();
+ tty_unlock();
mutex_lock(&tty_mutex);
- lock_kernel();
+ tty_lock();
spin_lock_irq(¤t->sighand->siglock);
if (!noctty &&
current->signal->leader &&
@@ -1889,7 +1895,7 @@ got_driver:
tty->session == NULL)
__proc_set_tty(current, tty);
spin_unlock_irq(¤t->sighand->siglock);
- unlock_kernel();
+ tty_unlock();
mutex_unlock(&tty_mutex);
return 0;
}
@@ -1925,13 +1931,12 @@ static unsigned int tty_poll(struct file *filp, poll_table *wait)
return ret;
}
-static int tty_fasync(int fd, struct file *filp, int on)
+static int __tty_fasync(int fd, struct file *filp, int on)
{
struct tty_struct *tty;
unsigned long flags;
int retval = 0;
- lock_kernel();
tty = (struct tty_struct *)filp->private_data;
if (tty_paranoia_check(tty, filp->f_path.dentry->d_inode, "tty_fasync"))
goto out;
@@ -1965,7 +1970,15 @@ static int tty_fasync(int fd, struct file *filp, int on)
}
retval = 0;
out:
- unlock_kernel();
+ return retval;
+}
+
+static int tty_fasync(int fd, struct file *filp, int on)
+{
+ int retval;
+ tty_lock();
+ retval = __tty_fasync(fd, filp, on);
+ tty_unlock();
return retval;
}
diff --git a/drivers/char/tty_ldisc.c b/drivers/char/tty_ldisc.c
index 500e740..97681ff 100644
--- a/drivers/char/tty_ldisc.c
+++ b/drivers/char/tty_ldisc.c
@@ -440,6 +440,8 @@ static void tty_set_termios_ldisc(struct tty_struct *tty, int num)
*
* A helper opening method. Also a convenient debugging and check
* point.
+ *
+ * Locking: always called with BTM already held.
*/
static int tty_ldisc_open(struct tty_struct *tty, struct tty_ldisc *ld)
@@ -447,10 +449,10 @@ static int tty_ldisc_open(struct tty_struct *tty, struct tty_ldisc *ld)
WARN_ON(test_and_set_bit(TTY_LDISC_OPEN, &tty->flags));
if (ld->ops->open) {
int ret;
- /* BKL here locks verus a hangup event */
- lock_kernel();
+ /* BTM here locks versus a hangup event */
+ tty_lock_nested(); /* always held here already */
ret = ld->ops->open(tty);
- unlock_kernel();
+ tty_unlock();
return ret;
}
return 0;
@@ -553,7 +555,7 @@ int tty_set_ldisc(struct tty_struct *tty, int ldisc)
if (IS_ERR(new_ldisc))
return PTR_ERR(new_ldisc);
- lock_kernel();
+ tty_lock();
/*
* We need to look at the tty locking here for pty/tty pairs
* when both sides try to change in parallel.
@@ -567,12 +569,12 @@ int tty_set_ldisc(struct tty_struct *tty, int ldisc)
*/
if (tty->ldisc->ops->num == ldisc) {
- unlock_kernel();
+ tty_unlock();
tty_ldisc_put(new_ldisc);
return 0;
}
- unlock_kernel();
+ tty_unlock();
/*
* Problem: What do we do if this blocks ?
* We could deadlock here
@@ -594,7 +596,7 @@ int tty_set_ldisc(struct tty_struct *tty, int ldisc)
mutex_lock(&tty->ldisc_mutex);
}
- lock_kernel();
+ tty_lock();
set_bit(TTY_LDISC_CHANGING, &tty->flags);
@@ -607,7 +609,7 @@ int tty_set_ldisc(struct tty_struct *tty, int ldisc)
o_ldisc = tty->ldisc;
- unlock_kernel();
+ tty_unlock();
/*
* Make sure we don't change while someone holds a
* reference to the line discipline. The TTY_LDISC bit
@@ -633,14 +635,14 @@ int tty_set_ldisc(struct tty_struct *tty, int ldisc)
flush_scheduled_work();
mutex_lock(&tty->ldisc_mutex);
- lock_kernel();
+ tty_lock();
if (test_bit(TTY_HUPPED, &tty->flags)) {
/* We were raced by the hangup method. It will have stomped
the ldisc data and closed the ldisc down */
clear_bit(TTY_LDISC_CHANGING, &tty->flags);
mutex_unlock(&tty->ldisc_mutex);
tty_ldisc_put(new_ldisc);
- unlock_kernel();
+ tty_unlock();
return -EIO;
}
@@ -682,7 +684,7 @@ int tty_set_ldisc(struct tty_struct *tty, int ldisc)
if (o_work)
schedule_delayed_work(&o_tty->buf.work, 1);
mutex_unlock(&tty->ldisc_mutex);
- unlock_kernel();
+ tty_unlock();
return retval;
}
diff --git a/drivers/char/vc_screen.c b/drivers/char/vc_screen.c
index c1791a6..bcce46c 100644
--- a/drivers/char/vc_screen.c
+++ b/drivers/char/vc_screen.c
@@ -463,10 +463,10 @@ vcs_open(struct inode *inode, struct file *filp)
unsigned int currcons = iminor(inode) & 127;
int ret = 0;
- lock_kernel();
+ tty_lock();
if(currcons && !vc_cons_allocated(currcons-1))
ret = -ENXIO;
- unlock_kernel();
+ tty_unlock();
return ret;
}
diff --git a/drivers/char/vt_ioctl.c b/drivers/char/vt_ioctl.c
index 6aa1028..0ebda0e 100644
--- a/drivers/char/vt_ioctl.c
+++ b/drivers/char/vt_ioctl.c
@@ -509,7 +509,7 @@ int vt_ioctl(struct tty_struct *tty, struct file * file,
console = vc->vc_num;
- lock_kernel();
+ tty_lock();
if (!vc_cons_allocated(console)) { /* impossible? */
ret = -ENOIOCTLCMD;
@@ -1334,7 +1334,7 @@ int vt_ioctl(struct tty_struct *tty, struct file * file,
ret = -ENOIOCTLCMD;
}
out:
- unlock_kernel();
+ tty_unlock();
return ret;
eperm:
ret = -EPERM;
@@ -1501,7 +1501,7 @@ long vt_compat_ioctl(struct tty_struct *tty, struct file * file,
console = vc->vc_num;
- lock_kernel();
+ tty_lock();
if (!vc_cons_allocated(console)) { /* impossible? */
ret = -ENOIOCTLCMD;
@@ -1569,11 +1569,11 @@ long vt_compat_ioctl(struct tty_struct *tty, struct file * file,
goto fallback;
}
out:
- unlock_kernel();
+ tty_unlock();
return ret;
fallback:
- unlock_kernel();
+ tty_unlock();
return vt_ioctl(tty, file, cmd, arg);
}
diff --git a/drivers/isdn/i4l/isdn_common.c b/drivers/isdn/i4l/isdn_common.c
index 70044ee..ff0c0cd 100644
--- a/drivers/isdn/i4l/isdn_common.c
+++ b/drivers/isdn/i4l/isdn_common.c
@@ -1070,7 +1070,7 @@ isdn_read(struct file *file, char __user *buf, size_t count, loff_t * off)
int retval;
char *p;
- lock_kernel();
+ tty_lock();
if (minor == ISDN_MINOR_STATUS) {
if (!file->private_data) {
if (file->f_flags & O_NONBLOCK) {
@@ -1163,7 +1163,7 @@ isdn_read(struct file *file, char __user *buf, size_t count, loff_t * off)
#endif
retval = -ENODEV;
out:
- unlock_kernel();
+ tty_unlock();
return retval;
}
@@ -1180,7 +1180,7 @@ isdn_write(struct file *file, const char __user *buf, size_t count, loff_t * off
if (!dev->drivers)
return -ENODEV;
- lock_kernel();
+ tty_lock();
if (minor <= ISDN_MINOR_BMAX) {
printk(KERN_WARNING "isdn_write minor %d obsolete!\n", minor);
drvidx = isdn_minor2drv(minor);
@@ -1225,7 +1225,7 @@ isdn_write(struct file *file, const char __user *buf, size_t count, loff_t * off
#endif
retval = -ENODEV;
out:
- unlock_kernel();
+ tty_unlock();
return retval;
}
@@ -1236,7 +1236,7 @@ isdn_poll(struct file *file, poll_table * wait)
unsigned int minor = iminor(file->f_path.dentry->d_inode);
int drvidx = isdn_minor2drv(minor - ISDN_MINOR_CTRL);
- lock_kernel();
+ tty_lock();
if (minor == ISDN_MINOR_STATUS) {
poll_wait(file, &(dev->info_waitq), wait);
/* mask = POLLOUT | POLLWRNORM; */
@@ -1266,7 +1266,7 @@ isdn_poll(struct file *file, poll_table * wait)
#endif
mask = POLLERR;
out:
- unlock_kernel();
+ tty_unlock();
return mask;
}
@@ -1733,7 +1733,7 @@ isdn_open(struct inode *ino, struct file *filep)
int chidx;
int retval = -ENODEV;
- lock_kernel();
+ tty_lock();
if (minor == ISDN_MINOR_STATUS) {
infostruct *p;
@@ -1784,7 +1784,7 @@ isdn_open(struct inode *ino, struct file *filep)
#endif
out:
nonseekable_open(ino, filep);
- unlock_kernel();
+ tty_unlock();
return retval;
}
@@ -1793,7 +1793,7 @@ isdn_close(struct inode *ino, struct file *filep)
{
uint minor = iminor(ino);
- lock_kernel();
+ tty_lock();
if (minor == ISDN_MINOR_STATUS) {
infostruct *p = dev->infochain;
infostruct *q = NULL;
@@ -1827,7 +1827,7 @@ isdn_close(struct inode *ino, struct file *filep)
#endif
out:
- unlock_kernel();
+ tty_unlock();
return 0;
}
diff --git a/drivers/isdn/i4l/isdn_tty.c b/drivers/isdn/i4l/isdn_tty.c
index fc8454d..fd91de8 100644
--- a/drivers/isdn/i4l/isdn_tty.c
+++ b/drivers/isdn/i4l/isdn_tty.c
@@ -1354,14 +1354,14 @@ isdn_tty_tiocmget(struct tty_struct *tty, struct file *file)
if (tty->flags & (1 << TTY_IO_ERROR))
return -EIO;
- lock_kernel();
+ tty_lock();
#ifdef ISDN_DEBUG_MODEM_IOCTL
printk(KERN_DEBUG "ttyI%d ioctl TIOCMGET\n", info->line);
#endif
control = info->mcr;
status = info->msr;
- unlock_kernel();
+ tty_unlock();
return ((control & UART_MCR_RTS) ? TIOCM_RTS : 0)
| ((control & UART_MCR_DTR) ? TIOCM_DTR : 0)
| ((status & UART_MSR_DCD) ? TIOCM_CAR : 0)
@@ -1385,7 +1385,7 @@ isdn_tty_tiocmset(struct tty_struct *tty, struct file *file,
printk(KERN_DEBUG "ttyI%d ioctl TIOCMxxx: %x %x\n", info->line, set, clear);
#endif
- lock_kernel();
+ tty_lock();
if (set & TIOCM_RTS)
info->mcr |= UART_MCR_RTS;
if (set & TIOCM_DTR) {
@@ -1407,7 +1407,7 @@ isdn_tty_tiocmset(struct tty_struct *tty, struct file *file,
isdn_tty_modem_hup(info, 1);
}
}
- unlock_kernel();
+ tty_unlock();
return 0;
}
diff --git a/drivers/serial/68360serial.c b/drivers/serial/68360serial.c
index 24661cd..c17a595 100644
--- a/drivers/serial/68360serial.c
+++ b/drivers/serial/68360serial.c
@@ -1705,7 +1705,7 @@ static void rs_360_wait_until_sent(struct tty_struct *tty, int timeout)
printk("jiff=%lu...", jiffies);
#endif
- lock_kernel();
+ tty_lock_nested(); /* always held already since we come from ->close */
/* We go through the loop at least once because we can't tell
* exactly when the last character exits the shifter. There can
* be at least two characters waiting to be sent after the buffers
@@ -1734,7 +1734,7 @@ static void rs_360_wait_until_sent(struct tty_struct *tty, int timeout)
bdp--;
} while (bdp->status & BD_SC_READY);
current->state = TASK_RUNNING;
- unlock_kernel();
+ tty_unlock();
#ifdef SERIAL_DEBUG_RS_WAIT_UNTIL_SENT
printk("lsr = %d (jiff=%lu)...done\n", lsr, jiffies);
#endif
diff --git a/drivers/serial/crisv10.c b/drivers/serial/crisv10.c
index 31f1723..d11d898 100644
--- a/drivers/serial/crisv10.c
+++ b/drivers/serial/crisv10.c
@@ -3924,7 +3924,7 @@ static void rs_wait_until_sent(struct tty_struct *tty, int timeout)
* Check R_DMA_CHx_STATUS bit 0-6=number of available bytes in FIFO
* R_DMA_CHx_HWSW bit 31-16=nbr of bytes left in DMA buffer (0=64k)
*/
- lock_kernel();
+ tty_lock_nested(); /* locked already when coming from close */
orig_jiffies = jiffies;
while (info->xmit.head != info->xmit.tail || /* More in send queue */
(*info->ostatusadr & 0x007f) || /* more in FIFO */
@@ -3941,7 +3941,7 @@ static void rs_wait_until_sent(struct tty_struct *tty, int timeout)
curr_time_usec - info->last_tx_active_usec;
}
set_current_state(TASK_RUNNING);
- unlock_kernel();
+ tty_unlock();
}
/*
diff --git a/drivers/serial/serial_core.c b/drivers/serial/serial_core.c
index 570dca2..803332b 100644
--- a/drivers/serial/serial_core.c
+++ b/drivers/serial/serial_core.c
@@ -1273,7 +1273,7 @@ static void uart_close(struct tty_struct *tty, struct file *filp)
struct tty_port *port;
struct uart_port *uport;
- BUG_ON(!kernel_locked());
+ BUG_ON(!tty_locked());
if (!state)
return;
@@ -1369,7 +1369,7 @@ static void uart_wait_until_sent(struct tty_struct *tty, int timeout)
if (port->type == PORT_UNKNOWN || port->fifosize == 0)
return;
- lock_kernel();
+ tty_lock_nested(); /* already locked when coming from close */
/*
* Set the check interval to be 1/5 of the estimated time to
@@ -1416,7 +1416,7 @@ static void uart_wait_until_sent(struct tty_struct *tty, int timeout)
break;
}
set_current_state(TASK_RUNNING); /* might not be needed */
- unlock_kernel();
+ tty_unlock();
}
/*
@@ -1430,7 +1430,7 @@ static void uart_hangup(struct tty_struct *tty)
struct uart_state *state = tty->driver_data;
struct tty_port *port = &state->port;
- BUG_ON(!kernel_locked());
+ BUG_ON(!tty_locked());
pr_debug("uart_hangup(%d)\n", state->uart_port->line);
mutex_lock(&port->mutex);
@@ -1615,7 +1615,7 @@ static int uart_open(struct tty_struct *tty, struct file *filp)
struct tty_port *port;
int retval, line = tty->index;
- BUG_ON(!kernel_locked());
+ BUG_ON(!tty_locked());
pr_debug("uart_open(%d) called\n", line);
/*
diff --git a/drivers/usb/serial/usb-serial.c b/drivers/usb/serial/usb-serial.c
index 3873660..3138a29 100644
--- a/drivers/usb/serial/usb-serial.c
+++ b/drivers/usb/serial/usb-serial.c
@@ -709,17 +709,17 @@ int usb_serial_probe(struct usb_interface *interface,
int num_ports = 0;
int max_endpoints;
- lock_kernel(); /* guard against unloading a serial driver module */
+ tty_lock(); /* guard against unloading a serial driver module */
type = search_serial_device(interface);
if (!type) {
- unlock_kernel();
+ tty_unlock();
dbg("none matched");
return -ENODEV;
}
serial = create_serial(dev, interface, type);
if (!serial) {
- unlock_kernel();
+ tty_unlock();
dev_err(&interface->dev, "%s - out of memory\n", __func__);
return -ENOMEM;
}
@@ -729,7 +729,7 @@ int usb_serial_probe(struct usb_interface *interface,
const struct usb_device_id *id;
if (!try_module_get(type->driver.owner)) {
- unlock_kernel();
+ tty_unlock();
dev_err(&interface->dev,
"module get failed, exiting\n");
kfree(serial);
@@ -741,7 +741,7 @@ int usb_serial_probe(struct usb_interface *interface,
module_put(type->driver.owner);
if (retval) {
- unlock_kernel();
+ tty_unlock();
dbg("sub driver rejected device");
kfree(serial);
return retval;
@@ -813,7 +813,7 @@ int usb_serial_probe(struct usb_interface *interface,
* properly during a later invocation of usb_serial_probe
*/
if (num_bulk_in == 0 || num_bulk_out == 0) {
- unlock_kernel();
+ tty_unlock();
dev_info(&interface->dev, "PL-2303 hack: descriptors matched but endpoints did not\n");
kfree(serial);
return -ENODEV;
@@ -826,7 +826,7 @@ int usb_serial_probe(struct usb_interface *interface,
if (type == &usb_serial_generic_device) {
num_ports = num_bulk_out;
if (num_ports == 0) {
- unlock_kernel();
+ tty_unlock();
dev_err(&interface->dev,
"Generic device with no bulk out, not allowed.\n");
kfree(serial);
@@ -838,7 +838,7 @@ int usb_serial_probe(struct usb_interface *interface,
/* if this device type has a calc_num_ports function, call it */
if (type->calc_num_ports) {
if (!try_module_get(type->driver.owner)) {
- unlock_kernel();
+ tty_unlock();
dev_err(&interface->dev,
"module get failed, exiting\n");
kfree(serial);
@@ -869,7 +869,7 @@ int usb_serial_probe(struct usb_interface *interface,
max_endpoints = max(max_endpoints, num_interrupt_out);
max_endpoints = max(max_endpoints, (int)serial->num_ports);
serial->num_port_pointers = max_endpoints;
- unlock_kernel();
+ tty_unlock();
dbg("%s - setting up %d port structures for this device",
__func__, max_endpoints);
diff --git a/drivers/video/console/vgacon.c b/drivers/video/console/vgacon.c
index 182dd6f..7197005 100644
--- a/drivers/video/console/vgacon.c
+++ b/drivers/video/console/vgacon.c
@@ -1108,7 +1108,7 @@ static int vgacon_do_font_op(struct vgastate *state,char *arg,int set,int ch512)
charmap += 4 * cmapsz;
#endif
- unlock_kernel();
+ tty_unlock();
spin_lock_irq(&vga_lock);
/* First, the Sequencer */
vga_wseq(state->vgabase, VGA_SEQ_RESET, 0x1);
@@ -1192,7 +1192,7 @@ static int vgacon_do_font_op(struct vgastate *state,char *arg,int set,int ch512)
vga_wattr(state->vgabase, VGA_AR_ENABLE_DISPLAY, 0);
}
spin_unlock_irq(&vga_lock);
- lock_kernel();
+ tty_lock();
return 0;
}
diff --git a/include/linux/tty.h b/include/linux/tty.h
index 4409967..60b3d69 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -13,6 +13,7 @@
#include <linux/tty_driver.h>
#include <linux/tty_ldisc.h>
#include <linux/mutex.h>
+#include <linux/smp_lock.h>
#include <asm/system.h>
@@ -571,5 +572,173 @@ extern int vt_ioctl(struct tty_struct *tty, struct file *file,
extern long vt_compat_ioctl(struct tty_struct *tty, struct file * file,
unsigned int cmd, unsigned long arg);
+/* functions for preparation of BKL removal */
+
+/*
+ * tty_lock_nested get the tty_lock while potentially holding it
+ *
+ * The Big TTY Mutex is a recursive lock, meaning you can take it
+ * from a thread that is already holding it.
+ * This is bad for a number of reasons, so tty_lock_nested should
+ * really be used as rarely as possible. If a code location can
+ * be shown to never get called with this held already, it should
+ * use tty_lock() instead.
+ */
+static inline void __lockfunc tty_lock_nested(void) __acquires(kernel_lock)
+{
+ lock_kernel();
+}
+static inline void tty_lock(void) __acquires(kernel_lock)
+{
+ WARN_ON(kernel_locked());
+ lock_kernel();
+}
+static inline void tty_unlock(void) __releases(kernel_lock)
+{
+ unlock_kernel();
+}
+#define tty_locked() (kernel_locked())
+static inline int __reacquire_tty_lock(void)
+{
+ return 0;
+}
+static inline void __release_tty_lock(void)
+{
+}
+
+#define release_tty_lock(tsk) do { } while (0)
+#define reacquire_tty_lock(tsk) do { } while (0)
+
+/*
+ * mutex_lock_tty - lock a mutex without holding the BTM
+ *
+ * These three functions replace calls to mutex_lock
+ * in the tty layer, when locking on of the following
+ * mutexes:
+ * tty->ldisc_mutex
+ * tty->termios_mutex
+ * tty->atomic_write_lock
+ * port->mutex
+ * pn->all_ppp_mutex
+ *
+ * The reason we need these is to avoid an AB-BA deadlock
+ * with tty_lock(). When it can be shown that one of the
+ * above mutexes is either never acquired with the tty_lock()
+ * held, or is never held when tty_lock() is acquired, that
+ * mutex should be converted back to the regular mutex_lock
+ * function.
+ *
+ * In order to annotate the lock order, the mutex_lock_tty_on
+ * and mutex_lock_tty_off versions should be used whereever
+ * possible, to show if the mutex is used with or without
+ * tty_lock already held.
+ */
+#define mutex_lock_tty(mutex) \
+({ \
+ if (!mutex_trylock(mutex)) { \
+ if (tty_locked()) { \
+ __release_tty_lock(); \
+ mutex_lock(mutex); \
+ __reacquire_tty_lock(); \
+ } else \
+ mutex_lock(mutex); \
+ } \
+})
+
+#define mutex_lock_tty_on(mutex) \
+({ \
+ if (!mutex_trylock(mutex)) { \
+ if (!WARN_ON(!tty_locked())) { \
+ __release_tty_lock(); \
+ mutex_lock(mutex); \
+ __reacquire_tty_lock(); \
+ } else \
+ mutex_lock(mutex); \
+ } \
+})
+
+#define mutex_lock_tty_off(mutex) \
+({ \
+ if (!mutex_trylock(mutex)) { \
+ if (WARN_ON(tty_locked())) { \
+ __release_tty_lock(); \
+ mutex_lock(mutex); \
+ __reacquire_tty_lock(); \
+ } else \
+ mutex_lock(mutex); \
+ } \
+})
+
+#define mutex_lock_interruptible_tty(mutex) \
+({ \
+ int __ret = 0; \
+ if (!mutex_trylock(mutex)) { \
+ if (tty_locked()) { \
+ __release_tty_lock(); \
+ __ret = mutex_lock_interruptible(mutex); \
+ __reacquire_tty_lock(); \
+ } else \
+ __ret = mutex_lock_interruptible(mutex); \
+ } \
+ __ret; \
+})
+
+/*
+ * wait_event*_tty -- wait for a condition with the tty lock held
+ *
+ * similar to the mutex_lock_tty family, these should be used when
+ * waiting for an event in a code location that may get called
+ * while tty_lock is held.
+ *
+ * The problem is that the condition we are waiting for might
+ * only become true if another thread runs that needs the tty_lock
+ * in order to get there.
+ *
+ * None of these should be needed and all callers should be removed
+ * when either the caller or the condition it waits for can be show
+ * to not rely on the tty_lock.
+ */
+#define wait_event_tty(wq, condition) \
+do { \
+ if (condition) \
+ break; \
+ release_tty_lock(current); \
+ __wait_event(wq, condition); \
+ reacquire_tty_lock(current); \
+} while (0)
+
+#define wait_event_timeout_tty(wq, condition, timeout) \
+({ \
+ long __ret = timeout; \
+ if (!(condition)) { \
+ release_tty_lock(current); \
+ __wait_event_timeout(wq, condition, __ret); \
+ reacquire_tty_lock(current); \
+ } \
+ __ret; \
+})
+
+#define wait_event_interruptible_tty(wq, condition) \
+({ \
+ int __ret = 0; \
+ if (!(condition)) { \
+ release_tty_lock(current); \
+ __wait_event_interruptible(wq, condition, __ret); \
+ reacquire_tty_lock(current); \
+ } \
+ __ret; \
+})
+
+#define wait_event_interruptible_timeout_tty(wq, condition, timeout) \
+({ \
+ long __ret = timeout; \
+ if (!(condition)) { \
+ release_tty_lock(current); \
+ __wait_event_interruptible_timeout(wq, condition, __ret); \
+ reacquire_tty_lock(current); \
+ } \
+ __ret; \
+})
+
#endif /* __KERNEL__ */
#endif
--
1.7.0.4
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH 02/13] tty: make atomic_write_lock release tty_lock
2010-05-04 22:33 [PATCH v2 00/13] BKL conversion in tty layer Arnd Bergmann
2010-05-04 22:33 ` [PATCH 01/13] tty: replace BKL with a new tty_lock Arnd Bergmann
@ 2010-05-04 22:33 ` Arnd Bergmann
2010-05-05 10:57 ` Alan Cox
2010-05-04 22:33 ` [PATCH 03/13] tty: make tty_port->mutex nest under tty_lock Arnd Bergmann
` (11 subsequent siblings)
13 siblings, 1 reply; 25+ messages in thread
From: Arnd Bergmann @ 2010-05-04 22:33 UTC (permalink / raw)
To: linux-kernel
Cc: Arnd Bergmann, Alan Cox, Greg KH, Frederic Weisbecker,
Thomas Gleixner, Andrew Morton, John Kacur, Al Viro, Ingo Molnar
atomic_write_lock never nests below BTM as far
as I can tell, so this can eventually get
reverted again unless it triggers bugs.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/char/tty_io.c | 9 +++++++--
1 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/char/tty_io.c b/drivers/char/tty_io.c
index 3bf2c75..8331dd3 100644
--- a/drivers/char/tty_io.c
+++ b/drivers/char/tty_io.c
@@ -912,10 +912,15 @@ void tty_write_unlock(struct tty_struct *tty)
int tty_write_lock(struct tty_struct *tty, int ndelay)
{
+ /*
+ * code inspection has shown that this is never called
+ * with the BTM held. Make sure this stays that way.
+ */
+ WARN_ON_ONCE(tty_locked());
if (!mutex_trylock(&tty->atomic_write_lock)) {
if (ndelay)
return -EAGAIN;
- if (mutex_lock_interruptible(&tty->atomic_write_lock))
+ if (mutex_lock_interruptible_tty(&tty->atomic_write_lock))
return -ERESTARTSYS;
}
return 0;
@@ -1024,7 +1029,7 @@ out:
void tty_write_message(struct tty_struct *tty, char *msg)
{
if (tty) {
- mutex_lock(&tty->atomic_write_lock);
+ mutex_lock_tty_off(&tty->atomic_write_lock);
tty_lock();
if (tty->ops->write && !test_bit(TTY_CLOSING, &tty->flags)) {
tty_unlock();
--
1.7.0.4
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH 02/13] tty: make atomic_write_lock release tty_lock
2010-05-04 22:33 ` [PATCH 02/13] tty: make atomic_write_lock release tty_lock Arnd Bergmann
@ 2010-05-05 10:57 ` Alan Cox
0 siblings, 0 replies; 25+ messages in thread
From: Alan Cox @ 2010-05-05 10:57 UTC (permalink / raw)
To: Arnd Bergmann
Cc: linux-kernel, Greg KH, Frederic Weisbecker, Thomas Gleixner,
Andrew Morton, John Kacur, Al Viro, Ingo Molnar
On Wed, 5 May 2010 00:33:41 +0200
Arnd Bergmann <arnd@arndb.de> wrote:
> atomic_write_lock never nests below BTM as far
> as I can tell, so this can eventually get
> reverted again unless it triggers bugs.
That appears correct to me as a working assumpion as well
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 03/13] tty: make tty_port->mutex nest under tty_lock
2010-05-04 22:33 [PATCH v2 00/13] BKL conversion in tty layer Arnd Bergmann
2010-05-04 22:33 ` [PATCH 01/13] tty: replace BKL with a new tty_lock Arnd Bergmann
2010-05-04 22:33 ` [PATCH 02/13] tty: make atomic_write_lock release tty_lock Arnd Bergmann
@ 2010-05-04 22:33 ` Arnd Bergmann
2010-05-04 22:33 ` [PATCH 04/13] tty: make termios mutex " Arnd Bergmann
` (10 subsequent siblings)
13 siblings, 0 replies; 25+ messages in thread
From: Arnd Bergmann @ 2010-05-04 22:33 UTC (permalink / raw)
To: linux-kernel
Cc: Arnd Bergmann, Alan Cox, Greg KH, Frederic Weisbecker,
Thomas Gleixner, Andrew Morton, John Kacur, Al Viro, Ingo Molnar
tty_port->mutex has a lock order problem with
tty_lock. By using mutex_lock_tty to drop tty_lock
while waiting for tty_port->mutex we can work
around this.
Make sure that we document for each mutex_lock(port->mutex)
whether we hold the BTM or not, in the hope that this
helps us eliminate the BTM eventually.
All users of tty_port_hangup call that with the BTM held,
except for stl_cleanup_panels. Take the BTM explicitly
there to make the locking more consistent.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/char/cyclades.c | 2 +-
drivers/char/isicom.c | 4 ++--
drivers/char/istallion.c | 4 ++--
drivers/char/moxa.c | 6 +++---
drivers/char/mxser.c | 10 +++++-----
drivers/char/riscom8.c | 4 ++--
drivers/char/rocket.c | 6 +++---
drivers/char/specialix.c | 4 ++--
drivers/char/stallion.c | 10 ++++++----
drivers/char/synclink.c | 8 ++++----
drivers/char/synclink_gt.c | 8 ++++----
drivers/char/synclinkmp.c | 10 +++++-----
drivers/char/tty_port.c | 4 ++--
drivers/mmc/card/sdio_uart.c | 2 +-
drivers/serial/pmac_zilog.c | 4 ++--
drivers/serial/serial_core.c | 32 ++++++++++++++++----------------
drivers/usb/serial/opticon.c | 2 +-
17 files changed, 61 insertions(+), 59 deletions(-)
diff --git a/drivers/char/cyclades.c b/drivers/char/cyclades.c
index 51acfe3..5b3b419 100644
--- a/drivers/char/cyclades.c
+++ b/drivers/char/cyclades.c
@@ -2359,7 +2359,7 @@ cy_set_serial_info(struct cyclades_port *info, struct tty_struct *tty,
if (copy_from_user(&new_serial, new_info, sizeof(new_serial)))
return -EFAULT;
- mutex_lock(&info->port.mutex);
+ mutex_lock_tty_off(&info->port.mutex);
if (!capable(CAP_SYS_ADMIN)) {
if (new_serial.close_delay != info->port.close_delay ||
new_serial.baud_base != info->baud ||
diff --git a/drivers/char/isicom.c b/drivers/char/isicom.c
index 078d69f..2c46fd6 100644
--- a/drivers/char/isicom.c
+++ b/drivers/char/isicom.c
@@ -1117,7 +1117,7 @@ static int isicom_set_serial_info(struct tty_struct *tty,
if (copy_from_user(&newinfo, info, sizeof(newinfo)))
return -EFAULT;
- mutex_lock(&port->port.mutex);
+ mutex_lock_tty_off(&port->port.mutex);
reconfig_port = ((port->port.flags & ASYNC_SPD_MASK) !=
(newinfo.flags & ASYNC_SPD_MASK));
@@ -1152,7 +1152,7 @@ static int isicom_get_serial_info(struct isi_port *port,
{
struct serial_struct out_info;
- mutex_lock(&port->port.mutex);
+ mutex_lock_tty_off(&port->port.mutex);
memset(&out_info, 0, sizeof(out_info));
/* out_info.type = ? */
out_info.line = port - isi_ports;
diff --git a/drivers/char/istallion.c b/drivers/char/istallion.c
index 5e6fe7e..0618797 100644
--- a/drivers/char/istallion.c
+++ b/drivers/char/istallion.c
@@ -4080,7 +4080,7 @@ static int stli_portcmdstats(struct tty_struct *tty, struct stliport *portp)
if (brdp == NULL)
return -ENODEV;
- mutex_lock(&portp->port.mutex);
+ mutex_lock_tty_off(&portp->port.mutex);
if (test_bit(BST_STARTED, &brdp->state)) {
if ((rc = stli_cmdwait(brdp, portp, A_GETSTATS,
&stli_cdkstats, sizeof(asystats_t), 1)) < 0) {
@@ -4194,7 +4194,7 @@ static int stli_clrportstats(struct stliport *portp, comstats_t __user *cp)
if (!brdp)
return -ENODEV;
- mutex_lock(&portp->port.mutex);
+ mutex_lock_tty_off(&portp->port.mutex);
if (test_bit(BST_STARTED, &brdp->state)) {
if ((rc = stli_cmdwait(brdp, portp, A_CLEARSTATS, NULL, 0, 0)) < 0) {
diff --git a/drivers/char/moxa.c b/drivers/char/moxa.c
index 107b0bd..e3f943d 100644
--- a/drivers/char/moxa.c
+++ b/drivers/char/moxa.c
@@ -380,12 +380,12 @@ copy:
break;
}
case TIOCGSERIAL:
- mutex_lock(&ch->port.mutex);
+ mutex_lock_tty_off(&ch->port.mutex);
ret = moxa_get_serial_info(ch, argp);
mutex_unlock(&ch->port.mutex);
break;
case TIOCSSERIAL:
- mutex_lock(&ch->port.mutex);
+ mutex_lock_tty_off(&ch->port.mutex);
ret = moxa_set_serial_info(ch, argp);
mutex_unlock(&ch->port.mutex);
break;
@@ -1178,7 +1178,7 @@ static int moxa_open(struct tty_struct *tty, struct file *filp)
ch->port.count++;
tty->driver_data = ch;
tty_port_tty_set(&ch->port, tty);
- mutex_lock(&ch->port.mutex);
+ mutex_lock_tty_on(&ch->port.mutex);
if (!(ch->port.flags & ASYNC_INITIALIZED)) {
ch->statusflags = 0;
moxa_set_tty_param(tty, tty->termios);
diff --git a/drivers/char/mxser.c b/drivers/char/mxser.c
index d2692d4..ff2db07 100644
--- a/drivers/char/mxser.c
+++ b/drivers/char/mxser.c
@@ -1079,7 +1079,7 @@ static void mxser_close(struct tty_struct *tty, struct file *filp)
return;
if (tty_port_close_start(port, tty, filp) == 0)
return;
- mutex_lock(&port->mutex);
+ mutex_lock_tty_on(&port->mutex);
mxser_close_port(port);
mxser_flush_buffer(tty);
mxser_shutdown_port(port);
@@ -1513,7 +1513,7 @@ static int mxser_ioctl_special(unsigned int cmd, void __user *argp)
port = &ip->port;
memset(&ms, 0, sizeof(ms));
- mutex_lock(&port->mutex);
+ mutex_lock_tty_off(&port->mutex);
if (!ip->ioaddr)
goto copy;
@@ -1559,7 +1559,7 @@ static int mxser_ioctl_special(unsigned int cmd, void __user *argp)
ip = &mxser_boards[i].ports[j];
port = &ip->port;
- mutex_lock(&port->mutex);
+ mutex_lock_tty_off(&port->mutex);
if (!ip->ioaddr) {
mutex_unlock(&port->mutex);
continue;
@@ -1706,12 +1706,12 @@ static int mxser_ioctl(struct tty_struct *tty, struct file *file,
switch (cmd) {
case TIOCGSERIAL:
- mutex_lock(&port->mutex);
+ mutex_lock_tty_off(&port->mutex);
retval = mxser_get_serial_info(tty, argp);
mutex_unlock(&port->mutex);
return retval;
case TIOCSSERIAL:
- mutex_lock(&port->mutex);
+ mutex_lock_tty_off(&port->mutex);
retval = mxser_set_serial_info(tty, argp);
mutex_unlock(&port->mutex);
return retval;
diff --git a/drivers/char/riscom8.c b/drivers/char/riscom8.c
index af4de1f..8fc0956 100644
--- a/drivers/char/riscom8.c
+++ b/drivers/char/riscom8.c
@@ -1183,7 +1183,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);
+ mutex_lock_tty_off(&port->port.mutex);
change_speed = ((port->port.flags & ASYNC_SPD_MASK) !=
(tmp.flags & ASYNC_SPD_MASK));
@@ -1224,7 +1224,7 @@ static int rc_get_serial_info(struct riscom_port *port,
tmp.type = PORT_CIRRUS;
tmp.line = port - rc_port;
- mutex_lock(&port->port.mutex);
+ mutex_lock_tty_off(&port->port.mutex);
tmp.port = bp->base;
tmp.irq = bp->irq;
tmp.flags = port->port.flags;
diff --git a/drivers/char/rocket.c b/drivers/char/rocket.c
index 79c3bc6..b32147b 100644
--- a/drivers/char/rocket.c
+++ b/drivers/char/rocket.c
@@ -1016,7 +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);
+ mutex_lock_tty_on(&port->mutex);
cp = &info->channel;
/*
* Before we drop DTR, make sure the UART transmitter
@@ -1214,7 +1214,7 @@ 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);
+ mutex_lock_tty_off(&info->port.mutex);
tmp.line = info->line;
tmp.flags = info->flags;
tmp.close_delay = info->port.close_delay;
@@ -1235,7 +1235,7 @@ 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);
+ mutex_lock_tty_off(&info->port.mutex);
if (!capable(CAP_SYS_ADMIN))
{
if ((new_serial.flags & ~ROCKET_USR_MASK) != (info->flags & ~ROCKET_USR_MASK)) {
diff --git a/drivers/char/specialix.c b/drivers/char/specialix.c
index 7be456f..234aefc 100644
--- a/drivers/char/specialix.c
+++ b/drivers/char/specialix.c
@@ -1863,7 +1863,7 @@ static int sx_set_serial_info(struct specialix_port *port,
return -EFAULT;
}
- mutex_lock(&port->port.mutex);
+ mutex_lock_tty_off(&port->port.mutex);
change_speed = ((port->port.flags & ASYNC_SPD_MASK) !=
(tmp.flags & ASYNC_SPD_MASK));
change_speed |= (tmp.custom_divisor != port->custom_divisor);
@@ -1905,7 +1905,7 @@ static int sx_get_serial_info(struct specialix_port *port,
func_enter();
memset(&tmp, 0, sizeof(tmp));
- mutex_lock(&port->port.mutex);
+ mutex_lock_tty_off(&port->port.mutex);
tmp.type = PORT_CIRRUS;
tmp.line = port - sx_port;
tmp.port = bp->base;
diff --git a/drivers/char/stallion.c b/drivers/char/stallion.c
index f2167f8..4053d19 100644
--- a/drivers/char/stallion.c
+++ b/drivers/char/stallion.c
@@ -1028,7 +1028,7 @@ static int stl_getserial(struct stlport *portp, struct serial_struct __user *sp)
memset(&sio, 0, sizeof(struct serial_struct));
- mutex_lock(&portp->port.mutex);
+ mutex_lock_tty_off(&portp->port.mutex);
sio.line = portp->portnr;
sio.port = portp->ioaddr;
sio.flags = portp->port.flags;
@@ -1070,7 +1070,7 @@ 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);
+ mutex_lock_tty_off(&portp->port.mutex);
if (!capable(CAP_SYS_ADMIN)) {
if ((sio.baud_base != portp->baud_base) ||
(sio.close_delay != portp->close_delay) ||
@@ -1656,7 +1656,9 @@ static void stl_cleanup_panels(struct stlbrd *brdp)
continue;
tty = tty_port_tty_get(&portp->port);
if (tty != NULL) {
+ tty_lock(); /* call tty_port_hangup with BTM */
stl_hangup(tty);
+ tty_unlock();
tty_kref_put(tty);
}
kfree(portp->tx.buf);
@@ -2329,7 +2331,7 @@ static int stl_getportstats(struct tty_struct *tty, struct stlport *portp, comst
return -ENODEV;
}
- mutex_lock(&portp->port.mutex);
+ mutex_lock_tty_off(&portp->port.mutex);
portp->stats.state = portp->istate;
portp->stats.flags = portp->port.flags;
portp->stats.hwid = portp->hwid;
@@ -2386,7 +2388,7 @@ static int stl_clrportstats(struct stlport *portp, comstats_t __user *cp)
return -ENODEV;
}
- mutex_lock(&portp->port.mutex);
+ mutex_lock_tty_off(&portp->port.mutex);
memset(&portp->stats, 0, sizeof(comstats_t));
portp->stats.brd = portp->brdnr;
portp->stats.panel = portp->panelnr;
diff --git a/drivers/char/synclink.c b/drivers/char/synclink.c
index 4de1246..da64db8 100644
--- a/drivers/char/synclink.c
+++ b/drivers/char/synclink.c
@@ -2435,7 +2435,7 @@ 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);
+ mutex_lock_tty_off(&info->port.mutex);
COPY_TO_USER(err, user_icount, &info->icount, sizeof(struct mgsl_icount));
mutex_unlock(&info->port.mutex);
if (err)
@@ -2462,7 +2462,7 @@ 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);
+ mutex_lock_tty_off(&info->port.mutex);
COPY_TO_USER(err,user_params, &info->params, sizeof(MGSL_PARAMS));
mutex_unlock(&info->port.mutex);
if (err) {
@@ -2504,7 +2504,7 @@ static int mgsl_set_params(struct mgsl_struct * info, MGSL_PARAMS __user *new_pa
return -EFAULT;
}
- mutex_lock(&info->port.mutex);
+ mutex_lock_tty_off(&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);
@@ -3111,7 +3111,7 @@ 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);
+ mutex_lock_tty_on(&info->port.mutex);
if (info->port.flags & ASYNC_INITIALIZED)
mgsl_wait_until_sent(tty, info->timeout);
mgsl_flush_buffer(tty);
diff --git a/drivers/char/synclink_gt.c b/drivers/char/synclink_gt.c
index c56b70a..171ec04 100644
--- a/drivers/char/synclink_gt.c
+++ b/drivers/char/synclink_gt.c
@@ -675,7 +675,7 @@ static int open(struct tty_struct *tty, struct file *filp)
goto cleanup;
}
- mutex_lock(&info->port.mutex);
+ mutex_lock_tty_on(&info->port.mutex);
info->port.tty->low_latency = (info->port.flags & ASYNC_LOW_LATENCY) ? 1 : 0;
spin_lock_irqsave(&info->netlock, flags);
@@ -726,7 +726,7 @@ 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);
+ mutex_lock_tty_on(&info->port.mutex);
if (info->port.flags & ASYNC_INITIALIZED)
wait_until_sent(tty, info->timeout);
flush_buffer(tty);
@@ -752,7 +752,7 @@ static void hangup(struct tty_struct *tty)
flush_buffer(tty);
- mutex_lock(&info->port.mutex);
+ mutex_lock_tty_on(&info->port.mutex);
shutdown(info);
spin_lock_irqsave(&info->port.lock, flags);
@@ -1076,7 +1076,7 @@ static int ioctl(struct tty_struct *tty, struct file *file,
case MGSL_IOCWAITGPIO:
return wait_gpio(info, argp);
}
- mutex_lock(&info->port.mutex);
+ mutex_lock_tty_off(&info->port.mutex);
switch (cmd) {
case MGSL_IOCGPARAMS:
ret = get_params(info, argp);
diff --git a/drivers/char/synclinkmp.c b/drivers/char/synclinkmp.c
index cfa581e..10b07a6 100644
--- a/drivers/char/synclinkmp.c
+++ b/drivers/char/synclinkmp.c
@@ -813,7 +813,7 @@ 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);
+ mutex_lock_tty_on(&info->port.mutex);
if (info->port.flags & ASYNC_INITIALIZED)
wait_until_sent(tty, info->timeout);
@@ -845,7 +845,7 @@ static void hangup(struct tty_struct *tty)
if (sanity_check(info, tty->name, "hangup"))
return;
- mutex_lock(&info->port.mutex);
+ mutex_lock_tty_on(&info->port.mutex);
flush_buffer(tty);
shutdown(info);
@@ -2876,7 +2876,7 @@ 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);
+ mutex_lock_tty_off(&info->port.mutex);
COPY_TO_USER(err, user_icount, &info->icount, sizeof(struct mgsl_icount));
mutex_unlock(&info->port.mutex);
if (err)
@@ -2893,7 +2893,7 @@ 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);
+ mutex_lock_tty_off(&info->port.mutex);
COPY_TO_USER(err,user_params, &info->params, sizeof(MGSL_PARAMS));
mutex_unlock(&info->port.mutex);
if (err) {
@@ -2923,7 +2923,7 @@ static int set_params(SLMP_INFO * info, MGSL_PARAMS __user *new_params)
return -EFAULT;
}
- mutex_lock(&info->port.mutex);
+ mutex_lock_tty_off(&info->port.mutex);
spin_lock_irqsave(&info->lock,flags);
memcpy(&info->params,&tmp_params,sizeof(MGSL_PARAMS));
spin_unlock_irqrestore(&info->lock,flags);
diff --git a/drivers/char/tty_port.c b/drivers/char/tty_port.c
index a3bd1d0..dbcfc48 100644
--- a/drivers/char/tty_port.c
+++ b/drivers/char/tty_port.c
@@ -118,7 +118,7 @@ EXPORT_SYMBOL(tty_port_tty_set);
static void tty_port_shutdown(struct tty_port *port)
{
- mutex_lock(&port->mutex);
+ mutex_lock_tty_on(&port->mutex);
if (port->ops->shutdown && !port->console &&
test_and_clear_bit(ASYNCB_INITIALIZED, &port->flags))
port->ops->shutdown(port);
@@ -424,7 +424,7 @@ int tty_port_open(struct tty_port *port, struct tty_struct *tty,
* port mutex.
*/
- mutex_lock(&port->mutex);
+ mutex_lock_tty_on(&port->mutex);
if (!test_bit(ASYNCB_INITIALIZED, &port->flags)) {
clear_bit(TTY_IO_ERROR, &tty->flags);
diff --git a/drivers/mmc/card/sdio_uart.c b/drivers/mmc/card/sdio_uart.c
index a071696..d9bb5bd 100644
--- a/drivers/mmc/card/sdio_uart.c
+++ b/drivers/mmc/card/sdio_uart.c
@@ -160,7 +160,7 @@ static void sdio_uart_port_remove(struct sdio_uart_port *port)
* give up on that port ASAP.
* Beware: the lock ordering is critical.
*/
- mutex_lock(&port->port.mutex);
+ mutex_lock_tty_off(&port->port.mutex);
mutex_lock(&port->func_lock);
func = port->func;
sdio_claim_host(func);
diff --git a/drivers/serial/pmac_zilog.c b/drivers/serial/pmac_zilog.c
index 700e108..1f3757e 100644
--- a/drivers/serial/pmac_zilog.c
+++ b/drivers/serial/pmac_zilog.c
@@ -1668,7 +1668,7 @@ static int pmz_suspend(struct macio_dev *mdev, pm_message_t pm_state)
state = pmz_uart_reg.state + uap->port.line;
mutex_lock(&pmz_irq_mutex);
- mutex_lock(&state->port.mutex);
+ mutex_lock_tty_off(&state->port.mutex);
spin_lock_irqsave(&uap->port.lock, flags);
@@ -1728,7 +1728,7 @@ static int pmz_resume(struct macio_dev *mdev)
state = pmz_uart_reg.state + uap->port.line;
mutex_lock(&pmz_irq_mutex);
- mutex_lock(&state->port.mutex);
+ mutex_lock_tty_off(&state->port.mutex);
spin_lock_irqsave(&uap->port.lock, flags);
if (!ZS_IS_OPEN(uap) && !ZS_IS_CONS(uap)) {
diff --git a/drivers/serial/serial_core.c b/drivers/serial/serial_core.c
index 803332b..a9d51eb 100644
--- a/drivers/serial/serial_core.c
+++ b/drivers/serial/serial_core.c
@@ -645,7 +645,7 @@ static int uart_get_info(struct uart_state *state,
/* Ensure the state we copy is consistent and no hardware changes
occur as we go */
- mutex_lock(&port->mutex);
+ mutex_lock_tty_off(&port->mutex);
tmp.type = uport->type;
tmp.line = uport->line;
@@ -704,7 +704,7 @@ static int uart_set_info(struct tty_struct *tty, struct uart_state *state,
* module insertion/removal doesn't change anything
* under us.
*/
- mutex_lock(&port->mutex);
+ mutex_lock_tty_off(&port->mutex);
change_irq = !(uport->flags & UPF_FIXED_PORT)
&& new_serial.irq != uport->irq;
@@ -913,7 +913,7 @@ static int uart_tiocmget(struct tty_struct *tty, struct file *file)
struct uart_port *uport = state->uart_port;
int result = -EIO;
- mutex_lock(&port->mutex);
+ mutex_lock_tty_off(&port->mutex);
if ((!file || !tty_hung_up_p(file)) &&
!(tty->flags & (1 << TTY_IO_ERROR))) {
result = uport->mctrl;
@@ -936,7 +936,7 @@ uart_tiocmset(struct tty_struct *tty, struct file *file,
struct tty_port *port = &state->port;
int ret = -EIO;
- mutex_lock(&port->mutex);
+ mutex_lock_tty_off(&port->mutex);
if ((!file || !tty_hung_up_p(file)) &&
!(tty->flags & (1 << TTY_IO_ERROR))) {
uart_update_mctrl(uport, set, clear);
@@ -952,7 +952,7 @@ static int uart_break_ctl(struct tty_struct *tty, int break_state)
struct tty_port *port = &state->port;
struct uart_port *uport = state->uart_port;
- mutex_lock(&port->mutex);
+ mutex_lock_tty_off(&port->mutex);
if (uport->type != PORT_UNKNOWN)
uport->ops->break_ctl(uport, break_state);
@@ -975,7 +975,7 @@ static int uart_do_autoconfig(struct tty_struct *tty,struct uart_state *state)
* changing, and hence any extra opens of the port while
* we're auto-configuring.
*/
- if (mutex_lock_interruptible(&port->mutex))
+ if (mutex_lock_interruptible_tty(&port->mutex))
return -ERESTARTSYS;
ret = -EBUSY;
@@ -1159,7 +1159,7 @@ uart_ioctl(struct tty_struct *tty, struct file *filp, unsigned int cmd,
if (ret != -ENOIOCTLCMD)
goto out;
- mutex_lock(&port->mutex);
+ mutex_lock_tty_off(&port->mutex);
if (tty_hung_up_p(filp)) {
ret = -EIO;
@@ -1283,7 +1283,7 @@ static void uart_close(struct tty_struct *tty, struct file *filp)
pr_debug("uart_close(%d) called\n", uport->line);
- mutex_lock(&port->mutex);
+ mutex_lock_tty_on(&port->mutex);
if (tty_hung_up_p(filp))
goto done;
@@ -1433,7 +1433,7 @@ static void uart_hangup(struct tty_struct *tty)
BUG_ON(!tty_locked());
pr_debug("uart_hangup(%d)\n", state->uart_port->line);
- mutex_lock(&port->mutex);
+ mutex_lock_tty_on(&port->mutex);
if (port->flags & ASYNC_NORMAL_ACTIVE) {
uart_flush_buffer(tty);
uart_shutdown(tty, state);
@@ -1551,7 +1551,7 @@ uart_block_til_ready(struct file *filp, struct uart_state *state)
mutex_unlock(&port->mutex);
schedule();
- mutex_lock(&port->mutex);
+ mutex_lock_tty_on(&port->mutex);
if (signal_pending(current))
break;
@@ -1579,7 +1579,7 @@ static struct uart_state *uart_get(struct uart_driver *drv, int line)
state = drv->state + line;
port = &state->port;
- if (mutex_lock_interruptible(&port->mutex)) {
+ if (mutex_lock_interruptible_tty(&port->mutex)) {
ret = -ERESTARTSYS;
goto err;
}
@@ -1736,7 +1736,7 @@ static void uart_line_info(struct seq_file *m, struct uart_driver *drv, int i)
}
if (capable(CAP_SYS_ADMIN)) {
- mutex_lock(&port->mutex);
+ mutex_lock_tty_off(&port->mutex);
pm_state = state->pm_state;
if (pm_state)
uart_change_pm(state, 0);
@@ -2016,7 +2016,7 @@ int uart_suspend_port(struct uart_driver *drv, struct uart_port *uport)
struct uart_match match = {uport, drv};
struct tty_struct *tty;
- mutex_lock(&port->mutex);
+ mutex_lock_tty_off(&port->mutex);
/* Must be inside the mutex lock until we convert to tty_port */
tty = port->tty;
@@ -2085,7 +2085,7 @@ int uart_resume_port(struct uart_driver *drv, struct uart_port *uport)
struct uart_match match = {uport, drv};
struct ktermios termios;
- mutex_lock(&port->mutex);
+ mutex_lock_tty_off(&port->mutex);
tty_dev = device_find_child(uport->dev, &match, serial_match_port);
if (!uport->suspended && device_may_wakeup(tty_dev)) {
@@ -2447,7 +2447,7 @@ int uart_add_one_port(struct uart_driver *drv, struct uart_port *uport)
port = &state->port;
mutex_lock(&port_mutex);
- mutex_lock(&port->mutex);
+ mutex_lock_tty_off(&port->mutex);
if (state->uart_port) {
ret = -EINVAL;
goto out;
@@ -2520,7 +2520,7 @@ int uart_remove_one_port(struct uart_driver *drv, struct uart_port *uport)
* Mark the port "dead" - this prevents any opens from
* succeeding while we shut down the port.
*/
- mutex_lock(&port->mutex);
+ mutex_lock_tty_off(&port->mutex);
uport->flags |= UPF_DEAD;
mutex_unlock(&port->mutex);
diff --git a/drivers/usb/serial/opticon.c b/drivers/usb/serial/opticon.c
index ed01f3b..ebc45e1 100644
--- a/drivers/usb/serial/opticon.c
+++ b/drivers/usb/serial/opticon.c
@@ -497,7 +497,7 @@ static int opticon_resume(struct usb_interface *intf)
struct usb_serial_port *port = serial->port[0];
int result;
- mutex_lock(&port->port.mutex);
+ mutex_lock_tty_off(&port->port.mutex);
/* This is protected by the port mutex against close/open */
if (test_bit(ASYNCB_INITIALIZED, &port->port.flags))
result = usb_submit_urb(priv->bulk_read_urb, GFP_NOIO);
--
1.7.0.4
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH 04/13] tty: make termios mutex nest under tty_lock
2010-05-04 22:33 [PATCH v2 00/13] BKL conversion in tty layer Arnd Bergmann
` (2 preceding siblings ...)
2010-05-04 22:33 ` [PATCH 03/13] tty: make tty_port->mutex nest under tty_lock Arnd Bergmann
@ 2010-05-04 22:33 ` Arnd Bergmann
2010-05-05 16:11 ` Arnd Bergmann
2010-05-04 22:33 ` [PATCH 05/13] tty: make ldisc_mutex " Arnd Bergmann
` (9 subsequent siblings)
13 siblings, 1 reply; 25+ messages in thread
From: Arnd Bergmann @ 2010-05-04 22:33 UTC (permalink / raw)
To: linux-kernel
Cc: Arnd Bergmann, Alan Cox, Greg KH, Frederic Weisbecker,
Thomas Gleixner, Andrew Morton, John Kacur, Al Viro, Ingo Molnar
Most of these are always called without the BTM held.
Annotate them so we know in the future which places
to look at. If we can change the code to never
get termios_mutex while holding the BTM, this
will solve all lock-order problems between the two.
tty_set_termios_ldisc and tty_reset_termios are currently
the only functions that always get termios_mutex while
already holding the BTM.
tty_throttle and tty_unthrottle are called from
receive_buf which in turn is called from a lot
of places. It needs more investigation to prove
that we never hold the BTM while calling these
two.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/char/pty.c | 2 +-
drivers/char/tty_io.c | 4 ++--
drivers/char/tty_ioctl.c | 37 ++++++++++++++++++++-----------------
drivers/char/tty_ldisc.c | 4 ++--
drivers/net/irda/irtty-sir.c | 5 +++--
drivers/staging/strip/strip.c | 2 +-
6 files changed, 29 insertions(+), 25 deletions(-)
diff --git a/drivers/char/pty.c b/drivers/char/pty.c
index 384e79f..dbb144b 100644
--- a/drivers/char/pty.c
+++ b/drivers/char/pty.c
@@ -232,7 +232,7 @@ int pty_resize(struct tty_struct *tty, struct winsize *ws)
struct tty_struct *pty = tty->link;
/* For a PTY we need to lock the tty side */
- mutex_lock(&tty->termios_mutex);
+ mutex_lock_tty_off(&tty->termios_mutex);
if (!memcmp(ws, &tty->winsize, sizeof(*ws)))
goto done;
diff --git a/drivers/char/tty_io.c b/drivers/char/tty_io.c
index 8331dd3..b1a40a1 100644
--- a/drivers/char/tty_io.c
+++ b/drivers/char/tty_io.c
@@ -2035,7 +2035,7 @@ static int tiocgwinsz(struct tty_struct *tty, struct winsize __user *arg)
{
int err;
- mutex_lock(&tty->termios_mutex);
+ mutex_lock_tty_off(&tty->termios_mutex);
err = copy_to_user(arg, &tty->winsize, sizeof(*arg));
mutex_unlock(&tty->termios_mutex);
@@ -2058,7 +2058,7 @@ int tty_do_resize(struct tty_struct *tty, struct winsize *ws)
unsigned long flags;
/* Lock the tty */
- mutex_lock(&tty->termios_mutex);
+ mutex_lock_tty_off(&tty->termios_mutex);
if (!memcmp(ws, &tty->winsize, sizeof(*ws)))
goto done;
/* Get the PID values and reference them so we can
diff --git a/drivers/char/tty_ioctl.c b/drivers/char/tty_ioctl.c
index 6bd5f88..4b57e73 100644
--- a/drivers/char/tty_ioctl.c
+++ b/drivers/char/tty_ioctl.c
@@ -99,11 +99,14 @@ EXPORT_SYMBOL(tty_driver_flush_buffer);
* Takes the termios mutex to protect against parallel throttle/unthrottle
* and also to ensure the driver can consistently reference its own
* termios data at this point when implementing software flow control.
+ *
+ * Locking: potentially called with BTM from n_tty_receive_buf, needs
+ * investigation.
*/
void tty_throttle(struct tty_struct *tty)
{
- mutex_lock(&tty->termios_mutex);
+ mutex_lock_tty(&tty->termios_mutex); /* BTM unknown */
/* check TTY_THROTTLED first so it indicates our state */
if (!test_and_set_bit(TTY_THROTTLED, &tty->flags) &&
tty->ops->throttle)
@@ -127,7 +130,7 @@ EXPORT_SYMBOL(tty_throttle);
void tty_unthrottle(struct tty_struct *tty)
{
- mutex_lock(&tty->termios_mutex);
+ mutex_lock_tty(&tty->termios_mutex); /* BTM unknown */
if (test_and_clear_bit(TTY_THROTTLED, &tty->flags) &&
tty->ops->unthrottle)
tty->ops->unthrottle(tty);
@@ -510,7 +513,7 @@ static void change_termios(struct tty_struct *tty, struct ktermios *new_termios)
/* FIXME: we need to decide on some locking/ordering semantics
for the set_termios notification eventually */
- mutex_lock(&tty->termios_mutex);
+ mutex_lock_tty_off(&tty->termios_mutex);
old_termios = *tty->termios;
*tty->termios = *new_termios;
unset_locked_termios(tty->termios, &old_termios, tty->termios_locked);
@@ -571,7 +574,7 @@ static int set_termios(struct tty_struct *tty, void __user *arg, int opt)
if (retval)
return retval;
- mutex_lock(&tty->termios_mutex);
+ mutex_lock_tty_off(&tty->termios_mutex);
memcpy(&tmp_termios, tty->termios, sizeof(struct ktermios));
mutex_unlock(&tty->termios_mutex);
@@ -625,14 +628,14 @@ static int set_termios(struct tty_struct *tty, void __user *arg, int opt)
static void copy_termios(struct tty_struct *tty, struct ktermios *kterm)
{
- mutex_lock(&tty->termios_mutex);
+ mutex_lock_tty_off(&tty->termios_mutex);
memcpy(kterm, tty->termios, sizeof(struct ktermios));
mutex_unlock(&tty->termios_mutex);
}
static void copy_termios_locked(struct tty_struct *tty, struct ktermios *kterm)
{
- mutex_lock(&tty->termios_mutex);
+ mutex_lock_tty_off(&tty->termios_mutex);
memcpy(kterm, tty->termios_locked, sizeof(struct ktermios));
mutex_unlock(&tty->termios_mutex);
}
@@ -681,7 +684,7 @@ static int set_termiox(struct tty_struct *tty, void __user *arg, int opt)
return -EINTR;
}
- mutex_lock(&tty->termios_mutex);
+ mutex_lock_tty_off(&tty->termios_mutex);
if (tty->ops->set_termiox)
tty->ops->set_termiox(tty, &tnew);
mutex_unlock(&tty->termios_mutex);
@@ -719,7 +722,7 @@ static int get_sgttyb(struct tty_struct *tty, struct sgttyb __user *sgttyb)
{
struct sgttyb tmp;
- mutex_lock(&tty->termios_mutex);
+ mutex_lock_tty_off(&tty->termios_mutex);
tmp.sg_ispeed = tty->termios->c_ispeed;
tmp.sg_ospeed = tty->termios->c_ospeed;
tmp.sg_erase = tty->termios->c_cc[VERASE];
@@ -780,7 +783,7 @@ static int set_sgttyb(struct tty_struct *tty, struct sgttyb __user *sgttyb)
if (copy_from_user(&tmp, sgttyb, sizeof(tmp)))
return -EFAULT;
- mutex_lock(&tty->termios_mutex);
+ mutex_lock_tty_off(&tty->termios_mutex);
termios = *tty->termios;
termios.c_cc[VERASE] = tmp.sg_erase;
termios.c_cc[VKILL] = tmp.sg_kill;
@@ -801,7 +804,7 @@ static int get_tchars(struct tty_struct *tty, struct tchars __user *tchars)
{
struct tchars tmp;
- mutex_lock(&tty->termios_mutex);
+ mutex_lock_tty_off(&tty->termios_mutex);
tmp.t_intrc = tty->termios->c_cc[VINTR];
tmp.t_quitc = tty->termios->c_cc[VQUIT];
tmp.t_startc = tty->termios->c_cc[VSTART];
@@ -818,7 +821,7 @@ static int set_tchars(struct tty_struct *tty, struct tchars __user *tchars)
if (copy_from_user(&tmp, tchars, sizeof(tmp)))
return -EFAULT;
- mutex_lock(&tty->termios_mutex);
+ mutex_lock_tty_off(&tty->termios_mutex);
tty->termios->c_cc[VINTR] = tmp.t_intrc;
tty->termios->c_cc[VQUIT] = tmp.t_quitc;
tty->termios->c_cc[VSTART] = tmp.t_startc;
@@ -835,7 +838,7 @@ static int get_ltchars(struct tty_struct *tty, struct ltchars __user *ltchars)
{
struct ltchars tmp;
- mutex_lock(&tty->termios_mutex);
+ mutex_lock_tty_off(&tty->termios_mutex);
tmp.t_suspc = tty->termios->c_cc[VSUSP];
/* what is dsuspc anyway? */
tmp.t_dsuspc = tty->termios->c_cc[VSUSP];
@@ -855,7 +858,7 @@ static int set_ltchars(struct tty_struct *tty, struct ltchars __user *ltchars)
if (copy_from_user(&tmp, ltchars, sizeof(tmp)))
return -EFAULT;
- mutex_lock(&tty->termios_mutex);
+ mutex_lock_tty_off(&tty->termios_mutex);
tty->termios->c_cc[VSUSP] = tmp.t_suspc;
/* what is dsuspc anyway? */
tty->termios->c_cc[VEOL2] = tmp.t_dsuspc;
@@ -913,7 +916,7 @@ static int tty_change_softcar(struct tty_struct *tty, int arg)
int bit = arg ? CLOCAL : 0;
struct ktermios old;
- mutex_lock(&tty->termios_mutex);
+ mutex_lock_tty_off(&tty->termios_mutex);
old = *tty->termios;
tty->termios->c_cflag &= ~CLOCAL;
tty->termios->c_cflag |= bit;
@@ -1022,7 +1025,7 @@ int tty_mode_ioctl(struct tty_struct *tty, struct file *file,
if (user_termios_to_kernel_termios(&kterm,
(struct termios __user *) arg))
return -EFAULT;
- mutex_lock(&real_tty->termios_mutex);
+ mutex_lock_tty_off(&real_tty->termios_mutex);
memcpy(real_tty->termios_locked, &kterm, sizeof(struct ktermios));
mutex_unlock(&real_tty->termios_mutex);
return 0;
@@ -1039,7 +1042,7 @@ int tty_mode_ioctl(struct tty_struct *tty, struct file *file,
if (user_termios_to_kernel_termios_1(&kterm,
(struct termios __user *) arg))
return -EFAULT;
- mutex_lock(&real_tty->termios_mutex);
+ mutex_lock_tty_off(&real_tty->termios_mutex);
memcpy(real_tty->termios_locked, &kterm, sizeof(struct ktermios));
mutex_unlock(&real_tty->termios_mutex);
return ret;
@@ -1049,7 +1052,7 @@ int tty_mode_ioctl(struct tty_struct *tty, struct file *file,
struct termiox ktermx;
if (real_tty->termiox == NULL)
return -EINVAL;
- mutex_lock(&real_tty->termios_mutex);
+ mutex_lock_tty_off(&real_tty->termios_mutex);
memcpy(&ktermx, real_tty->termiox, sizeof(struct termiox));
mutex_unlock(&real_tty->termios_mutex);
if (copy_to_user(p, &ktermx, sizeof(struct termiox)))
diff --git a/drivers/char/tty_ldisc.c b/drivers/char/tty_ldisc.c
index 97681ff..f0efca2 100644
--- a/drivers/char/tty_ldisc.c
+++ b/drivers/char/tty_ldisc.c
@@ -428,7 +428,7 @@ EXPORT_SYMBOL_GPL(tty_ldisc_flush);
static void tty_set_termios_ldisc(struct tty_struct *tty, int num)
{
- mutex_lock(&tty->termios_mutex);
+ mutex_lock_tty_on(&tty->termios_mutex);
tty->termios->c_line = num;
mutex_unlock(&tty->termios_mutex);
}
@@ -697,7 +697,7 @@ int tty_set_ldisc(struct tty_struct *tty, int ldisc)
static void tty_reset_termios(struct tty_struct *tty)
{
- mutex_lock(&tty->termios_mutex);
+ mutex_lock_tty_on(&tty->termios_mutex);
*tty->termios = tty->driver->init_termios;
tty->termios->c_ispeed = tty_termios_input_baud_rate(tty->termios);
tty->termios->c_ospeed = tty_termios_baud_rate(tty->termios);
diff --git a/drivers/net/irda/irtty-sir.c b/drivers/net/irda/irtty-sir.c
index ee1dde5..26d8230 100644
--- a/drivers/net/irda/irtty-sir.c
+++ b/drivers/net/irda/irtty-sir.c
@@ -34,6 +34,7 @@
#include <asm/uaccess.h>
#include <linux/delay.h>
#include <linux/mutex.h>
+#include <linux/sched.h>
#include <net/irda/irda.h>
#include <net/irda/irda_device.h>
@@ -123,7 +124,7 @@ static int irtty_change_speed(struct sir_dev *dev, unsigned speed)
tty = priv->tty;
- mutex_lock(&tty->termios_mutex);
+ mutex_lock_tty_off(&tty->termios_mutex);
old_termios = *(tty->termios);
cflag = tty->termios->c_cflag;
tty_encode_baud_rate(tty, speed, speed);
@@ -280,7 +281,7 @@ static inline void irtty_stop_receiver(struct tty_struct *tty, int stop)
struct ktermios old_termios;
int cflag;
- mutex_lock(&tty->termios_mutex);
+ mutex_lock_tty(&tty->termios_mutex); /* locked from tty->close */
old_termios = *(tty->termios);
cflag = tty->termios->c_cflag;
diff --git a/drivers/staging/strip/strip.c b/drivers/staging/strip/strip.c
index c976c6b..85e00d9 100644
--- a/drivers/staging/strip/strip.c
+++ b/drivers/staging/strip/strip.c
@@ -776,7 +776,7 @@ static void set_baud(struct tty_struct *tty, speed_t baudrate)
{
struct ktermios old_termios;
- mutex_lock(&tty->termios_mutex);
+ mutex_lock_tty_off(&tty->termios_mutex);
old_termios =*(tty->termios);
tty_encode_baud_rate(tty, baudrate, baudrate);
tty->ops->set_termios(tty, &old_termios);
--
1.7.0.4
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH 04/13] tty: make termios mutex nest under tty_lock
2010-05-04 22:33 ` [PATCH 04/13] tty: make termios mutex " Arnd Bergmann
@ 2010-05-05 16:11 ` Arnd Bergmann
0 siblings, 0 replies; 25+ messages in thread
From: Arnd Bergmann @ 2010-05-05 16:11 UTC (permalink / raw)
To: linux-kernel
Cc: Alan Cox, Greg KH, Frederic Weisbecker, Thomas Gleixner,
Andrew Morton, John Kacur, Al Viro, Ingo Molnar
On Wednesday 05 May 2010, Arnd Bergmann wrote:
> Most of these are always called without the BTM held.
> Annotate them so we know in the future which places
> to look at. If we can change the code to never
> get termios_mutex while holding the BTM, this
> will solve all lock-order problems between the two.
>
> tty_set_termios_ldisc and tty_reset_termios are currently
> the only functions that always get termios_mutex while
> already holding the BTM.
>
> tty_throttle and tty_unthrottle are called from
> receive_buf which in turn is called from a lot
> of places. It needs more investigation to prove
> that we never hold the BTM while calling these
> two.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Update: I have verified that this one is not needed
at all, because we never take the BTM/BKL while holding
termios_mutex. This is very helpful and will significantly
simplify things towards the end.
With this patch removed, my patch 13 (ldisc_mutex cleanup)
can potentially be applied again, I still need to check
some corner cases with line disciplines calling
tty_driver_flush_buffer and with v253_open calling tty->ops-write
under BTM+ldisc_mutex.
If no tty driver ever takes or releases the BTM/BKL in
their flush_buffer or write functions, things should be
fine.
Arnd
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 05/13] tty: make ldisc_mutex nest under tty_lock
2010-05-04 22:33 [PATCH v2 00/13] BKL conversion in tty layer Arnd Bergmann
` (3 preceding siblings ...)
2010-05-04 22:33 ` [PATCH 04/13] tty: make termios mutex " Arnd Bergmann
@ 2010-05-04 22:33 ` Arnd Bergmann
2010-05-04 22:33 ` [PATCH 06/13] tty: never hold BTM while getting tty_mutex Arnd Bergmann
` (8 subsequent siblings)
13 siblings, 0 replies; 25+ messages in thread
From: Arnd Bergmann @ 2010-05-04 22:33 UTC (permalink / raw)
To: linux-kernel
Cc: Arnd Bergmann, Alan Cox, Greg KH, Frederic Weisbecker,
Thomas Gleixner, Andrew Morton, John Kacur, Al Viro, Ingo Molnar
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/char/tty_io.c | 2 +-
drivers/char/tty_ldisc.c | 10 +++++-----
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/char/tty_io.c b/drivers/char/tty_io.c
index b1a40a1..82c723a 100644
--- a/drivers/char/tty_io.c
+++ b/drivers/char/tty_io.c
@@ -1284,7 +1284,7 @@ static int tty_reopen(struct tty_struct *tty)
tty->count++;
tty->driver = driver; /* N.B. why do this every time?? */
- mutex_lock(&tty->ldisc_mutex);
+ mutex_lock_tty_on(&tty->ldisc_mutex);
WARN_ON(!test_bit(TTY_LDISC, &tty->flags));
mutex_unlock(&tty->ldisc_mutex);
diff --git a/drivers/char/tty_ldisc.c b/drivers/char/tty_ldisc.c
index f0efca2..630e1ef 100644
--- a/drivers/char/tty_ldisc.c
+++ b/drivers/char/tty_ldisc.c
@@ -582,7 +582,7 @@ int tty_set_ldisc(struct tty_struct *tty, int ldisc)
tty_wait_until_sent(tty, 0);
- mutex_lock(&tty->ldisc_mutex);
+ mutex_lock_tty_off(&tty->ldisc_mutex);
/*
* We could be midstream of another ldisc change which has
@@ -593,7 +593,7 @@ int tty_set_ldisc(struct tty_struct *tty, int ldisc)
mutex_unlock(&tty->ldisc_mutex);
wait_event(tty_ldisc_wait,
test_bit(TTY_LDISC_CHANGING, &tty->flags) == 0);
- mutex_lock(&tty->ldisc_mutex);
+ mutex_lock_tty_off(&tty->ldisc_mutex);
}
tty_lock();
@@ -634,7 +634,7 @@ int tty_set_ldisc(struct tty_struct *tty, int ldisc)
flush_scheduled_work();
- mutex_lock(&tty->ldisc_mutex);
+ mutex_lock_tty_off(&tty->ldisc_mutex);
tty_lock();
if (test_bit(TTY_HUPPED, &tty->flags)) {
/* We were raced by the hangup method. It will have stomped
@@ -781,7 +781,7 @@ void tty_ldisc_hangup(struct tty_struct *tty)
*
* Avoid racing set_ldisc or tty_ldisc_release
*/
- mutex_lock(&tty->ldisc_mutex);
+ mutex_lock_tty_on(&tty->ldisc_mutex);
tty_ldisc_halt(tty);
/* At this point we have a closed ldisc and we want to
reopen it. We could defer this to the next open but
@@ -856,7 +856,7 @@ void tty_ldisc_release(struct tty_struct *tty, struct tty_struct *o_tty)
tty_ldisc_halt(tty);
flush_scheduled_work();
- mutex_lock(&tty->ldisc_mutex);
+ mutex_lock_tty_on(&tty->ldisc_mutex);
/*
* Now kill off the ldisc
*/
--
1.7.0.4
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH 06/13] tty: never hold BTM while getting tty_mutex
2010-05-04 22:33 [PATCH v2 00/13] BKL conversion in tty layer Arnd Bergmann
` (4 preceding siblings ...)
2010-05-04 22:33 ` [PATCH 05/13] tty: make ldisc_mutex " Arnd Bergmann
@ 2010-05-04 22:33 ` Arnd Bergmann
2010-05-04 22:33 ` [PATCH 07/13] tty: give up BTM in acquire_console_sem Arnd Bergmann
` (7 subsequent siblings)
13 siblings, 0 replies; 25+ messages in thread
From: Arnd Bergmann @ 2010-05-04 22:33 UTC (permalink / raw)
To: linux-kernel
Cc: Arnd Bergmann, Alan Cox, Greg KH, Frederic Weisbecker,
Thomas Gleixner, Andrew Morton, John Kacur, Al Viro, Ingo Molnar
tty_mutex is never taken with the BTM held, except for
two corner cases that are worked around here.
We give up the BTM before calling tty_release() in the
error path of tty_open().
Similarly, we reorder the locking in ptmx_open()
to get tty_mutex before the BTM.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/char/pty.c | 22 ++++++++--------------
drivers/char/tty_io.c | 13 +++++++------
2 files changed, 15 insertions(+), 20 deletions(-)
diff --git a/drivers/char/pty.c b/drivers/char/pty.c
index dbb144b..889de89 100644
--- a/drivers/char/pty.c
+++ b/drivers/char/pty.c
@@ -626,7 +626,7 @@ static const struct tty_operations pty_unix98_ops = {
* allocated_ptys_lock handles the list of free pty numbers
*/
-static int __ptmx_open(struct inode *inode, struct file *filp)
+static int ptmx_open(struct inode *inode, struct file *filp)
{
struct tty_struct *tty;
int retval;
@@ -635,11 +635,14 @@ static int __ptmx_open(struct inode *inode, struct file *filp)
nonseekable_open(inode, filp);
/* find a device that is not in use. */
+ tty_lock();
index = devpts_new_index(inode);
+ tty_unlock();
if (index < 0)
return index;
mutex_lock(&tty_mutex);
+ tty_lock();
tty = tty_init_dev(ptm_driver, index, 1);
mutex_unlock(&tty_mutex);
@@ -657,24 +660,15 @@ static int __ptmx_open(struct inode *inode, struct file *filp)
goto out1;
retval = ptm_driver->ops->open(tty, filp);
- if (!retval)
- return 0;
+ if (retval)
+ tty_release(inode, filp);
out1:
- tty_release(inode, filp);
+ tty_unlock();
return retval;
out:
devpts_kill_index(inode, index);
- return retval;
-}
-
-static int ptmx_open(struct inode *inode, struct file *filp)
-{
- int ret;
-
- tty_lock();
- ret = __ptmx_open(inode, filp);
tty_unlock();
- return ret;
+ return retval;
}
static struct file_operations ptmx_fops;
diff --git a/drivers/char/tty_io.c b/drivers/char/tty_io.c
index 82c723a..e1b2fca 100644
--- a/drivers/char/tty_io.c
+++ b/drivers/char/tty_io.c
@@ -1871,21 +1871,22 @@ got_driver:
printk(KERN_DEBUG "error %d in opening %s...", retval,
tty->name);
#endif
+ tty_unlock(); /* need to call tty_release without BTM */
tty_release(inode, filp);
- if (retval != -ERESTARTSYS) {
- tty_unlock();
+ if (retval != -ERESTARTSYS)
return retval;
- }
- if (signal_pending(current)) {
- tty_unlock();
+
+ if (signal_pending(current))
return retval;
- }
+
schedule();
/*
* Need to reset f_op in case a hangup happened.
*/
+ tty_lock();
if (filp->f_op == &hung_up_tty_fops)
filp->f_op = &tty_fops;
+ tty_unlock();
goto retry_open;
}
tty_unlock();
--
1.7.0.4
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH 07/13] tty: give up BTM in acquire_console_sem
2010-05-04 22:33 [PATCH v2 00/13] BKL conversion in tty layer Arnd Bergmann
` (5 preceding siblings ...)
2010-05-04 22:33 ` [PATCH 06/13] tty: never hold BTM while getting tty_mutex Arnd Bergmann
@ 2010-05-04 22:33 ` Arnd Bergmann
2010-05-04 22:33 ` [PATCH 08/13] tty: release tty lock when blocking Arnd Bergmann
` (6 subsequent siblings)
13 siblings, 0 replies; 25+ messages in thread
From: Arnd Bergmann @ 2010-05-04 22:33 UTC (permalink / raw)
To: linux-kernel
Cc: Arnd Bergmann, Alan Cox, Greg KH, Frederic Weisbecker,
Thomas Gleixner, Andrew Morton, John Kacur, Al Viro, Ingo Molnar
This was not caught in the initial sweep through
the TTY layer, nor by lockdep, which does not know
about classic semaphores.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
kernel/printk.c | 11 ++++++++++-
1 files changed, 10 insertions(+), 1 deletions(-)
diff --git a/kernel/printk.c b/kernel/printk.c
index 75077ad..30669a3 100644
--- a/kernel/printk.c
+++ b/kernel/printk.c
@@ -970,7 +970,16 @@ void resume_console(void)
void acquire_console_sem(void)
{
BUG_ON(in_interrupt());
- down(&console_sem);
+ if (down_trylock(&console_sem)) {
+ /*
+ * we may get called here with the BTM held.
+ * it's not clear whether we get the BTM while
+ * holding console_sem, need to check.
+ */
+ release_tty_lock(current);
+ down(&console_sem);
+ reacquire_tty_lock(current);
+ }
if (console_suspended)
return;
console_locked = 1;
--
1.7.0.4
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH 08/13] tty: release tty lock when blocking
2010-05-04 22:33 [PATCH v2 00/13] BKL conversion in tty layer Arnd Bergmann
` (6 preceding siblings ...)
2010-05-04 22:33 ` [PATCH 07/13] tty: give up BTM in acquire_console_sem Arnd Bergmann
@ 2010-05-04 22:33 ` Arnd Bergmann
2010-05-04 22:33 ` [PATCH 09/13] tty: implement BTM as mutex instead of BKL Arnd Bergmann
` (5 subsequent siblings)
13 siblings, 0 replies; 25+ messages in thread
From: Arnd Bergmann @ 2010-05-04 22:33 UTC (permalink / raw)
To: linux-kernel
Cc: Arnd Bergmann, Alan Cox, Greg KH, Frederic Weisbecker,
Thomas Gleixner, Andrew Morton, John Kacur, Al Viro, Ingo Molnar
All wait_event variants and work queue functions
that block on another thread need to release the
tty lock if the thread they are waiting on might
take it.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/char/cyclades.c | 2 +-
drivers/char/istallion.c | 12 ++++++++----
drivers/char/n_r3964.c | 2 +-
drivers/char/tty_buffer.c | 6 +++++-
drivers/char/tty_ioctl.c | 2 +-
drivers/char/tty_ldisc.c | 26 ++++++++++++++++++++++----
drivers/char/tty_port.c | 2 +-
drivers/char/vt_ioctl.c | 2 +-
drivers/serial/crisv10.c | 4 ++--
drivers/usb/class/cdc-acm.c | 2 +-
10 files changed, 43 insertions(+), 17 deletions(-)
diff --git a/drivers/char/cyclades.c b/drivers/char/cyclades.c
index 5b3b419..c425dbc 100644
--- a/drivers/char/cyclades.c
+++ b/drivers/char/cyclades.c
@@ -1607,7 +1607,7 @@ static int cy_open(struct tty_struct *tty, struct file *filp)
* If the port is the middle of closing, bail out now
*/
if (tty_hung_up_p(filp) || (info->port.flags & ASYNC_CLOSING)) {
- wait_event_interruptible(info->port.close_wait,
+ wait_event_interruptible_tty(info->port.close_wait,
!(info->port.flags & ASYNC_CLOSING));
return (info->port.flags & ASYNC_HUP_NOTIFY) ? -EAGAIN: -ERESTARTSYS;
}
diff --git a/drivers/char/istallion.c b/drivers/char/istallion.c
index 0618797..ab72858 100644
--- a/drivers/char/istallion.c
+++ b/drivers/char/istallion.c
@@ -955,7 +955,7 @@ static int stli_rawopen(struct stlibrd *brdp, struct stliport *portp, unsigned l
* order of opens and closes may not be preserved across shared
* memory, so we must wait until it is complete.
*/
- wait_event_interruptible(portp->raw_wait,
+ wait_event_interruptible_tty(portp->raw_wait,
!test_bit(ST_CLOSING, &portp->state));
if (signal_pending(current)) {
return -ERESTARTSYS;
@@ -990,7 +990,7 @@ static int stli_rawopen(struct stlibrd *brdp, struct stliport *portp, unsigned l
set_bit(ST_OPENING, &portp->state);
spin_unlock_irqrestore(&brd_lock, flags);
- wait_event_interruptible(portp->raw_wait,
+ wait_event_interruptible_tty(portp->raw_wait,
!test_bit(ST_OPENING, &portp->state));
if (signal_pending(current))
rc = -ERESTARTSYS;
@@ -1021,7 +1021,7 @@ static int stli_rawclose(struct stlibrd *brdp, struct stliport *portp, unsigned
* occurs on this port.
*/
if (wait) {
- wait_event_interruptible(portp->raw_wait,
+ wait_event_interruptible_tty(portp->raw_wait,
!test_bit(ST_CLOSING, &portp->state));
if (signal_pending(current)) {
return -ERESTARTSYS;
@@ -1053,7 +1053,7 @@ static int stli_rawclose(struct stlibrd *brdp, struct stliport *portp, unsigned
* to come back.
*/
rc = 0;
- wait_event_interruptible(portp->raw_wait,
+ wait_event_interruptible_tty(portp->raw_wait,
!test_bit(ST_CLOSING, &portp->state));
if (signal_pending(current))
rc = -ERESTARTSYS;
@@ -1074,6 +1074,10 @@ static int stli_rawclose(struct stlibrd *brdp, struct stliport *portp, unsigned
static int stli_cmdwait(struct stlibrd *brdp, struct stliport *portp, unsigned long cmd, void *arg, int size, int copyback)
{
+ /*
+ * no need for wait_event_tty because clearing ST_CMDING cannot block
+ * on BTM
+ */
wait_event_interruptible(portp->raw_wait,
!test_bit(ST_CMDING, &portp->state));
if (signal_pending(current))
diff --git a/drivers/char/n_r3964.c b/drivers/char/n_r3964.c
index f4bd259..a98290d 100644
--- a/drivers/char/n_r3964.c
+++ b/drivers/char/n_r3964.c
@@ -1079,7 +1079,7 @@ static ssize_t r3964_read(struct tty_struct *tty, struct file *file,
goto unlock;
}
/* block until there is a message: */
- wait_event_interruptible(pInfo->read_wait,
+ wait_event_interruptible_tty(pInfo->read_wait,
(pMsg = remove_msg(pInfo, pClient)));
}
diff --git a/drivers/char/tty_buffer.c b/drivers/char/tty_buffer.c
index 7ee5216..57db0a1 100644
--- a/drivers/char/tty_buffer.c
+++ b/drivers/char/tty_buffer.c
@@ -143,7 +143,11 @@ void tty_buffer_flush(struct tty_struct *tty)
if (test_bit(TTY_FLUSHING, &tty->flags)) {
set_bit(TTY_FLUSHPENDING, &tty->flags);
spin_unlock_irqrestore(&tty->buf.lock, flags);
- wait_event(tty->read_wait,
+ if (tty_locked())
+ wait_event_tty(tty->read_wait,
+ test_bit(TTY_FLUSHPENDING, &tty->flags) == 0);
+ else
+ wait_event(tty->read_wait,
test_bit(TTY_FLUSHPENDING, &tty->flags) == 0);
return;
} else
diff --git a/drivers/char/tty_ioctl.c b/drivers/char/tty_ioctl.c
index 4b57e73..6455434 100644
--- a/drivers/char/tty_ioctl.c
+++ b/drivers/char/tty_ioctl.c
@@ -158,7 +158,7 @@ void tty_wait_until_sent(struct tty_struct *tty, long timeout)
#endif
if (!timeout)
timeout = MAX_SCHEDULE_TIMEOUT;
- if (wait_event_interruptible_timeout(tty->write_wait,
+ if (wait_event_interruptible_timeout_tty(tty->write_wait,
!tty_chars_in_buffer(tty), timeout) >= 0) {
if (tty->ops->wait_until_sent)
tty->ops->wait_until_sent(tty, timeout);
diff --git a/drivers/char/tty_ldisc.c b/drivers/char/tty_ldisc.c
index 630e1ef..d4259cf 100644
--- a/drivers/char/tty_ldisc.c
+++ b/drivers/char/tty_ldisc.c
@@ -331,8 +331,16 @@ struct tty_ldisc *tty_ldisc_ref_wait(struct tty_struct *tty)
{
struct tty_ldisc *ld;
- /* wait_event is a macro */
- wait_event(tty_ldisc_wait, (ld = tty_ldisc_try(tty)) != NULL);
+ ld = tty_ldisc_try(tty);
+ if (ld)
+ return ld;
+
+ if (tty_locked())
+ /* need to give up BTM when coming from paste_selection */
+ wait_event_tty(tty_ldisc_wait, (ld = tty_ldisc_try(tty)) != NULL);
+ else
+ __wait_event(tty_ldisc_wait, (ld = tty_ldisc_try(tty)) != NULL);
+
return ld;
}
EXPORT_SYMBOL_GPL(tty_ldisc_ref_wait);
@@ -527,8 +535,17 @@ static void tty_ldisc_restore(struct tty_struct *tty, struct tty_ldisc *old)
static int tty_ldisc_halt(struct tty_struct *tty)
{
+ int ret;
clear_bit(TTY_LDISC, &tty->flags);
- return cancel_delayed_work_sync(&tty->buf.work);
+ if (tty_locked()) {
+ __release_tty_lock();
+ ret = cancel_delayed_work_sync(&tty->buf.work);
+ __reacquire_tty_lock();
+ } else {
+ ret = cancel_delayed_work_sync(&tty->buf.work);
+
+ }
+ return ret;
}
/**
@@ -854,8 +871,9 @@ void tty_ldisc_release(struct tty_struct *tty, struct tty_struct *o_tty)
*/
tty_ldisc_halt(tty);
+ release_tty_lock(current);
flush_scheduled_work();
-
+ reacquire_tty_lock(current);
mutex_lock_tty_on(&tty->ldisc_mutex);
/*
* Now kill off the ldisc
diff --git a/drivers/char/tty_port.c b/drivers/char/tty_port.c
index dbcfc48..70e1349 100644
--- a/drivers/char/tty_port.c
+++ b/drivers/char/tty_port.c
@@ -231,7 +231,7 @@ int tty_port_block_til_ready(struct tty_port *port,
/* block if port is in the process of being closed */
if (tty_hung_up_p(filp) || port->flags & ASYNC_CLOSING) {
- wait_event_interruptible(port->close_wait,
+ wait_event_interruptible_tty(port->close_wait,
!(port->flags & ASYNC_CLOSING));
if (port->flags & ASYNC_HUP_NOTIFY)
return -EAGAIN;
diff --git a/drivers/char/vt_ioctl.c b/drivers/char/vt_ioctl.c
index 0ebda0e..d54f73f 100644
--- a/drivers/char/vt_ioctl.c
+++ b/drivers/char/vt_ioctl.c
@@ -133,7 +133,7 @@ static void vt_event_wait(struct vt_event_wait *vw)
list_add(&vw->list, &vt_events);
spin_unlock_irqrestore(&vt_event_lock, flags);
/* Wait for it to pass */
- wait_event_interruptible(vt_event_waitqueue, vw->done);
+ wait_event_interruptible_tty(vt_event_waitqueue, vw->done);
/* Dequeue it */
spin_lock_irqsave(&vt_event_lock, flags);
list_del(&vw->list);
diff --git a/drivers/serial/crisv10.c b/drivers/serial/crisv10.c
index d11d898..e6a1cb7 100644
--- a/drivers/serial/crisv10.c
+++ b/drivers/serial/crisv10.c
@@ -3981,7 +3981,7 @@ block_til_ready(struct tty_struct *tty, struct file * filp,
*/
if (tty_hung_up_p(filp) ||
(info->flags & ASYNC_CLOSING)) {
- wait_event_interruptible(info->close_wait,
+ wait_event_interruptible_tty(info->close_wait,
!(info->flags & ASYNC_CLOSING));
#ifdef SERIAL_DO_RESTART
if (info->flags & ASYNC_HUP_NOTIFY)
@@ -4139,7 +4139,7 @@ rs_open(struct tty_struct *tty, struct file * filp)
*/
if (tty_hung_up_p(filp) ||
(info->flags & ASYNC_CLOSING)) {
- wait_event_interruptible(info->close_wait,
+ wait_event_interruptible_tty(info->close_wait,
!(info->flags & ASYNC_CLOSING));
#ifdef SERIAL_DO_RESTART
return ((info->flags & ASYNC_HUP_NOTIFY) ?
diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
index 5e1a253..bdd4bfc 100644
--- a/drivers/usb/class/cdc-acm.c
+++ b/drivers/usb/class/cdc-acm.c
@@ -645,7 +645,7 @@ static void acm_port_down(struct acm *acm, int drain)
acm_set_control(acm, acm->ctrlout = 0);
/* try letting the last writes drain naturally */
if (drain) {
- wait_event_interruptible_timeout(acm->drain_wait,
+ wait_event_interruptible_timeout_tty(acm->drain_wait,
(ACM_NW == acm_wb_is_avail(acm)) || !acm->dev,
ACM_CLOSE_TIMEOUT * HZ);
}
--
1.7.0.4
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH 09/13] tty: implement BTM as mutex instead of BKL
2010-05-04 22:33 [PATCH v2 00/13] BKL conversion in tty layer Arnd Bergmann
` (7 preceding siblings ...)
2010-05-04 22:33 ` [PATCH 08/13] tty: release tty lock when blocking Arnd Bergmann
@ 2010-05-04 22:33 ` Arnd Bergmann
2010-05-04 22:33 ` [PATCH 10/13] tty: untangle locking of wait_until_sent Arnd Bergmann
` (4 subsequent siblings)
13 siblings, 0 replies; 25+ messages in thread
From: Arnd Bergmann @ 2010-05-04 22:33 UTC (permalink / raw)
To: linux-kernel
Cc: Arnd Bergmann, Alan Cox, Greg KH, Frederic Weisbecker,
Thomas Gleixner, Andrew Morton, John Kacur, Al Viro, Ingo Molnar
The TTY layer now has its own ways to deal with recursive
locking and release-on-sleep for the tty_lock() calls,
meaning that it's safe to replace the Big Kernel Lock
with a subsystem specific Big TTY Mutex (BTM).
This patch for now makes the new behaviour an optional
experimental feature that can be enabled for testing
purposes.
Using a regular mutex here will change the behaviour
when blocked on the BTM from spinning to sleeping,
but that should be visible to the user.
Using the mutex also means that all the BTM is now
covered by lockdep.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/char/Makefile | 1 +
drivers/char/tty_mutex.c | 100 +++++++++++++++++++++++++++++++++++++++++++++
include/linux/init_task.h | 1 +
include/linux/sched.h | 1 +
include/linux/tty.h | 19 +++++++++
kernel/fork.c | 1 +
lib/Kconfig.debug | 10 +++++
7 files changed, 133 insertions(+), 0 deletions(-)
create mode 100644 drivers/char/tty_mutex.c
diff --git a/drivers/char/Makefile b/drivers/char/Makefile
index f957edf..74ee3fa 100644
--- a/drivers/char/Makefile
+++ b/drivers/char/Makefile
@@ -9,6 +9,7 @@ FONTMAPFILE = cp437.uni
obj-y += mem.o random.o tty_io.o n_tty.o tty_ioctl.o tty_ldisc.o tty_buffer.o tty_port.o
+obj-$(CONFIG_TTY_MUTEX) += tty_mutex.o
obj-$(CONFIG_LEGACY_PTYS) += pty.o
obj-$(CONFIG_UNIX98_PTYS) += pty.o
obj-y += misc.o
diff --git a/drivers/char/tty_mutex.c b/drivers/char/tty_mutex.c
new file mode 100644
index 0000000..51e0852
--- /dev/null
+++ b/drivers/char/tty_mutex.c
@@ -0,0 +1,100 @@
+/*
+ * drivers/char/tty_lock.c
+ */
+#include <linux/tty.h>
+#include <linux/module.h>
+#include <linux/kallsyms.h>
+#include <linux/semaphore.h>
+#include <linux/sched.h>
+
+/*
+ * The 'big kernel semaphore'
+ *
+ * This mutex is taken and released recursively by tty_lock()
+ * and tty_unlock(). It is transparently dropped and reacquired
+ * over schedule(). It is used to protect legacy code that hasn't
+ * been migrated to a proper locking design yet.
+ *
+ * Note: code locked by this semaphore will only be serialized against
+ * other code using the same locking facility. The code guarantees that
+ * the task remains on the same CPU.
+ *
+ * Don't use in new code.
+ */
+static DEFINE_MUTEX(big_tty_mutex);
+
+/*
+ * Re-acquire the kernel semaphore.
+ *
+ * This function is called with preemption off.
+ *
+ * We are executing in schedule() so the code must be extremely careful
+ * about recursion, both due to the down() and due to the enabling of
+ * preemption. schedule() will re-check the preemption flag after
+ * reacquiring the semaphore.
+ */
+int __lockfunc __reacquire_tty_lock(void)
+{
+ struct task_struct *task = current;
+ int saved_tty_lock_depth = task->tty_lock_depth;
+
+ BUG_ON(saved_tty_lock_depth < 0);
+
+ task->tty_lock_depth = -1;
+ preempt_enable_no_resched();
+
+ mutex_lock(&big_tty_mutex);
+
+ preempt_disable();
+ task->tty_lock_depth = saved_tty_lock_depth;
+
+ return 0;
+}
+EXPORT_SYMBOL(__reacquire_tty_lock);
+
+void __lockfunc __release_tty_lock(void)
+{
+ mutex_unlock(&big_tty_mutex);
+}
+EXPORT_SYMBOL(__release_tty_lock);
+
+/*
+ * Getting the big tty mutex.
+ */
+void __lockfunc tty_lock(void)
+{
+ struct task_struct *task = current;
+ int depth = task->tty_lock_depth + 1;
+
+ if (!WARN_ON(depth))
+ mutex_lock(&big_tty_mutex);
+
+ task->tty_lock_depth = depth;
+}
+EXPORT_SYMBOL(tty_lock);
+
+void __lockfunc tty_lock_nested(void)
+{
+ struct task_struct *task = current;
+ int depth = task->tty_lock_depth + 1;
+
+ if (likely(!depth))
+ /*
+ * No recursion worries - we set up tty_lock_depth _after_
+ */
+ mutex_lock(&big_tty_mutex);
+
+ task->tty_lock_depth = depth;
+}
+EXPORT_SYMBOL(tty_lock_nested);
+
+void __lockfunc tty_unlock(void)
+{
+ struct task_struct *task = current;
+
+ BUG_ON(task->tty_lock_depth < 0);
+
+ if (likely(--task->tty_lock_depth < 0))
+ mutex_unlock(&big_tty_mutex);
+}
+EXPORT_SYMBOL(tty_unlock);
diff --git a/include/linux/init_task.h b/include/linux/init_task.h
index b1ed1cd..3c0b4ab 100644
--- a/include/linux/init_task.h
+++ b/include/linux/init_task.h
@@ -114,6 +114,7 @@ extern struct cred init_cred;
.usage = ATOMIC_INIT(2), \
.flags = PF_KTHREAD, \
.lock_depth = -1, \
+ .tty_lock_depth = -1, \
.prio = MAX_PRIO-20, \
.static_prio = MAX_PRIO-20, \
.normal_prio = MAX_PRIO-20, \
diff --git a/include/linux/sched.h b/include/linux/sched.h
index dad7f66..5f03259 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1175,6 +1175,7 @@ struct task_struct {
unsigned int ptrace;
int lock_depth; /* BKL lock depth */
+ int tty_lock_depth; /* TTY lock depth */
#ifdef CONFIG_SMP
#ifdef __ARCH_WANT_UNLOCKED_CTXSW
diff --git a/include/linux/tty.h b/include/linux/tty.h
index 60b3d69..1659ba8 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -572,6 +572,7 @@ extern int vt_ioctl(struct tty_struct *tty, struct file *file,
extern long vt_compat_ioctl(struct tty_struct *tty, struct file * file,
unsigned int cmd, unsigned long arg);
+/* tty_mutex.c */
/* functions for preparation of BKL removal */
/*
@@ -584,6 +585,22 @@ extern long vt_compat_ioctl(struct tty_struct *tty, struct file * file,
* be shown to never get called with this held already, it should
* use tty_lock() instead.
*/
+#ifdef CONFIG_TTY_MUTEX
+extern void __lockfunc tty_lock_nested(void) __acquires(tty_lock);
+extern void __lockfunc tty_lock(void) __acquires(tty_lock);
+extern void __lockfunc tty_unlock(void) __releases(tty_lock);
+#define tty_locked() (current->tty_lock_depth >= 0)
+int __lockfunc __reacquire_tty_lock(void);
+void __lockfunc __release_tty_lock(void);
+#define release_tty_lock(tsk) do { \
+ if (unlikely((tsk)->tty_lock_depth >= 0)) \
+ __release_tty_lock(); \
+} while (0)
+#define reacquire_tty_lock(tsk) \
+ ((tsk->tty_lock_depth >= 0) ? \
+ __reacquire_tty_lock() : 0 )
+
+#else
static inline void __lockfunc tty_lock_nested(void) __acquires(kernel_lock)
{
lock_kernel();
@@ -609,6 +626,8 @@ static inline void __release_tty_lock(void)
#define release_tty_lock(tsk) do { } while (0)
#define reacquire_tty_lock(tsk) do { } while (0)
+#endif
+
/*
* mutex_lock_tty - lock a mutex without holding the BTM
*
diff --git a/kernel/fork.c b/kernel/fork.c
index 44b0791..cf81613 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -1064,6 +1064,7 @@ static struct task_struct *copy_process(unsigned long clone_flags,
posix_cpu_timers_init(p);
p->lock_depth = -1; /* -1 = no lock */
+ p->tty_lock_depth = -1; /* -1 = no lock */
do_posix_clock_monotonic_gettime(&p->start_time);
p->real_start_time = p->start_time;
monotonic_to_bootbased(&p->real_start_time);
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 935248b..0b3e1ad 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -428,6 +428,16 @@ config RT_MUTEX_TESTER
help
This option enables a rt-mutex tester.
+config TTY_MUTEX
+ bool "Use a mutex instead of BKL for TTY locking"
+ depends on EXPERIMENTAL && SMP
+ help
+ The TTY subsystem traditionally depends on the big kernel lock
+ for serialization. Saying Y here replaces the BKL with the Big
+ TTY Mutex (BTM).
+ Building a kernel without the BKL is only possible with TTY_MUTEX
+ enabled.
+
config DEBUG_SPINLOCK
bool "Spinlock and rw-lock debugging: basic checks"
depends on DEBUG_KERNEL
--
1.7.0.4
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH 10/13] tty: untangle locking of wait_until_sent
2010-05-04 22:33 [PATCH v2 00/13] BKL conversion in tty layer Arnd Bergmann
` (8 preceding siblings ...)
2010-05-04 22:33 ` [PATCH 09/13] tty: implement BTM as mutex instead of BKL Arnd Bergmann
@ 2010-05-04 22:33 ` Arnd Bergmann
2010-05-05 19:59 ` Alan Cox
2010-05-04 22:33 ` [PATCH 11/13] tty: remove tty_lock_nested Arnd Bergmann
` (3 subsequent siblings)
13 siblings, 1 reply; 25+ messages in thread
From: Arnd Bergmann @ 2010-05-04 22:33 UTC (permalink / raw)
To: linux-kernel
Cc: Arnd Bergmann, Alan Cox, Greg KH, Frederic Weisbecker,
Thomas Gleixner, Andrew Morton, John Kacur, Al Viro, Ingo Molnar
Some wait_until_sent versions require the big
tty mutex, others don't and some callers of
wait_until_sent already hold it while other don't.
That leads to recursive use of the BTM in these
functions, which we're trying to get rid of.
This patch changes both the implementations and
the users of wait_until_sent so that the
device driver implementations are always called
with the lock held so they don't need to get
it themselves.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/char/amiserial.c | 6 +++---
drivers/char/generic_serial.c | 2 +-
drivers/char/hvc_console.c | 2 +-
drivers/char/hvcs.c | 2 +-
drivers/char/ip2/ip2main.c | 20 +++++++++++++++++---
drivers/char/serial167.c | 6 +++---
drivers/char/specialix.c | 2 +-
drivers/char/tty_ioctl.c | 36 ++++++++++++++++++++++++++++++++++--
drivers/char/tty_port.c | 2 +-
drivers/isdn/i4l/isdn_tty.c | 2 +-
drivers/serial/68328serial.c | 2 +-
drivers/serial/68360serial.c | 5 ++---
drivers/serial/crisv10.c | 12 +++++++-----
drivers/serial/serial_core.c | 5 +----
include/linux/tty.h | 1 +
net/irda/ircomm/ircomm_tty.c | 2 +-
16 files changed, 76 insertions(+), 31 deletions(-)
diff --git a/drivers/char/amiserial.c b/drivers/char/amiserial.c
index 5bd382e..bfcf520 100644
--- a/drivers/char/amiserial.c
+++ b/drivers/char/amiserial.c
@@ -1479,7 +1479,7 @@ static void rs_close(struct tty_struct *tty, struct file * filp)
*/
tty->closing = 1;
if (info->closing_wait != ASYNC_CLOSING_WAIT_NONE)
- tty_wait_until_sent(tty, info->closing_wait);
+ tty_wait_until_sent_locked(tty, info->closing_wait);
/*
* At this point we stop accepting input. To do this, we
* disable the receive line status interrupts, and tell the
@@ -1537,7 +1537,7 @@ static void rs_wait_until_sent(struct tty_struct *tty, int timeout)
orig_jiffies = jiffies;
- tty_lock_nested(); /* tty_wait_until_sent is called from lots of places */
+ WARN_ON(!tty_locked());
/*
* 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
@@ -1578,12 +1578,12 @@ static void rs_wait_until_sent(struct tty_struct *tty, int timeout)
break;
}
__set_current_state(TASK_RUNNING);
- tty_unlock();
#ifdef SERIAL_DEBUG_RS_WAIT_UNTIL_SENT
printk("lsr = %d (jiff=%lu)...done\n", lsr, jiffies);
#endif
}
+
/*
* rs_hangup() --- called by tty_hangup() when a hangup is signaled.
*/
diff --git a/drivers/char/generic_serial.c b/drivers/char/generic_serial.c
index 5954ee1..400830d 100644
--- a/drivers/char/generic_serial.c
+++ b/drivers/char/generic_serial.c
@@ -557,7 +557,7 @@ void gs_close(struct tty_struct * tty, struct file * filp)
*/
tty->closing = 1;
/* if (port->closing_wait != ASYNC_CLOSING_WAIT_NONE)
- tty_wait_until_sent(tty, port->closing_wait); */
+ tty_wait_until_sent_locked(tty, port->closing_wait); */
/*
* At this point we stop accepting input. To do this, we
diff --git a/drivers/char/hvc_console.c b/drivers/char/hvc_console.c
index 35cca4c..79d45cc 100644
--- a/drivers/char/hvc_console.c
+++ b/drivers/char/hvc_console.c
@@ -385,7 +385,7 @@ static void hvc_close(struct tty_struct *tty, struct file * filp)
* there is no buffered data otherwise sleeps on a wait queue
* waking periodically to check chars_in_buffer().
*/
- tty_wait_until_sent(tty, HVC_CLOSE_WAIT);
+ tty_wait_until_sent_locked(tty, HVC_CLOSE_WAIT);
} else {
if (hp->count < 0)
printk(KERN_ERR "hvc_close %X: oops, count is %d\n",
diff --git a/drivers/char/hvcs.c b/drivers/char/hvcs.c
index bedc6c1..4710438 100644
--- a/drivers/char/hvcs.c
+++ b/drivers/char/hvcs.c
@@ -1228,7 +1228,7 @@ static void hvcs_close(struct tty_struct *tty, struct file *filp)
irq = hvcsd->vdev->irq;
spin_unlock_irqrestore(&hvcsd->lock, flags);
- tty_wait_until_sent(tty, HVCS_CLOSE_WAIT);
+ tty_wait_until_sent_locked(tty, HVCS_CLOSE_WAIT);
/*
* This line is important because it tells hvcs_open that this
diff --git a/drivers/char/ip2/ip2main.c b/drivers/char/ip2/ip2main.c
index 911e1da..68642ee 100644
--- a/drivers/char/ip2/ip2main.c
+++ b/drivers/char/ip2/ip2main.c
@@ -193,6 +193,7 @@ static void do_input(struct work_struct *);
static void do_status(struct work_struct *);
static void ip2_wait_until_sent(PTTY,int);
+static void __ip2_wait_until_sent(PTTY,int);
static void set_params (i2ChanStrPtr, struct ktermios *);
static int get_serial_info(i2ChanStrPtr, struct serial_struct __user *);
@@ -1634,7 +1635,7 @@ ip2_close( PTTY tty, struct file *pFile )
* This uses an timeout, after which the close
* completes.
*/
- ip2_wait_until_sent(tty, pCh->ClosingWaitTime );
+ __ip2_wait_until_sent(tty, pCh->ClosingWaitTime );
}
/*
* At this point we stop accepting input. Here we flush the channel
@@ -1925,16 +1926,29 @@ ip2_flush_buffer( PTTY tty )
/* indeterminate number of bytes buffered on the board. */
/******************************************************************************/
static void
-ip2_wait_until_sent ( PTTY tty, int timeout )
+__ip2_wait_until_sent ( PTTY tty, int timeout )
{
int i = jiffies;
i2ChanStrPtr pCh = tty->driver_data;
- tty_wait_until_sent(tty, timeout );
+ tty_wait_until_sent_locked(tty, timeout );
if ( (i = timeout - (jiffies -i)) > 0)
i2DrainOutput( pCh, i );
}
+static void
+ip2_wait_until_sent ( PTTY tty, int timeout )
+{
+ /*
+ * BTM is held when coming from close,
+ * but not from ->ioctl
+ */
+ tty_lock();
+ __ip2_wait_until_sent(tty, timeout);
+ tty_unlock();
+}
+
+
/******************************************************************************/
/******************************************************************************/
/* Device Input Section */
diff --git a/drivers/char/serial167.c b/drivers/char/serial167.c
index 26a7089..ae495f1 100644
--- a/drivers/char/serial167.c
+++ b/drivers/char/serial167.c
@@ -1571,7 +1571,7 @@ cy_ioctl(struct tty_struct *tty, struct file *file,
ret_val = tty_check_change(tty);
if (ret_val)
break;
- tty_wait_until_sent(tty, 0);
+ tty_wait_until_sent_locked(tty, 0);
if (!arg)
send_break(info, HZ / 4); /* 1/4 second */
break;
@@ -1579,7 +1579,7 @@ cy_ioctl(struct tty_struct *tty, struct file *file,
ret_val = tty_check_change(tty);
if (ret_val)
break;
- tty_wait_until_sent(tty, 0);
+ tty_wait_until_sent_locked(tty, 0);
send_break(info, arg ? arg * (HZ / 10) : HZ / 4);
break;
@@ -1670,7 +1670,7 @@ static void cy_close(struct tty_struct *tty, struct file *filp)
return;
info->flags |= ASYNC_CLOSING;
if (info->flags & ASYNC_INITIALIZED)
- tty_wait_until_sent(tty, 3000); /* 30 seconds timeout */
+ tty_wait_until_sent_locked(tty, 3000); /* 30 seconds timeout */
shutdown(info);
cy_flush_buffer(tty);
tty_ldisc_flush(tty);
diff --git a/drivers/char/specialix.c b/drivers/char/specialix.c
index 234aefc..79ac31a 100644
--- a/drivers/char/specialix.c
+++ b/drivers/char/specialix.c
@@ -1516,7 +1516,7 @@ static void sx_close(struct tty_struct *tty, struct file *filp)
spin_unlock_irqrestore(&port->lock, flags);
dprintk(SX_DEBUG_OPEN, "Closing\n");
if (port->port.closing_wait != ASYNC_CLOSING_WAIT_NONE)
- tty_wait_until_sent(tty, port->port.closing_wait);
+ tty_wait_until_sent_locked(tty, port->port.closing_wait);
/*
* At this point we stop accepting input. To do this, we
* disable the receive line status interrupts, and tell the
diff --git a/drivers/char/tty_ioctl.c b/drivers/char/tty_ioctl.c
index 6455434..0d402eb 100644
--- a/drivers/char/tty_ioctl.c
+++ b/drivers/char/tty_ioctl.c
@@ -146,7 +146,7 @@ EXPORT_SYMBOL(tty_unthrottle);
* Wait for characters pending in a tty driver to hit the wire, or
* for a timeout to occur (eg due to flow control)
*
- * Locking: none
+ * Locking: must be called with BTM not held
*/
void tty_wait_until_sent(struct tty_struct *tty, long timeout)
@@ -156,6 +156,38 @@ void tty_wait_until_sent(struct tty_struct *tty, long timeout)
printk(KERN_DEBUG "%s wait until sent...\n", tty_name(tty, buf));
#endif
+ WARN_ON_ONCE(tty_locked());
+ if (!timeout)
+ timeout = MAX_SCHEDULE_TIMEOUT;
+ if (wait_event_interruptible_timeout(tty->write_wait,
+ !tty_chars_in_buffer(tty), timeout) >= 0) {
+ if (tty->ops->wait_until_sent) {
+ tty_lock();
+ tty->ops->wait_until_sent(tty, timeout);
+ tty_unlock();
+ }
+ }
+}
+EXPORT_SYMBOL(tty_wait_until_sent);
+
+/**
+ * tty_wait_until_sent - wait for I/O to finish
+ * @tty: tty we are waiting for
+ * @timeout: how long we will wait
+ *
+ * Wait for characters pending in a tty driver to hit the wire, or
+ * for a timeout to occur (eg due to flow control)
+ *
+ * Locking: must be called with BTM held
+ */
+void tty_wait_until_sent_locked(struct tty_struct *tty, long timeout)
+{
+#ifdef TTY_DEBUG_WAIT_UNTIL_SENT
+ char buf[64];
+
+ printk(KERN_DEBUG "%s wait until sent...\n", tty_name(tty, buf));
+#endif
+ WARN_ON_ONCE(!tty_locked());
if (!timeout)
timeout = MAX_SCHEDULE_TIMEOUT;
if (wait_event_interruptible_timeout_tty(tty->write_wait,
@@ -164,7 +196,7 @@ void tty_wait_until_sent(struct tty_struct *tty, long timeout)
tty->ops->wait_until_sent(tty, timeout);
}
}
-EXPORT_SYMBOL(tty_wait_until_sent);
+EXPORT_SYMBOL(tty_wait_until_sent_locked);
/*
diff --git a/drivers/char/tty_port.c b/drivers/char/tty_port.c
index 70e1349..7ee4999 100644
--- a/drivers/char/tty_port.c
+++ b/drivers/char/tty_port.c
@@ -348,7 +348,7 @@ int tty_port_close_start(struct tty_port *port,
tty_driver_flush_buffer(tty);
if (test_bit(ASYNCB_INITIALIZED, &port->flags) &&
port->closing_wait != ASYNC_CLOSING_WAIT_NONE)
- tty_wait_until_sent(tty, port->closing_wait);
+ tty_wait_until_sent_locked(tty, port->closing_wait);
if (port->drain_delay) {
unsigned int bps = tty_get_baud_rate(tty);
long timeout;
diff --git a/drivers/isdn/i4l/isdn_tty.c b/drivers/isdn/i4l/isdn_tty.c
index fd91de8..50102a8 100644
--- a/drivers/isdn/i4l/isdn_tty.c
+++ b/drivers/isdn/i4l/isdn_tty.c
@@ -1695,7 +1695,7 @@ isdn_tty_close(struct tty_struct *tty, struct file *filp)
* line status register.
*/
if (info->flags & ISDN_ASYNC_INITIALIZED) {
- tty_wait_until_sent(tty, 3000); /* 30 seconds timeout */
+ tty_wait_until_sent_locked(tty, 3000); /* 30 seconds timeout */
/*
* Before we drop DTR, make sure the UART transmitter
* has completely drained; this is especially
diff --git a/drivers/serial/68328serial.c b/drivers/serial/68328serial.c
index 78ed24b..f20d90c 100644
--- a/drivers/serial/68328serial.c
+++ b/drivers/serial/68328serial.c
@@ -1108,7 +1108,7 @@ static void rs_close(struct tty_struct *tty, struct file * filp)
*/
tty->closing = 1;
if (info->closing_wait != S_CLOSING_WAIT_NONE)
- tty_wait_until_sent(tty, info->closing_wait);
+ tty_wait_until_sent_locked(tty, info->closing_wait);
/*
* At this point we stop accepting input. To do this, we
* disable the receive line status interrupts, and tell the
diff --git a/drivers/serial/68360serial.c b/drivers/serial/68360serial.c
index c17a595..a56c5a5 100644
--- a/drivers/serial/68360serial.c
+++ b/drivers/serial/68360serial.c
@@ -1626,7 +1626,7 @@ static void rs_360_close(struct tty_struct *tty, struct file * filp)
*/
tty->closing = 1;
if (info->closing_wait != ASYNC_CLOSING_WAIT_NONE)
- tty_wait_until_sent(tty, info->closing_wait);
+ tty_wait_until_sent_locked(tty, info->closing_wait);
/*
* At this point we stop accepting input. To do this, we
* disable the receive line status interrupts, and tell the
@@ -1705,7 +1705,7 @@ static void rs_360_wait_until_sent(struct tty_struct *tty, int timeout)
printk("jiff=%lu...", jiffies);
#endif
- tty_lock_nested(); /* always held already since we come from ->close */
+ WARN_ON(!tty_locked()); /* always held already since we come from ->close */
/* We go through the loop at least once because we can't tell
* exactly when the last character exits the shifter. There can
* be at least two characters waiting to be sent after the buffers
@@ -1734,7 +1734,6 @@ static void rs_360_wait_until_sent(struct tty_struct *tty, int timeout)
bdp--;
} while (bdp->status & BD_SC_READY);
current->state = TASK_RUNNING;
- tty_unlock();
#ifdef SERIAL_DEBUG_RS_WAIT_UNTIL_SENT
printk("lsr = %d (jiff=%lu)...done\n", lsr, jiffies);
#endif
diff --git a/drivers/serial/crisv10.c b/drivers/serial/crisv10.c
index e6a1cb7..9047384 100644
--- a/drivers/serial/crisv10.c
+++ b/drivers/serial/crisv10.c
@@ -3254,7 +3254,9 @@ rs_write(struct tty_struct *tty,
*/
/* Sleep until all sent */
- tty_wait_until_sent(tty, 0);
+ tty_lock_nested();
+ tty_wait_until_sent_locked(tty, 0);
+ tty_unlock();
#ifdef CONFIG_ETRAX_FAST_TIMER
/* Now sleep a little more so that shift register is empty */
schedule_usleep(info->char_time_usec * 2);
@@ -3825,7 +3827,7 @@ rs_close(struct tty_struct *tty, struct file * filp)
*/
tty->closing = 1;
if (info->closing_wait != ASYNC_CLOSING_WAIT_NONE)
- tty_wait_until_sent(tty, info->closing_wait);
+ tty_wait_until_sent_locked(tty, info->closing_wait);
/*
* At this point we stop accepting input. To do this, we
* disable the serial receiver and the DMA receive interrupt.
@@ -3844,7 +3846,7 @@ rs_close(struct tty_struct *tty, struct file * filp)
* has completely drained; this is especially
* important as we have a transmit FIFO!
*/
- rs_wait_until_sent(tty, HZ);
+ __rs_wait_until_sent(tty, HZ);
}
#endif
@@ -3920,11 +3922,12 @@ static void rs_wait_until_sent(struct tty_struct *tty, int timeout)
(curr_time - info->last_tx_active) * (1000000/HZ) +
curr_time_usec - info->last_tx_active_usec;
+ WARN_ON_ONCE(!tty_locked());
+
/*
* Check R_DMA_CHx_STATUS bit 0-6=number of available bytes in FIFO
* R_DMA_CHx_HWSW bit 31-16=nbr of bytes left in DMA buffer (0=64k)
*/
- tty_lock_nested(); /* locked already when coming from close */
orig_jiffies = jiffies;
while (info->xmit.head != info->xmit.tail || /* More in send queue */
(*info->ostatusadr & 0x007f) || /* more in FIFO */
@@ -3941,7 +3944,6 @@ static void rs_wait_until_sent(struct tty_struct *tty, int timeout)
curr_time_usec - info->last_tx_active_usec;
}
set_current_state(TASK_RUNNING);
- tty_unlock();
}
/*
diff --git a/drivers/serial/serial_core.c b/drivers/serial/serial_core.c
index a9d51eb..4fb1d31 100644
--- a/drivers/serial/serial_core.c
+++ b/drivers/serial/serial_core.c
@@ -1316,7 +1316,7 @@ static void uart_close(struct tty_struct *tty, struct file *filp)
tty->closing = 1;
if (port->closing_wait != ASYNC_CLOSING_WAIT_NONE)
- tty_wait_until_sent(tty, msecs_to_jiffies(port->closing_wait));
+ tty_wait_until_sent_locked(tty, msecs_to_jiffies(port->closing_wait));
/*
* At this point, we stop accepting input. To do this, we
@@ -1369,8 +1369,6 @@ static void uart_wait_until_sent(struct tty_struct *tty, int timeout)
if (port->type == PORT_UNKNOWN || port->fifosize == 0)
return;
- tty_lock_nested(); /* already locked when coming from close */
-
/*
* 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
@@ -1416,7 +1414,6 @@ static void uart_wait_until_sent(struct tty_struct *tty, int timeout)
break;
}
set_current_state(TASK_RUNNING); /* might not be needed */
- tty_unlock();
}
/*
diff --git a/include/linux/tty.h b/include/linux/tty.h
index 1659ba8..8ddd952 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -388,6 +388,7 @@ extern void tty_kref_put(struct tty_struct *tty);
extern int tty_paranoia_check(struct tty_struct *tty, struct inode *inode,
const char *routine);
extern char *tty_name(struct tty_struct *tty, char *buf);
+extern void tty_wait_until_sent_locked(struct tty_struct *tty, long timeout);
extern void tty_wait_until_sent(struct tty_struct *tty, long timeout);
extern int tty_check_change(struct tty_struct *tty);
extern void stop_tty(struct tty_struct *tty);
diff --git a/net/irda/ircomm/ircomm_tty.c b/net/irda/ircomm/ircomm_tty.c
index faa82ca..a176b9c 100644
--- a/net/irda/ircomm/ircomm_tty.c
+++ b/net/irda/ircomm/ircomm_tty.c
@@ -550,7 +550,7 @@ static void ircomm_tty_close(struct tty_struct *tty, struct file *filp)
*/
tty->closing = 1;
if (self->closing_wait != ASYNC_CLOSING_WAIT_NONE)
- tty_wait_until_sent(tty, self->closing_wait);
+ tty_wait_until_sent_locked(tty, self->closing_wait);
ircomm_tty_shutdown(self);
--
1.7.0.4
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH 10/13] tty: untangle locking of wait_until_sent
2010-05-04 22:33 ` [PATCH 10/13] tty: untangle locking of wait_until_sent Arnd Bergmann
@ 2010-05-05 19:59 ` Alan Cox
2010-05-05 21:31 ` Arnd Bergmann
` (2 more replies)
0 siblings, 3 replies; 25+ messages in thread
From: Alan Cox @ 2010-05-05 19:59 UTC (permalink / raw)
To: Arnd Bergmann
Cc: linux-kernel, Greg KH, Frederic Weisbecker, Thomas Gleixner,
Andrew Morton, John Kacur, Al Viro, Ingo Molnar
> Some wait_until_sent versions require the big
> tty mutex, others don't and some callers of
> wait_until_sent already hold it while other don't.
> That leads to recursive use of the BTM in these
> functions, which we're trying to get rid of.
I don't believe any of the currently live ones do.
> drivers/char/amiserial.c | 6 +++---
> drivers/char/generic_serial.c | 2 +-
Both ex drivers
> drivers/char/hvc_console.c | 2 +-
> drivers/char/hvcs.c | 2 +-
Doesn't seemn to need it
> drivers/char/ip2/ip2main.c | 20 +++++++++++++++++---
I don't think we care - the driver is a mess, its probably not been used
in years
> drivers/char/serial167.c | 6 +++---
Historical value only and broken
> drivers/char/specialix.c | 2 +-
Broken
> drivers/serial/68328serial.c | 2 +-
> drivers/serial/68360serial.c | 5 ++---
Both defunct
> drivers/serial/crisv10.c | 12 +++++++-----
Defunct
> net/irda/ircomm/ircomm_tty.c | 2 +-
Semi-defunct at best
This makes me think that now might be a good time to consign the broken
crap to the bitbucket unless someone stands up with hardware and who
wants to maintain it.
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH 10/13] tty: untangle locking of wait_until_sent
2010-05-05 19:59 ` Alan Cox
@ 2010-05-05 21:31 ` Arnd Bergmann
2010-05-05 22:51 ` Greg KH
2010-05-24 19:00 ` Pavel Machek
2 siblings, 0 replies; 25+ messages in thread
From: Arnd Bergmann @ 2010-05-05 21:31 UTC (permalink / raw)
To: Alan Cox
Cc: linux-kernel, Greg KH, Frederic Weisbecker, Thomas Gleixner,
Andrew Morton, John Kacur, Al Viro, Ingo Molnar
On Wednesday 05 May 2010 21:59:05 Alan Cox wrote:
> > Some wait_until_sent versions require the big
> > tty mutex, others don't and some callers of
> > wait_until_sent already hold it while other don't.
> > That leads to recursive use of the BTM in these
> > functions, which we're trying to get rid of.
>
> I don't believe any of the currently live ones do.
Ok, that simplifies things, at least we can call
tty->ops->wait_until_sent(tty, timeout) while holding
the BTM then, even with the next patch that makes it
non-recursive.
> > drivers/char/hvc_console.c | 2 +-
> > drivers/char/hvcs.c | 2 +-
>
> Doesn't seemn to need it
These, and most of the others in this patch call tty_wait_until_sent()
from their close() function. That contains the lines
if (wait_event_interruptible_timeout(tty->write_wait,
!tty_chars_in_buffer(tty), timeout) >= 0) {
Part of what my patch does is to give up the BTM when already
holding it, to mimic the BKL behavior.
If you can confirm that this wait_event never blocks indefinitely
or has to wait for the BTM from another function, that could just
be removed. Otherwise, it probably needs to become something ugly
like
if (tty_chars_in_buffer(tty)) {
if (tty_locked()) {
tty_unlock();
wait = wait_event_interruptible_timeout(tty->write_wait,
!tty_chars_in_buffer(tty), timeout) >= 0);
tty_lock();
} else {
wait = wait_event_interruptible_timeout(tty->write_wait,
!tty_chars_in_buffer(tty), timeout) >= 0);
}
if (wait && tty->ops->wait_until_sent)
tty->ops->wait_until_sent(tty, timeout);
}
I already had to introduce a few of these constructs to make the BTM
non-recursive, but I'd prefer to keep the number as low as possible
for obvious reasons.
> > drivers/char/specialix.c | 2 +-
>
> Broken
> This makes me think that now might be a good time to consign the broken
> crap to the bitbucket unless someone stands up with hardware and who
> wants to maintain it.
Fine with me.
While I technically own a 16-port specialix card somewhere in my
parents' basement, I'm not exactly interested in maintaining the
driver.
Arnd
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH 10/13] tty: untangle locking of wait_until_sent
2010-05-05 19:59 ` Alan Cox
2010-05-05 21:31 ` Arnd Bergmann
@ 2010-05-05 22:51 ` Greg KH
2010-05-05 23:52 ` Alan Cox
2010-05-24 19:00 ` Pavel Machek
2 siblings, 1 reply; 25+ messages in thread
From: Greg KH @ 2010-05-05 22:51 UTC (permalink / raw)
To: Alan Cox
Cc: Arnd Bergmann, linux-kernel, Frederic Weisbecker, Thomas Gleixner,
Andrew Morton, John Kacur, Al Viro, Ingo Molnar
On Wed, May 05, 2010 at 08:59:05PM +0100, Alan Cox wrote:
> > Some wait_until_sent versions require the big
> > tty mutex, others don't and some callers of
> > wait_until_sent already hold it while other don't.
> > That leads to recursive use of the BTM in these
> > functions, which we're trying to get rid of.
>
> I don't believe any of the currently live ones do.
>
> > drivers/char/amiserial.c | 6 +++---
> > drivers/char/generic_serial.c | 2 +-
>
> Both ex drivers
>
> > drivers/char/hvc_console.c | 2 +-
> > drivers/char/hvcs.c | 2 +-
>
> Doesn't seemn to need it
>
> > drivers/char/ip2/ip2main.c | 20 +++++++++++++++++---
>
> I don't think we care - the driver is a mess, its probably not been used
> in years
>
> > drivers/char/serial167.c | 6 +++---
>
> Historical value only and broken
>
> > drivers/char/specialix.c | 2 +-
>
> Broken
>
> > drivers/serial/68328serial.c | 2 +-
> > drivers/serial/68360serial.c | 5 ++---
>
> Both defunct
>
> > drivers/serial/crisv10.c | 12 +++++++-----
>
> Defunct
>
> > net/irda/ircomm/ircomm_tty.c | 2 +-
>
> Semi-defunct at best
>
> This makes me think that now might be a good time to consign the broken
> crap to the bitbucket unless someone stands up with hardware and who
> wants to maintain it.
I will be glad to do this, moving these drivers to the staging tree so
that they can be removed in 6-8 months, much like some wireless drivers
are about to have happen to them.
So, which ones should I move? How about to start with the ones you
listed above:
drivers/char/amiserial.c
drivers/char/generic_serial.c
drivers/char/ip2/ip2main.c
drivers/char/serial167.c
drivers/char/specialix.c
drivers/serial/68328serial.c
drivers/serial/68360serial.c
drivers/serial/crisv10.c
Any others you want to see move out of the tree?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 10/13] tty: untangle locking of wait_until_sent
2010-05-05 22:51 ` Greg KH
@ 2010-05-05 23:52 ` Alan Cox
0 siblings, 0 replies; 25+ messages in thread
From: Alan Cox @ 2010-05-05 23:52 UTC (permalink / raw)
To: Greg KH
Cc: Arnd Bergmann, linux-kernel, Frederic Weisbecker, Thomas Gleixner,
Andrew Morton, John Kacur, Al Viro, Ingo Molnar
> So, which ones should I move? How about to start with the ones you
> listed above:
> drivers/char/amiserial.c
> drivers/char/generic_serial.c
Generic serial covers several old drivers that are defunct
drivers/char/rio/
drivers/char/vme_scc
drivers/char/ser_a2232
drivers/char/sx/
drivers/char/ip2/
> drivers/char/specialix.c
Leave specialix.c for now - that one is probably salvagable fairly easily
and I think has a couple of users.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 10/13] tty: untangle locking of wait_until_sent
2010-05-05 19:59 ` Alan Cox
2010-05-05 21:31 ` Arnd Bergmann
2010-05-05 22:51 ` Greg KH
@ 2010-05-24 19:00 ` Pavel Machek
2010-05-24 20:27 ` Alan Cox
2 siblings, 1 reply; 25+ messages in thread
From: Pavel Machek @ 2010-05-24 19:00 UTC (permalink / raw)
To: Alan Cox
Cc: Arnd Bergmann, linux-kernel, Greg KH, Frederic Weisbecker,
Thomas Gleixner, Andrew Morton, John Kacur, Al Viro, Ingo Molnar
Hi!
> > net/irda/ircomm/ircomm_tty.c | 2 +-
>
> Semi-defunct at best
>
> This makes me think that now might be a good time to consign the broken
> crap to the bitbucket unless someone stands up with hardware and who
> wants to maintain it.
ircomm_tty is fairly common hardware, afaict, it is just not used
often. Main use case would be backup of your phone numbers, afaict.
It would be good to keep it for once-in-a-blue-moon use...
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 10/13] tty: untangle locking of wait_until_sent
2010-05-24 19:00 ` Pavel Machek
@ 2010-05-24 20:27 ` Alan Cox
0 siblings, 0 replies; 25+ messages in thread
From: Alan Cox @ 2010-05-24 20:27 UTC (permalink / raw)
To: Pavel Machek
Cc: Arnd Bergmann, linux-kernel, Greg KH, Frederic Weisbecker,
Thomas Gleixner, Andrew Morton, John Kacur, Al Viro, Ingo Molnar
On Mon, 24 May 2010 21:00:38 +0200
Pavel Machek <pavel@ucw.cz> wrote:
> Hi!
>
> > > net/irda/ircomm/ircomm_tty.c | 2 +-
> >
> > Semi-defunct at best
> >
> > This makes me think that now might be a good time to consign the broken
> > crap to the bitbucket unless someone stands up with hardware and who
> > wants to maintain it.
>
> ircomm_tty is fairly common hardware, afaict, it is just not used
> often. Main use case would be backup of your phone numbers, afaict.
> It would be good to keep it for once-in-a-blue-moon use...
Fine - send patches. Last time I looked at it I couldn't believe it still
worked and the last box I had with Irda capability exploded (literally)
some years ago.
I don't I've seen an Irda capable phone for a while either.
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 11/13] tty: remove tty_lock_nested
2010-05-04 22:33 [PATCH v2 00/13] BKL conversion in tty layer Arnd Bergmann
` (9 preceding siblings ...)
2010-05-04 22:33 ` [PATCH 10/13] tty: untangle locking of wait_until_sent Arnd Bergmann
@ 2010-05-04 22:33 ` Arnd Bergmann
2010-05-04 22:33 ` [PATCH 12/13] tty: remove release_tty_lock/reacquire_tty_lock Arnd Bergmann
` (2 subsequent siblings)
13 siblings, 0 replies; 25+ messages in thread
From: Arnd Bergmann @ 2010-05-04 22:33 UTC (permalink / raw)
To: linux-kernel
Cc: Arnd Bergmann, Alan Cox, Greg KH, Frederic Weisbecker,
Thomas Gleixner, Andrew Morton, John Kacur, Al Viro, Ingo Molnar
This changes all remaining users of tty_lock_nested
to be non-recursive, which lets us kill this function.
As a consequence, we won't need to keep the lock count
any more, which allows more simplifications later.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/char/pty.c | 2 +-
drivers/char/selection.c | 4 ++--
drivers/char/tty_io.c | 41 ++++++++++++++++++++---------------------
drivers/char/tty_ldisc.c | 3 +--
drivers/char/tty_mutex.c | 15 ---------------
drivers/serial/crisv10.c | 7 ++++---
include/linux/tty.h | 17 +----------------
7 files changed, 29 insertions(+), 60 deletions(-)
diff --git a/drivers/char/pty.c b/drivers/char/pty.c
index 889de89..76de961 100644
--- a/drivers/char/pty.c
+++ b/drivers/char/pty.c
@@ -62,7 +62,7 @@ static void pty_close(struct tty_struct *tty, struct file *filp)
if (tty->driver == ptm_driver)
devpts_pty_kill(tty->link);
#endif
- tty_vhangup(tty->link);
+ tty_vhangup_locked(tty->link);
}
}
diff --git a/drivers/char/selection.c b/drivers/char/selection.c
index 85211a3..8e314bb 100644
--- a/drivers/char/selection.c
+++ b/drivers/char/selection.c
@@ -313,7 +313,8 @@ int paste_selection(struct tty_struct *tty)
struct tty_ldisc *ld;
DECLARE_WAITQUEUE(wait, current);
- tty_lock_nested(); /* always called with BTM from vt_ioctl */
+ /* always called with BTM from vt_ioctl */
+ WARN_ON(!tty_locked());
acquire_console_sem();
poke_blanked_console();
@@ -338,6 +339,5 @@ int paste_selection(struct tty_struct *tty)
__set_current_state(TASK_RUNNING);
tty_ldisc_deref(ld);
- tty_unlock();
return 0;
}
diff --git a/drivers/char/tty_io.c b/drivers/char/tty_io.c
index e1b2fca..4e16b34 100644
--- a/drivers/char/tty_io.c
+++ b/drivers/char/tty_io.c
@@ -492,10 +492,8 @@ EXPORT_SYMBOL_GPL(tty_wakeup);
* tasklist_lock to walk task list for hangup event
* ->siglock to protect ->signal/->sighand
*/
-static void do_tty_hangup(struct work_struct *work)
+void tty_vhangup_locked(struct tty_struct *tty)
{
- struct tty_struct *tty =
- container_of(work, struct tty_struct, hangup_work);
struct file *cons_filp = NULL;
struct file *filp, *f = NULL;
struct task_struct *p;
@@ -517,8 +515,6 @@ static void do_tty_hangup(struct work_struct *work)
/* inuse_filps is protected by the single tty lock,
this really needs to change if we want to flush the
workqueue with the lock held */
- tty_lock_nested(); /* called with BTM held from pty_close and
- others */
check_tty_count(tty, "do_tty_hangup");
file_list_lock();
@@ -598,11 +594,20 @@ static void do_tty_hangup(struct work_struct *work)
*/
set_bit(TTY_HUPPED, &tty->flags);
tty_ldisc_enable(tty);
- tty_unlock();
if (f)
fput(f);
}
+static void do_tty_hangup(struct work_struct *work)
+{
+ struct tty_struct *tty =
+ container_of(work, struct tty_struct, hangup_work);
+
+ tty_lock();
+ tty_vhangup_locked(tty);
+ tty_unlock();
+}
+
/**
* tty_hangup - trigger a hangup event
* @tty: tty to hangup
@@ -638,7 +643,9 @@ void tty_vhangup(struct tty_struct *tty)
printk(KERN_DEBUG "%s vhangup...\n", tty_name(tty, buf));
#endif
- do_tty_hangup(&tty->hangup_work);
+ tty_lock();
+ tty_vhangup_locked(tty);
+ tty_unlock();
}
EXPORT_SYMBOL(tty_vhangup);
@@ -719,10 +726,12 @@ void disassociate_ctty(int on_exit)
tty = get_current_tty();
if (tty) {
tty_pgrp = get_pid(tty->pgrp);
- tty_lock_nested(); /* see above */
- if (on_exit && tty->driver->type != TTY_DRIVER_TYPE_PTY)
- tty_vhangup(tty);
- tty_unlock();
+ if (on_exit) {
+ tty_lock();
+ if (tty->driver->type != TTY_DRIVER_TYPE_PTY)
+ tty_vhangup_locked(tty);
+ tty_unlock();
+ }
tty_kref_put(tty);
} else if (on_exit) {
struct pid *old_pgrp;
@@ -1218,18 +1227,14 @@ static int tty_driver_install_tty(struct tty_driver *driver,
int ret;
if (driver->ops->install) {
- tty_lock_nested(); /* already called with BTM held */
ret = driver->ops->install(driver, tty);
- tty_unlock();
return ret;
}
if (tty_init_termios(tty) == 0) {
- tty_lock_nested();
tty_driver_kref_get(driver);
tty->count++;
driver->ttys[idx] = tty;
- tty_unlock();
return 0;
}
return -ENOMEM;
@@ -1322,15 +1327,11 @@ struct tty_struct *tty_init_dev(struct tty_driver *driver, int idx,
struct tty_struct *tty;
int retval;
- tty_lock_nested(); /* always called with tty lock held already */
-
/* Check if pty master is being opened multiple times */
if (driver->subtype == PTY_TYPE_MASTER &&
(driver->flags & TTY_DRIVER_DEVPTS_MEM) && !first_ok) {
- tty_unlock();
return ERR_PTR(-EIO);
}
- tty_unlock();
/*
* First time open is complex, especially for PTY devices.
@@ -1374,9 +1375,7 @@ release_mem_out:
if (printk_ratelimit())
printk(KERN_INFO "tty_init_dev: ldisc open failed, "
"clearing slot %d\n", idx);
- tty_lock_nested();
release_tty(tty, idx);
- tty_unlock();
return ERR_PTR(retval);
}
diff --git a/drivers/char/tty_ldisc.c b/drivers/char/tty_ldisc.c
index d4259cf..c71a94a 100644
--- a/drivers/char/tty_ldisc.c
+++ b/drivers/char/tty_ldisc.c
@@ -458,9 +458,8 @@ static int tty_ldisc_open(struct tty_struct *tty, struct tty_ldisc *ld)
if (ld->ops->open) {
int ret;
/* BTM here locks versus a hangup event */
- tty_lock_nested(); /* always held here already */
+ WARN_ON(!tty_locked());
ret = ld->ops->open(tty);
- tty_unlock();
return ret;
}
return 0;
diff --git a/drivers/char/tty_mutex.c b/drivers/char/tty_mutex.c
index 51e0852..6bd47e1 100644
--- a/drivers/char/tty_mutex.c
+++ b/drivers/char/tty_mutex.c
@@ -73,21 +73,6 @@ void __lockfunc tty_lock(void)
}
EXPORT_SYMBOL(tty_lock);
-void __lockfunc tty_lock_nested(void)
-{
- struct task_struct *task = current;
- int depth = task->tty_lock_depth + 1;
-
- if (likely(!depth))
- /*
- * No recursion worries - we set up tty_lock_depth _after_
- */
- mutex_lock(&big_tty_mutex);
-
- task->tty_lock_depth = depth;
-}
-EXPORT_SYMBOL(tty_lock_nested);
-
void __lockfunc tty_unlock(void)
{
struct task_struct *task = current;
diff --git a/drivers/serial/crisv10.c b/drivers/serial/crisv10.c
index 9047384..28ff46a 100644
--- a/drivers/serial/crisv10.c
+++ b/drivers/serial/crisv10.c
@@ -3254,9 +3254,10 @@ rs_write(struct tty_struct *tty,
*/
/* Sleep until all sent */
- tty_lock_nested();
- tty_wait_until_sent_locked(tty, 0);
- tty_unlock();
+ if (tty_locked())
+ tty_wait_until_sent_locked(tty, 0);
+ else
+ tty_wait_until_sent(tty, 0);
#ifdef CONFIG_ETRAX_FAST_TIMER
/* Now sleep a little more so that shift register is empty */
schedule_usleep(info->char_time_usec * 2);
diff --git a/include/linux/tty.h b/include/linux/tty.h
index 8ddd952..3cf4556 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -416,6 +416,7 @@ extern int is_ignored(int sig);
extern int tty_signal(int sig, struct tty_struct *tty);
extern void tty_hangup(struct tty_struct *tty);
extern void tty_vhangup(struct tty_struct *tty);
+extern void tty_vhangup_locked(struct tty_struct *tty);
extern void tty_vhangup_self(void);
extern void tty_unhangup(struct file *filp);
extern int tty_hung_up_p(struct file *filp);
@@ -575,19 +576,7 @@ extern long vt_compat_ioctl(struct tty_struct *tty, struct file * file,
/* tty_mutex.c */
/* functions for preparation of BKL removal */
-
-/*
- * tty_lock_nested get the tty_lock while potentially holding it
- *
- * The Big TTY Mutex is a recursive lock, meaning you can take it
- * from a thread that is already holding it.
- * This is bad for a number of reasons, so tty_lock_nested should
- * really be used as rarely as possible. If a code location can
- * be shown to never get called with this held already, it should
- * use tty_lock() instead.
- */
#ifdef CONFIG_TTY_MUTEX
-extern void __lockfunc tty_lock_nested(void) __acquires(tty_lock);
extern void __lockfunc tty_lock(void) __acquires(tty_lock);
extern void __lockfunc tty_unlock(void) __releases(tty_lock);
#define tty_locked() (current->tty_lock_depth >= 0)
@@ -602,10 +591,6 @@ void __lockfunc __release_tty_lock(void);
__reacquire_tty_lock() : 0 )
#else
-static inline void __lockfunc tty_lock_nested(void) __acquires(kernel_lock)
-{
- lock_kernel();
-}
static inline void tty_lock(void) __acquires(kernel_lock)
{
WARN_ON(kernel_locked());
--
1.7.0.4
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH 12/13] tty: remove release_tty_lock/reacquire_tty_lock
2010-05-04 22:33 [PATCH v2 00/13] BKL conversion in tty layer Arnd Bergmann
` (10 preceding siblings ...)
2010-05-04 22:33 ` [PATCH 11/13] tty: remove tty_lock_nested Arnd Bergmann
@ 2010-05-04 22:33 ` Arnd Bergmann
2010-05-04 22:33 ` [PATCH 13/13] tty: turn ldisc_mutex into a regular mutex Arnd Bergmann
2010-05-05 10:52 ` [PATCH v2 00/13] BKL conversion in tty layer Alan Cox
13 siblings, 0 replies; 25+ messages in thread
From: Arnd Bergmann @ 2010-05-04 22:33 UTC (permalink / raw)
To: linux-kernel
Cc: Arnd Bergmann, Alan Cox, Greg KH, Frederic Weisbecker,
Thomas Gleixner, Andrew Morton, John Kacur, Al Viro, Ingo Molnar
All users are nonrecursive now, and we don't call release_tty_lock()
in places where it's not held to start with, so it is safe to
replace it with tty_lock().
Same for reacquire_tty_lock()/tty_unlock().
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/char/tty_ldisc.c | 8 +++---
drivers/char/tty_mutex.c | 35 ------------------------
include/linux/tty.h | 65 ++++++++++++---------------------------------
kernel/printk.c | 9 ++++--
4 files changed, 28 insertions(+), 89 deletions(-)
diff --git a/drivers/char/tty_ldisc.c b/drivers/char/tty_ldisc.c
index c71a94a..0c0e935 100644
--- a/drivers/char/tty_ldisc.c
+++ b/drivers/char/tty_ldisc.c
@@ -537,9 +537,9 @@ static int tty_ldisc_halt(struct tty_struct *tty)
int ret;
clear_bit(TTY_LDISC, &tty->flags);
if (tty_locked()) {
- __release_tty_lock();
+ tty_unlock();
ret = cancel_delayed_work_sync(&tty->buf.work);
- __reacquire_tty_lock();
+ tty_lock();
} else {
ret = cancel_delayed_work_sync(&tty->buf.work);
@@ -870,9 +870,9 @@ void tty_ldisc_release(struct tty_struct *tty, struct tty_struct *o_tty)
*/
tty_ldisc_halt(tty);
- release_tty_lock(current);
+ tty_unlock();
flush_scheduled_work();
- reacquire_tty_lock(current);
+ tty_lock();
mutex_lock_tty_on(&tty->ldisc_mutex);
/*
* Now kill off the ldisc
diff --git a/drivers/char/tty_mutex.c b/drivers/char/tty_mutex.c
index 6bd47e1..5fd3332 100644
--- a/drivers/char/tty_mutex.c
+++ b/drivers/char/tty_mutex.c
@@ -24,41 +24,6 @@
static DEFINE_MUTEX(big_tty_mutex);
/*
- * Re-acquire the kernel semaphore.
- *
- * This function is called with preemption off.
- *
- * We are executing in schedule() so the code must be extremely careful
- * about recursion, both due to the down() and due to the enabling of
- * preemption. schedule() will re-check the preemption flag after
- * reacquiring the semaphore.
- */
-int __lockfunc __reacquire_tty_lock(void)
-{
- struct task_struct *task = current;
- int saved_tty_lock_depth = task->tty_lock_depth;
-
- BUG_ON(saved_tty_lock_depth < 0);
-
- task->tty_lock_depth = -1;
- preempt_enable_no_resched();
-
- mutex_lock(&big_tty_mutex);
-
- preempt_disable();
- task->tty_lock_depth = saved_tty_lock_depth;
-
- return 0;
-}
-EXPORT_SYMBOL(__reacquire_tty_lock);
-
-void __lockfunc __release_tty_lock(void)
-{
- mutex_unlock(&big_tty_mutex);
-}
-EXPORT_SYMBOL(__release_tty_lock);
-
-/*
* Getting the big tty mutex.
*/
void __lockfunc tty_lock(void)
diff --git a/include/linux/tty.h b/include/linux/tty.h
index 3cf4556..9b76f8b 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -580,16 +580,6 @@ extern long vt_compat_ioctl(struct tty_struct *tty, struct file * file,
extern void __lockfunc tty_lock(void) __acquires(tty_lock);
extern void __lockfunc tty_unlock(void) __releases(tty_lock);
#define tty_locked() (current->tty_lock_depth >= 0)
-int __lockfunc __reacquire_tty_lock(void);
-void __lockfunc __release_tty_lock(void);
-#define release_tty_lock(tsk) do { \
- if (unlikely((tsk)->tty_lock_depth >= 0)) \
- __release_tty_lock(); \
-} while (0)
-#define reacquire_tty_lock(tsk) \
- ((tsk->tty_lock_depth >= 0) ? \
- __reacquire_tty_lock() : 0 )
-
#else
static inline void tty_lock(void) __acquires(kernel_lock)
{
@@ -601,17 +591,6 @@ static inline void tty_unlock(void) __releases(kernel_lock)
unlock_kernel();
}
#define tty_locked() (kernel_locked())
-static inline int __reacquire_tty_lock(void)
-{
- return 0;
-}
-static inline void __release_tty_lock(void)
-{
-}
-
-#define release_tty_lock(tsk) do { } while (0)
-#define reacquire_tty_lock(tsk) do { } while (0)
-
#endif
/*
@@ -642,9 +621,9 @@ static inline void __release_tty_lock(void)
({ \
if (!mutex_trylock(mutex)) { \
if (tty_locked()) { \
- __release_tty_lock(); \
+ tty_unlock(); \
mutex_lock(mutex); \
- __reacquire_tty_lock(); \
+ tty_lock(); \
} else \
mutex_lock(mutex); \
} \
@@ -652,26 +631,18 @@ static inline void __release_tty_lock(void)
#define mutex_lock_tty_on(mutex) \
({ \
+ WARN_ON(!tty_locked()); \
if (!mutex_trylock(mutex)) { \
- if (!WARN_ON(!tty_locked())) { \
- __release_tty_lock(); \
- mutex_lock(mutex); \
- __reacquire_tty_lock(); \
- } else \
- mutex_lock(mutex); \
+ tty_unlock(); \
+ mutex_lock(mutex); \
+ tty_lock(); \
} \
})
#define mutex_lock_tty_off(mutex) \
({ \
- if (!mutex_trylock(mutex)) { \
- if (WARN_ON(tty_locked())) { \
- __release_tty_lock(); \
- mutex_lock(mutex); \
- __reacquire_tty_lock(); \
- } else \
- mutex_lock(mutex); \
- } \
+ WARN_ON(tty_locked()); \
+ mutex_lock(mutex); \
})
#define mutex_lock_interruptible_tty(mutex) \
@@ -679,9 +650,9 @@ static inline void __release_tty_lock(void)
int __ret = 0; \
if (!mutex_trylock(mutex)) { \
if (tty_locked()) { \
- __release_tty_lock(); \
+ tty_unlock(); \
__ret = mutex_lock_interruptible(mutex); \
- __reacquire_tty_lock(); \
+ tty_lock(); \
} else \
__ret = mutex_lock_interruptible(mutex); \
} \
@@ -707,18 +678,18 @@ static inline void __release_tty_lock(void)
do { \
if (condition) \
break; \
- release_tty_lock(current); \
+ tty_unlock(); \
__wait_event(wq, condition); \
- reacquire_tty_lock(current); \
+ tty_lock(); \
} while (0)
#define wait_event_timeout_tty(wq, condition, timeout) \
({ \
long __ret = timeout; \
if (!(condition)) { \
- release_tty_lock(current); \
+ tty_unlock(); \
__wait_event_timeout(wq, condition, __ret); \
- reacquire_tty_lock(current); \
+ tty_lock(); \
} \
__ret; \
})
@@ -727,9 +698,9 @@ do { \
({ \
int __ret = 0; \
if (!(condition)) { \
- release_tty_lock(current); \
+ tty_unlock(); \
__wait_event_interruptible(wq, condition, __ret); \
- reacquire_tty_lock(current); \
+ tty_lock(); \
} \
__ret; \
})
@@ -738,9 +709,9 @@ do { \
({ \
long __ret = timeout; \
if (!(condition)) { \
- release_tty_lock(current); \
+ tty_unlock(); \
__wait_event_interruptible_timeout(wq, condition, __ret); \
- reacquire_tty_lock(current); \
+ tty_lock(); \
} \
__ret; \
})
diff --git a/kernel/printk.c b/kernel/printk.c
index 30669a3..b9fb322 100644
--- a/kernel/printk.c
+++ b/kernel/printk.c
@@ -976,9 +976,12 @@ void acquire_console_sem(void)
* it's not clear whether we get the BTM while
* holding console_sem, need to check.
*/
- release_tty_lock(current);
- down(&console_sem);
- reacquire_tty_lock(current);
+ if (tty_locked()) {
+ tty_unlock();
+ down(&console_sem);
+ tty_lock();
+ } else
+ down(&console_sem);
}
if (console_suspended)
return;
--
1.7.0.4
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH 13/13] tty: turn ldisc_mutex into a regular mutex
2010-05-04 22:33 [PATCH v2 00/13] BKL conversion in tty layer Arnd Bergmann
` (11 preceding siblings ...)
2010-05-04 22:33 ` [PATCH 12/13] tty: remove release_tty_lock/reacquire_tty_lock Arnd Bergmann
@ 2010-05-04 22:33 ` Arnd Bergmann
2010-05-05 9:18 ` Arnd Bergmann
2010-05-05 10:52 ` [PATCH v2 00/13] BKL conversion in tty layer Alan Cox
13 siblings, 1 reply; 25+ messages in thread
From: Arnd Bergmann @ 2010-05-04 22:33 UTC (permalink / raw)
To: linux-kernel
Cc: Arnd Bergmann, Alan Cox, Greg KH, Frederic Weisbecker,
Thomas Gleixner, Andrew Morton, John Kacur, Al Viro, Ingo Molnar
This fixes the lock order of the BTM and
ldisc_mutex so that we never take the BTM
while holding ldisc_mutex, which means
we no longer have to use mutex_lock_tty
when getting ldisc_mutex.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/char/tty_io.c | 2 +-
drivers/char/tty_ldisc.c | 15 ++++++++-------
2 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/drivers/char/tty_io.c b/drivers/char/tty_io.c
index 4e16b34..80178cc 100644
--- a/drivers/char/tty_io.c
+++ b/drivers/char/tty_io.c
@@ -1289,7 +1289,7 @@ static int tty_reopen(struct tty_struct *tty)
tty->count++;
tty->driver = driver; /* N.B. why do this every time?? */
- mutex_lock_tty_on(&tty->ldisc_mutex);
+ mutex_lock(&tty->ldisc_mutex);
WARN_ON(!test_bit(TTY_LDISC, &tty->flags));
mutex_unlock(&tty->ldisc_mutex);
diff --git a/drivers/char/tty_ldisc.c b/drivers/char/tty_ldisc.c
index 0c0e935..dda31b0 100644
--- a/drivers/char/tty_ldisc.c
+++ b/drivers/char/tty_ldisc.c
@@ -598,7 +598,8 @@ int tty_set_ldisc(struct tty_struct *tty, int ldisc)
tty_wait_until_sent(tty, 0);
- mutex_lock_tty_off(&tty->ldisc_mutex);
+ tty_lock();
+ mutex_lock(&tty->ldisc_mutex);
/*
* We could be midstream of another ldisc change which has
@@ -607,13 +608,13 @@ int tty_set_ldisc(struct tty_struct *tty, int ldisc)
while (test_bit(TTY_LDISC_CHANGING, &tty->flags)) {
mutex_unlock(&tty->ldisc_mutex);
+ tty_unlock();
wait_event(tty_ldisc_wait,
test_bit(TTY_LDISC_CHANGING, &tty->flags) == 0);
- mutex_lock_tty_off(&tty->ldisc_mutex);
+ tty_lock();
+ mutex_lock(&tty->ldisc_mutex);
}
- tty_lock();
-
set_bit(TTY_LDISC_CHANGING, &tty->flags);
/*
@@ -650,8 +651,8 @@ int tty_set_ldisc(struct tty_struct *tty, int ldisc)
flush_scheduled_work();
- mutex_lock_tty_off(&tty->ldisc_mutex);
tty_lock();
+ mutex_lock(&tty->ldisc_mutex);
if (test_bit(TTY_HUPPED, &tty->flags)) {
/* We were raced by the hangup method. It will have stomped
the ldisc data and closed the ldisc down */
@@ -797,7 +798,7 @@ void tty_ldisc_hangup(struct tty_struct *tty)
*
* Avoid racing set_ldisc or tty_ldisc_release
*/
- mutex_lock_tty_on(&tty->ldisc_mutex);
+ mutex_lock(&tty->ldisc_mutex);
tty_ldisc_halt(tty);
/* At this point we have a closed ldisc and we want to
reopen it. We could defer this to the next open but
@@ -873,7 +874,7 @@ void tty_ldisc_release(struct tty_struct *tty, struct tty_struct *o_tty)
tty_unlock();
flush_scheduled_work();
tty_lock();
- mutex_lock_tty_on(&tty->ldisc_mutex);
+ mutex_lock(&tty->ldisc_mutex);
/*
* Now kill off the ldisc
*/
--
1.7.0.4
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH 13/13] tty: turn ldisc_mutex into a regular mutex
2010-05-04 22:33 ` [PATCH 13/13] tty: turn ldisc_mutex into a regular mutex Arnd Bergmann
@ 2010-05-05 9:18 ` Arnd Bergmann
0 siblings, 0 replies; 25+ messages in thread
From: Arnd Bergmann @ 2010-05-05 9:18 UTC (permalink / raw)
To: linux-kernel
Cc: Alan Cox, Greg KH, Frederic Weisbecker, Thomas Gleixner,
Andrew Morton, John Kacur, Al Viro, Ingo Molnar
On Wednesday 05 May 2010, Arnd Bergmann wrote:
> This fixes the lock order of the BTM and
> ldisc_mutex so that we never take the BTM
> while holding ldisc_mutex, which means
> we no longer have to use mutex_lock_tty
> when getting ldisc_mutex.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Ignore this one, I just realized that we're releasing
and reacquiring the BTM in tty_ldisc_halt,
tty_set_termios_ldisc and possibly others.
This patch needs more work to make that possible.
Arnd
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 00/13] BKL conversion in tty layer
2010-05-04 22:33 [PATCH v2 00/13] BKL conversion in tty layer Arnd Bergmann
` (12 preceding siblings ...)
2010-05-04 22:33 ` [PATCH 13/13] tty: turn ldisc_mutex into a regular mutex Arnd Bergmann
@ 2010-05-05 10:52 ` Alan Cox
2010-05-05 12:24 ` Arnd Bergmann
13 siblings, 1 reply; 25+ messages in thread
From: Alan Cox @ 2010-05-05 10:52 UTC (permalink / raw)
To: Arnd Bergmann
Cc: linux-kernel, Greg KH, Frederic Weisbecker, Thomas Gleixner,
Andrew Morton, John Kacur, Al Viro, Ingo Molnar
On Wed, 5 May 2010 00:33:39 +0200
Arnd Bergmann <arnd@arndb.de> wrote:
> This is the second attempt to get the BKL out of the
> TTY code. I've updated the patches to be based on top
> of Alan's series and improved a number of things.
I've updated the patches I first sent out with the fixes for the problems
you found and send out a new set.
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH v2 00/13] BKL conversion in tty layer
2010-05-05 10:52 ` [PATCH v2 00/13] BKL conversion in tty layer Alan Cox
@ 2010-05-05 12:24 ` Arnd Bergmann
0 siblings, 0 replies; 25+ messages in thread
From: Arnd Bergmann @ 2010-05-05 12:24 UTC (permalink / raw)
To: Alan Cox
Cc: linux-kernel, Greg KH, Frederic Weisbecker, Thomas Gleixner,
Andrew Morton, John Kacur, Al Viro, Ingo Molnar
On Wednesday 05 May 2010, Alan Cox wrote:
> On Wed, 5 May 2010 00:33:39 +0200
> Arnd Bergmann <arnd@arndb.de> wrote:
>
> > This is the second attempt to get the BKL out of the
> > TTY code. I've updated the patches to be based on top
> > of Alan's series and improved a number of things.
>
> I've updated the patches I first sent out with the fixes for the problems
> you found and send out a new set.
Ok, I've rebased my patches on top of those now and pushed to
git.kernel.org:/pub/scm/linux/kernel/git/arnd/playground.git#bkl/tty,
minus my patch 13/13, which I already mentioned was broken.
Arnd
^ permalink raw reply [flat|nested] 25+ messages in thread