From 36697529b5bbe36911e39a6309e7a7c9250d280a Mon Sep 17 00:00:00 2001 From: Peter Hurley Date: Sat, 15 Jun 2013 07:04:48 -0400 Subject: tty: Replace ldisc locking with ldisc_sem Line discipline locking was performed with a combination of a mutex, a status bit, a count, and a waitqueue -- basically, a rw semaphore. Replace the existing combination with an ld_semaphore. Fixes: 1) the 'reference acquire after ldisc locked' bug 2) the over-complicated halt mechanism 3) lock order wrt. tty_lock() 4) dropping locks while changing ldisc 5) previously unidentified deadlock while locking ldisc from both linked ttys concurrently 6) previously unidentified recursive deadlocks Adds much-needed lockdep diagnostics. Signed-off-by: Peter Hurley Signed-off-by: Greg Kroah-Hartman --- include/linux/tty.h | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'include/linux/tty.h') diff --git a/include/linux/tty.h b/include/linux/tty.h index 01ac30efd6a..7269daf7632 100644 --- a/include/linux/tty.h +++ b/include/linux/tty.h @@ -238,7 +238,7 @@ struct tty_struct { int index; /* Protects ldisc changes: Lock tty not pty */ - struct mutex ldisc_mutex; + struct ld_semaphore ldisc_sem; struct tty_ldisc *ldisc; struct mutex atomic_write_lock; @@ -305,8 +305,6 @@ struct tty_file_private { #define TTY_DO_WRITE_WAKEUP 5 /* Call write_wakeup after queuing new */ #define TTY_PUSH 6 /* n_tty private */ #define TTY_CLOSING 7 /* ->close() in progress */ -#define TTY_LDISC 9 /* Line discipline attached */ -#define TTY_LDISC_CHANGING 10 /* Line discipline changing */ #define TTY_LDISC_OPEN 11 /* Line discipline is open */ #define TTY_PTY_LOCK 16 /* pty private */ #define TTY_NO_WRITE_SPLIT 17 /* Preserve write boundaries to driver */ -- cgit v1.2.3-70-g09d2 From 24a89d1cb69b6c488cf16d98dd02e7820f62b40c Mon Sep 17 00:00:00 2001 From: Peter Hurley Date: Sat, 15 Jun 2013 09:14:15 -0400 Subject: tty: Make ldisc input flow control concurrency-friendly Although line discipline receiving is single-producer/single-consumer, using tty->receive_room to manage flow control creates unnecessary critical regions requiring additional lock use. Instead, introduce the optional .receive_buf2() ldisc method which returns the # of bytes actually received. Serialization is guaranteed by the caller. In turn, the line discipline should schedule the buffer work item whenever space becomes available; ie., when there is room to receive data and receive_room() previously returned 0 (the buffer work item stops processing if receive_buf2() returns 0). Note the 'no room' state need not be atomic despite concurrent use by two threads because only the buffer work thread can set the state and only the read() thread can clear the state. Add n_tty_receive_buf2() as the receive_buf2() method for N_TTY. Provide a public helper function, tty_ldisc_receive_buf(), to use when directly accessing the receive_buf() methods. Line disciplines not using input flow control can continue to set tty->receive_room to a fixed value and only provide the receive_buf() method. Signed-off-by: Peter Hurley Signed-off-by: Greg Kroah-Hartman --- drivers/tty/n_tty.c | 72 +++++++++++++++++++++++++++++----------------- drivers/tty/tty_buffer.c | 13 ++++++--- drivers/tty/vt/selection.c | 4 +-- include/linux/tty.h | 13 +++++++++ include/linux/tty_ldisc.h | 13 +++++++++ 5 files changed, 82 insertions(+), 33 deletions(-) (limited to 'include/linux/tty.h') diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c index 4bf0fc0843d..eddeb7889e6 100644 --- a/drivers/tty/n_tty.c +++ b/drivers/tty/n_tty.c @@ -79,6 +79,9 @@ struct n_tty_data { unsigned long overrun_time; int num_overrun; + /* non-atomic */ + bool no_room; + unsigned char lnext:1, erasing:1, raw:1, real_raw:1, icanon:1; unsigned char echo_overrun:1; @@ -114,25 +117,10 @@ static inline int tty_put_user(struct tty_struct *tty, unsigned char x, return put_user(x, ptr); } -/** - * n_tty_set_room - receive space - * @tty: terminal - * - * Updates tty->receive_room to reflect the currently available space - * in the input buffer, and re-schedules the flip buffer work if space - * just became available. - * - * Locks: Concurrent update is protected with read_lock - */ - -static int set_room(struct tty_struct *tty) +static int receive_room(struct tty_struct *tty) { struct n_tty_data *ldata = tty->disc_data; int left; - int old_left; - unsigned long flags; - - raw_spin_lock_irqsave(&ldata->read_lock, flags); if (I_PARMRK(tty)) { /* Multiply read_cnt by 3, since each byte might take up to @@ -150,18 +138,27 @@ static int set_room(struct tty_struct *tty) */ if (left <= 0) left = ldata->icanon && !ldata->canon_data; - old_left = tty->receive_room; - tty->receive_room = left; - raw_spin_unlock_irqrestore(&ldata->read_lock, flags); - - return left && !old_left; + return left; } +/** + * n_tty_set_room - receive space + * @tty: terminal + * + * Re-schedules the flip buffer work if space just became available. + * + * Locks: Concurrent update is protected with read_lock + */ + static void n_tty_set_room(struct tty_struct *tty) { + struct n_tty_data *ldata = tty->disc_data; + /* Did this open up the receive buffer? We may need to flip */ - if (set_room(tty)) { + if (unlikely(ldata->no_room) && receive_room(tty)) { + ldata->no_room = 0; + WARN_RATELIMIT(tty->port->itty == NULL, "scheduling with invalid itty\n"); /* see if ldisc has been killed - if so, this means that @@ -1408,8 +1405,8 @@ static void n_tty_write_wakeup(struct tty_struct *tty) * calls one at a time and in order (or using flush_to_ldisc) */ -static void n_tty_receive_buf(struct tty_struct *tty, const unsigned char *cp, - char *fp, int count) +static void __receive_buf(struct tty_struct *tty, const unsigned char *cp, + char *fp, int count) { struct n_tty_data *ldata = tty->disc_data; const unsigned char *p; @@ -1464,8 +1461,6 @@ static void n_tty_receive_buf(struct tty_struct *tty, const unsigned char *cp, tty->ops->flush_chars(tty); } - set_room(tty); - if ((!ldata->icanon && (ldata->read_cnt >= ldata->minimum_to_wake)) || L_EXTPROC(tty)) { kill_fasync(&tty->fasync, SIGIO, POLL_IN); @@ -1480,7 +1475,7 @@ static void n_tty_receive_buf(struct tty_struct *tty, const unsigned char *cp, */ while (1) { tty_set_flow_change(tty, TTY_THROTTLE_SAFE); - if (tty->receive_room >= TTY_THRESHOLD_THROTTLE) + if (receive_room(tty) >= TTY_THRESHOLD_THROTTLE) break; if (!tty_throttle_safe(tty)) break; @@ -1488,6 +1483,28 @@ static void n_tty_receive_buf(struct tty_struct *tty, const unsigned char *cp, __tty_set_flow_change(tty, 0); } +static void n_tty_receive_buf(struct tty_struct *tty, const unsigned char *cp, + char *fp, int count) +{ + __receive_buf(tty, cp, fp, count); +} + +static int n_tty_receive_buf2(struct tty_struct *tty, const unsigned char *cp, + char *fp, int count) +{ + struct n_tty_data *ldata = tty->disc_data; + int room; + + tty->receive_room = room = receive_room(tty); + if (!room) + ldata->no_room = 1; + count = min(count, room); + if (count) + __receive_buf(tty, cp, fp, count); + + return count; +} + int is_ignored(int sig) { return (sigismember(¤t->blocked, sig) || @@ -2203,6 +2220,7 @@ struct tty_ldisc_ops tty_ldisc_N_TTY = { .receive_buf = n_tty_receive_buf, .write_wakeup = n_tty_write_wakeup, .fasync = n_tty_fasync, + .receive_buf2 = n_tty_receive_buf2, }; /** diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c index 6c7a1d043c7..ff1b2e37c3c 100644 --- a/drivers/tty/tty_buffer.c +++ b/drivers/tty/tty_buffer.c @@ -407,11 +407,16 @@ static int receive_buf(struct tty_struct *tty, struct tty_buffer *head, int count) { struct tty_ldisc *disc = tty->ldisc; + char *p = head->char_buf_ptr + head->read; + unsigned char *f = head->flag_buf_ptr + head->read; - count = min_t(int, count, tty->receive_room); - if (count) - disc->ops->receive_buf(tty, head->char_buf_ptr + head->read, - head->flag_buf_ptr + head->read, count); + if (disc->ops->receive_buf2) + count = disc->ops->receive_buf2(tty, p, f, count); + else { + count = min_t(int, count, tty->receive_room); + if (count) + disc->ops->receive_buf(tty, p, f, count); + } head->read += count; return count; } diff --git a/drivers/tty/vt/selection.c b/drivers/tty/vt/selection.c index 60b7b692605..2ca8d6b6514 100644 --- a/drivers/tty/vt/selection.c +++ b/drivers/tty/vt/selection.c @@ -356,8 +356,8 @@ int paste_selection(struct tty_struct *tty) continue; } count = sel_buffer_lth - pasted; - count = min(count, tty->receive_room); - ld->ops->receive_buf(tty, sel_buffer + pasted, NULL, count); + count = tty_ldisc_receive_buf(ld, sel_buffer + pasted, NULL, + count); pasted += count; } remove_wait_queue(&vc->paste_wait, &wait); diff --git a/include/linux/tty.h b/include/linux/tty.h index 7269daf7632..8323ee4f95b 100644 --- a/include/linux/tty.h +++ b/include/linux/tty.h @@ -557,6 +557,19 @@ extern void tty_ldisc_init(struct tty_struct *tty); extern void tty_ldisc_deinit(struct tty_struct *tty); extern void tty_ldisc_begin(void); +static inline int tty_ldisc_receive_buf(struct tty_ldisc *ld, unsigned char *p, + char *f, int count) +{ + if (ld->ops->receive_buf2) + count = ld->ops->receive_buf2(ld->tty, p, f, count); + else { + count = min_t(int, count, ld->tty->receive_room); + if (count) + ld->ops->receive_buf(ld->tty, p, f, count); + } + return count; +} + /* n_tty.c */ extern struct tty_ldisc_ops tty_ldisc_N_TTY; diff --git a/include/linux/tty_ldisc.h b/include/linux/tty_ldisc.h index 23bdd9debb8..f15c898ff46 100644 --- a/include/linux/tty_ldisc.h +++ b/include/linux/tty_ldisc.h @@ -109,6 +109,17 @@ * * Tells the discipline that the DCD pin has changed its status. * Used exclusively by the N_PPS (Pulse-Per-Second) line discipline. + * + * int (*receive_buf2)(struct tty_struct *, const unsigned char *cp, + * char *fp, int count); + * + * This function is called by the low-level tty driver to send + * characters received by the hardware to the line discpline for + * processing. is a pointer to the buffer of input + * character received by the device. is a pointer to a + * pointer of flag bytes which indicate whether a character was + * received with a parity error, etc. + * If assigned, prefer this function for automatic flow control. */ #include @@ -195,6 +206,8 @@ struct tty_ldisc_ops { void (*write_wakeup)(struct tty_struct *); void (*dcd_change)(struct tty_struct *, unsigned int); void (*fasync)(struct tty_struct *tty, int on); + int (*receive_buf2)(struct tty_struct *, const unsigned char *cp, + char *fp, int count); struct module *owner; -- cgit v1.2.3-70-g09d2 From 6a1c0680cf3ba94356ecd58833e1540c93472a57 Mon Sep 17 00:00:00 2001 From: Peter Hurley Date: Sat, 15 Jun 2013 09:14:23 -0400 Subject: tty: Convert termios_mutex to termios_rwsem termios is commonly accessed unsafely (especially by N_TTY) because the existing mutex forces exclusive access. Convert existing usage. Signed-off-by: Peter Hurley Signed-off-by: Greg Kroah-Hartman --- drivers/net/irda/irtty-sir.c | 8 ++-- drivers/tty/n_tty.c | 2 +- drivers/tty/pty.c | 4 +- drivers/tty/tty_io.c | 14 +++---- drivers/tty/tty_ioctl.c | 90 ++++++++++++++++++++++---------------------- drivers/tty/tty_ldisc.c | 10 ++--- drivers/tty/vt/vt.c | 4 +- include/linux/tty.h | 7 ++-- 8 files changed, 70 insertions(+), 69 deletions(-) (limited to 'include/linux/tty.h') diff --git a/drivers/net/irda/irtty-sir.c b/drivers/net/irda/irtty-sir.c index a4126719783..177441afeb9 100644 --- a/drivers/net/irda/irtty-sir.c +++ b/drivers/net/irda/irtty-sir.c @@ -123,14 +123,14 @@ static int irtty_change_speed(struct sir_dev *dev, unsigned speed) tty = priv->tty; - mutex_lock(&tty->termios_mutex); + down_write(&tty->termios_rwsem); old_termios = tty->termios; cflag = tty->termios.c_cflag; tty_encode_baud_rate(tty, speed, speed); if (tty->ops->set_termios) tty->ops->set_termios(tty, &old_termios); priv->io.speed = speed; - mutex_unlock(&tty->termios_mutex); + up_write(&tty->termios_rwsem); return 0; } @@ -280,7 +280,7 @@ static inline void irtty_stop_receiver(struct tty_struct *tty, int stop) struct ktermios old_termios; int cflag; - mutex_lock(&tty->termios_mutex); + down_write(&tty->termios_rwsem); old_termios = tty->termios; cflag = tty->termios.c_cflag; @@ -292,7 +292,7 @@ static inline void irtty_stop_receiver(struct tty_struct *tty, int stop) tty->termios.c_cflag = cflag; if (tty->ops->set_termios) tty->ops->set_termios(tty, &old_termios); - mutex_unlock(&tty->termios_mutex); + up_write(&tty->termios_rwsem); } /*****************************************************************/ diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c index d159059560e..ab923bb2b26 100644 --- a/drivers/tty/n_tty.c +++ b/drivers/tty/n_tty.c @@ -1539,7 +1539,7 @@ int is_ignored(int sig) * guaranteed that this function will not be re-entered or in progress * when the ldisc is closed. * - * Locking: Caller holds tty->termios_mutex + * Locking: Caller holds tty->termios_rwsem */ static void n_tty_set_termios(struct tty_struct *tty, struct ktermios *old) diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c index abfd9908978..1b39dd639ee 100644 --- a/drivers/tty/pty.c +++ b/drivers/tty/pty.c @@ -287,7 +287,7 @@ static 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); + down_write(&tty->termios_rwsem); if (!memcmp(ws, &tty->winsize, sizeof(*ws))) goto done; @@ -314,7 +314,7 @@ static int pty_resize(struct tty_struct *tty, struct winsize *ws) tty->winsize = *ws; pty->winsize = *ws; /* Never used so will go away soon */ done: - mutex_unlock(&tty->termios_mutex); + up_write(&tty->termios_rwsem); return 0; } diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c index 0fa5db4c7a7..639e24ade9b 100644 --- a/drivers/tty/tty_io.c +++ b/drivers/tty/tty_io.c @@ -604,7 +604,7 @@ static int tty_signal_session_leader(struct tty_struct *tty, int exit_session) * redirect lock for undoing redirection * file list lock for manipulating list of ttys * tty_ldiscs_lock from called functions - * termios_mutex resetting termios data + * termios_rwsem resetting termios data * tasklist_lock to walk task list for hangup event * ->siglock to protect ->signal/->sighand */ @@ -2230,7 +2230,7 @@ static int tiocsti(struct tty_struct *tty, char __user *p) * * Copies the kernel idea of the window size into the user buffer. * - * Locking: tty->termios_mutex is taken to ensure the winsize data + * Locking: tty->termios_rwsem is taken to ensure the winsize data * is consistent. */ @@ -2238,9 +2238,9 @@ static int tiocgwinsz(struct tty_struct *tty, struct winsize __user *arg) { int err; - mutex_lock(&tty->termios_mutex); + down_read(&tty->termios_rwsem); err = copy_to_user(arg, &tty->winsize, sizeof(*arg)); - mutex_unlock(&tty->termios_mutex); + up_read(&tty->termios_rwsem); return err ? -EFAULT: 0; } @@ -2261,7 +2261,7 @@ int tty_do_resize(struct tty_struct *tty, struct winsize *ws) unsigned long flags; /* Lock the tty */ - mutex_lock(&tty->termios_mutex); + down_write(&tty->termios_rwsem); if (!memcmp(ws, &tty->winsize, sizeof(*ws))) goto done; /* Get the PID values and reference them so we can @@ -2276,7 +2276,7 @@ int tty_do_resize(struct tty_struct *tty, struct winsize *ws) tty->winsize = *ws; done: - mutex_unlock(&tty->termios_mutex); + up_write(&tty->termios_rwsem); return 0; } EXPORT_SYMBOL(tty_do_resize); @@ -3015,7 +3015,7 @@ void initialize_tty_struct(struct tty_struct *tty, tty->session = NULL; tty->pgrp = NULL; mutex_init(&tty->legacy_mutex); - mutex_init(&tty->termios_mutex); + init_rwsem(&tty->termios_rwsem); init_ldsem(&tty->ldisc_sem); init_waitqueue_head(&tty->write_wait); init_waitqueue_head(&tty->read_wait); diff --git a/drivers/tty/tty_ioctl.c b/drivers/tty/tty_ioctl.c index 3500d411414..9ce20df8a2c 100644 --- a/drivers/tty/tty_ioctl.c +++ b/drivers/tty/tty_ioctl.c @@ -94,20 +94,20 @@ EXPORT_SYMBOL(tty_driver_flush_buffer); * @tty: terminal * * Indicate that a tty should stop transmitting data down the stack. - * Takes the termios mutex to protect against parallel throttle/unthrottle + * Takes the termios rwsem 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. */ void tty_throttle(struct tty_struct *tty) { - mutex_lock(&tty->termios_mutex); + down_write(&tty->termios_rwsem); /* check TTY_THROTTLED first so it indicates our state */ if (!test_and_set_bit(TTY_THROTTLED, &tty->flags) && tty->ops->throttle) tty->ops->throttle(tty); tty->flow_change = 0; - mutex_unlock(&tty->termios_mutex); + up_write(&tty->termios_rwsem); } EXPORT_SYMBOL(tty_throttle); @@ -116,7 +116,7 @@ EXPORT_SYMBOL(tty_throttle); * @tty: terminal * * Indicate that a tty may continue transmitting data down the stack. - * Takes the termios mutex to protect against parallel throttle/unthrottle + * Takes the termios rwsem 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. * @@ -126,12 +126,12 @@ EXPORT_SYMBOL(tty_throttle); void tty_unthrottle(struct tty_struct *tty) { - mutex_lock(&tty->termios_mutex); + down_write(&tty->termios_rwsem); if (test_and_clear_bit(TTY_THROTTLED, &tty->flags) && tty->ops->unthrottle) tty->ops->unthrottle(tty); tty->flow_change = 0; - mutex_unlock(&tty->termios_mutex); + up_write(&tty->termios_rwsem); } EXPORT_SYMBOL(tty_unthrottle); @@ -151,7 +151,7 @@ int tty_throttle_safe(struct tty_struct *tty) { int ret = 0; - mutex_lock(&tty->termios_mutex); + down_write(&tty->termios_rwsem); if (!test_bit(TTY_THROTTLED, &tty->flags)) { if (tty->flow_change != TTY_THROTTLE_SAFE) ret = 1; @@ -161,7 +161,7 @@ int tty_throttle_safe(struct tty_struct *tty) tty->ops->throttle(tty); } } - mutex_unlock(&tty->termios_mutex); + up_write(&tty->termios_rwsem); return ret; } @@ -182,7 +182,7 @@ int tty_unthrottle_safe(struct tty_struct *tty) { int ret = 0; - mutex_lock(&tty->termios_mutex); + down_write(&tty->termios_rwsem); if (test_bit(TTY_THROTTLED, &tty->flags)) { if (tty->flow_change != TTY_UNTHROTTLE_SAFE) ret = 1; @@ -192,7 +192,7 @@ int tty_unthrottle_safe(struct tty_struct *tty) tty->ops->unthrottle(tty); } } - mutex_unlock(&tty->termios_mutex); + up_write(&tty->termios_rwsem); return ret; } @@ -468,7 +468,7 @@ EXPORT_SYMBOL_GPL(tty_termios_encode_baud_rate); * @obad: output baud rate * * Update the current termios data for the tty with the new speed - * settings. The caller must hold the termios_mutex for the tty in + * settings. The caller must hold the termios_rwsem for the tty in * question. */ @@ -528,7 +528,7 @@ EXPORT_SYMBOL(tty_termios_hw_change); * is a bit of layering violation here with n_tty in terms of the * internal knowledge of this function. * - * Locking: termios_mutex + * Locking: termios_rwsem */ int tty_set_termios(struct tty_struct *tty, struct ktermios *new_termios) @@ -544,7 +544,7 @@ int tty_set_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); + down_write(&tty->termios_rwsem); old_termios = tty->termios; tty->termios = *new_termios; unset_locked_termios(&tty->termios, &old_termios, &tty->termios_locked); @@ -586,7 +586,7 @@ int tty_set_termios(struct tty_struct *tty, struct ktermios *new_termios) (ld->ops->set_termios)(tty, &old_termios); tty_ldisc_deref(ld); } - mutex_unlock(&tty->termios_mutex); + up_write(&tty->termios_rwsem); return 0; } EXPORT_SYMBOL_GPL(tty_set_termios); @@ -601,7 +601,7 @@ EXPORT_SYMBOL_GPL(tty_set_termios); * functions before using tty_set_termios to do the actual changes. * * Locking: - * Called functions take ldisc and termios_mutex locks + * Called functions take ldisc and termios_rwsem locks */ static int set_termios(struct tty_struct *tty, void __user *arg, int opt) @@ -613,9 +613,9 @@ static int set_termios(struct tty_struct *tty, void __user *arg, int opt) if (retval) return retval; - mutex_lock(&tty->termios_mutex); + down_read(&tty->termios_rwsem); tmp_termios = tty->termios; - mutex_unlock(&tty->termios_mutex); + up_read(&tty->termios_rwsem); if (opt & TERMIOS_TERMIO) { if (user_termio_to_kernel_termios(&tmp_termios, @@ -667,16 +667,16 @@ 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); + down_read(&tty->termios_rwsem); *kterm = tty->termios; - mutex_unlock(&tty->termios_mutex); + up_read(&tty->termios_rwsem); } static void copy_termios_locked(struct tty_struct *tty, struct ktermios *kterm) { - mutex_lock(&tty->termios_mutex); + down_read(&tty->termios_rwsem); *kterm = tty->termios_locked; - mutex_unlock(&tty->termios_mutex); + up_read(&tty->termios_rwsem); } static int get_termio(struct tty_struct *tty, struct termio __user *termio) @@ -723,10 +723,10 @@ static int set_termiox(struct tty_struct *tty, void __user *arg, int opt) return -ERESTARTSYS; } - mutex_lock(&tty->termios_mutex); + down_write(&tty->termios_rwsem); if (tty->ops->set_termiox) tty->ops->set_termiox(tty, &tnew); - mutex_unlock(&tty->termios_mutex); + up_write(&tty->termios_rwsem); return 0; } @@ -761,13 +761,13 @@ static int get_sgttyb(struct tty_struct *tty, struct sgttyb __user *sgttyb) { struct sgttyb tmp; - mutex_lock(&tty->termios_mutex); + down_read(&tty->termios_rwsem); tmp.sg_ispeed = tty->termios.c_ispeed; tmp.sg_ospeed = tty->termios.c_ospeed; tmp.sg_erase = tty->termios.c_cc[VERASE]; tmp.sg_kill = tty->termios.c_cc[VKILL]; tmp.sg_flags = get_sgflags(tty); - mutex_unlock(&tty->termios_mutex); + up_read(&tty->termios_rwsem); return copy_to_user(sgttyb, &tmp, sizeof(tmp)) ? -EFAULT : 0; } @@ -806,7 +806,7 @@ static void set_sgflags(struct ktermios *termios, int flags) * Updates a terminal from the legacy BSD style terminal information * structure. * - * Locking: termios_mutex + * Locking: termios_rwsem */ static int set_sgttyb(struct tty_struct *tty, struct sgttyb __user *sgttyb) @@ -822,7 +822,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); + down_write(&tty->termios_rwsem); termios = tty->termios; termios.c_cc[VERASE] = tmp.sg_erase; termios.c_cc[VKILL] = tmp.sg_kill; @@ -832,7 +832,7 @@ static int set_sgttyb(struct tty_struct *tty, struct sgttyb __user *sgttyb) tty_termios_encode_baud_rate(&termios, termios.c_ispeed, termios.c_ospeed); #endif - mutex_unlock(&tty->termios_mutex); + up_write(&tty->termios_rwsem); tty_set_termios(tty, &termios); return 0; } @@ -843,14 +843,14 @@ static int get_tchars(struct tty_struct *tty, struct tchars __user *tchars) { struct tchars tmp; - mutex_lock(&tty->termios_mutex); + down_read(&tty->termios_rwsem); tmp.t_intrc = tty->termios.c_cc[VINTR]; tmp.t_quitc = tty->termios.c_cc[VQUIT]; tmp.t_startc = tty->termios.c_cc[VSTART]; tmp.t_stopc = tty->termios.c_cc[VSTOP]; tmp.t_eofc = tty->termios.c_cc[VEOF]; tmp.t_brkc = tty->termios.c_cc[VEOL2]; /* what is brkc anyway? */ - mutex_unlock(&tty->termios_mutex); + up_read(&tty->termios_rwsem); return copy_to_user(tchars, &tmp, sizeof(tmp)) ? -EFAULT : 0; } @@ -860,14 +860,14 @@ 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); + down_write(&tty->termios_rwsem); tty->termios.c_cc[VINTR] = tmp.t_intrc; tty->termios.c_cc[VQUIT] = tmp.t_quitc; tty->termios.c_cc[VSTART] = tmp.t_startc; tty->termios.c_cc[VSTOP] = tmp.t_stopc; tty->termios.c_cc[VEOF] = tmp.t_eofc; tty->termios.c_cc[VEOL2] = tmp.t_brkc; /* what is brkc anyway? */ - mutex_unlock(&tty->termios_mutex); + up_write(&tty->termios_rwsem); return 0; } #endif @@ -877,7 +877,7 @@ static int get_ltchars(struct tty_struct *tty, struct ltchars __user *ltchars) { struct ltchars tmp; - mutex_lock(&tty->termios_mutex); + down_read(&tty->termios_rwsem); tmp.t_suspc = tty->termios.c_cc[VSUSP]; /* what is dsuspc anyway? */ tmp.t_dsuspc = tty->termios.c_cc[VSUSP]; @@ -886,7 +886,7 @@ static int get_ltchars(struct tty_struct *tty, struct ltchars __user *ltchars) tmp.t_flushc = tty->termios.c_cc[VEOL2]; tmp.t_werasc = tty->termios.c_cc[VWERASE]; tmp.t_lnextc = tty->termios.c_cc[VLNEXT]; - mutex_unlock(&tty->termios_mutex); + up_read(&tty->termios_rwsem); return copy_to_user(ltchars, &tmp, sizeof(tmp)) ? -EFAULT : 0; } @@ -897,7 +897,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); + down_write(&tty->termios_rwsem); tty->termios.c_cc[VSUSP] = tmp.t_suspc; /* what is dsuspc anyway? */ tty->termios.c_cc[VEOL2] = tmp.t_dsuspc; @@ -906,7 +906,7 @@ static int set_ltchars(struct tty_struct *tty, struct ltchars __user *ltchars) tty->termios.c_cc[VEOL2] = tmp.t_flushc; tty->termios.c_cc[VWERASE] = tmp.t_werasc; tty->termios.c_cc[VLNEXT] = tmp.t_lnextc; - mutex_unlock(&tty->termios_mutex); + up_write(&tty->termios_rwsem); return 0; } #endif @@ -946,7 +946,7 @@ static int send_prio_char(struct tty_struct *tty, char ch) * @arg: enable/disable CLOCAL * * Perform a change to the CLOCAL state and call into the driver - * layer to make it visible. All done with the termios mutex + * layer to make it visible. All done with the termios rwsem */ static int tty_change_softcar(struct tty_struct *tty, int arg) @@ -955,7 +955,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); + down_write(&tty->termios_rwsem); old = tty->termios; tty->termios.c_cflag &= ~CLOCAL; tty->termios.c_cflag |= bit; @@ -963,7 +963,7 @@ static int tty_change_softcar(struct tty_struct *tty, int arg) tty->ops->set_termios(tty, &old); if ((tty->termios.c_cflag & CLOCAL) != bit) ret = -EINVAL; - mutex_unlock(&tty->termios_mutex); + up_write(&tty->termios_rwsem); return ret; } @@ -1066,9 +1066,9 @@ 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); + down_write(&real_tty->termios_rwsem); real_tty->termios_locked = kterm; - mutex_unlock(&real_tty->termios_mutex); + up_write(&real_tty->termios_rwsem); return 0; #else case TIOCGLCKTRMIOS: @@ -1083,9 +1083,9 @@ 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); + down_write(&real_tty->termios_rwsem); real_tty->termios_locked = kterm; - mutex_unlock(&real_tty->termios_mutex); + up_write(&real_tty->termios_rwsem); return ret; #endif #ifdef TCGETX @@ -1093,9 +1093,9 @@ 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); + down_read(&real_tty->termios_rwsem); memcpy(&ktermx, real_tty->termiox, sizeof(struct termiox)); - mutex_unlock(&real_tty->termios_mutex); + up_read(&real_tty->termios_rwsem); if (copy_to_user(p, &ktermx, sizeof(struct termiox))) ret = -EFAULT; return ret; diff --git a/drivers/tty/tty_ldisc.c b/drivers/tty/tty_ldisc.c index e52d6157355..6458e11e8e9 100644 --- a/drivers/tty/tty_ldisc.c +++ b/drivers/tty/tty_ldisc.c @@ -415,14 +415,14 @@ EXPORT_SYMBOL_GPL(tty_ldisc_flush); * they are not on hot paths so a little discipline won't do * any harm. * - * Locking: takes termios_mutex + * Locking: takes termios_rwsem */ static void tty_set_termios_ldisc(struct tty_struct *tty, int num) { - mutex_lock(&tty->termios_mutex); + down_write(&tty->termios_rwsem); tty->termios.c_line = num; - mutex_unlock(&tty->termios_mutex); + up_write(&tty->termios_rwsem); } /** @@ -602,11 +602,11 @@ int tty_set_ldisc(struct tty_struct *tty, int ldisc) static void tty_reset_termios(struct tty_struct *tty) { - mutex_lock(&tty->termios_mutex); + down_write(&tty->termios_rwsem); 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); - mutex_unlock(&tty->termios_mutex); + up_write(&tty->termios_rwsem); } diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c index c677829baa8..02af6ccefe6 100644 --- a/drivers/tty/vt/vt.c +++ b/drivers/tty/vt/vt.c @@ -828,7 +828,7 @@ static inline int resize_screen(struct vc_data *vc, int width, int height, * If the caller passes a tty structure then update the termios winsize * information and perform any necessary signal handling. * - * Caller must hold the console semaphore. Takes the termios mutex and + * Caller must hold the console semaphore. Takes the termios rwsem and * ctrl_lock of the tty IFF a tty is passed. */ @@ -972,7 +972,7 @@ int vc_resize(struct vc_data *vc, unsigned int cols, unsigned int rows) * the actual work. * * Takes the console sem and the called methods then take the tty - * termios_mutex and the tty ctrl_lock in that order. + * termios_rwsem and the tty ctrl_lock in that order. */ static int vt_resize(struct tty_struct *tty, struct winsize *ws) { diff --git a/include/linux/tty.h b/include/linux/tty.h index 8323ee4f95b..d3042076d16 100644 --- a/include/linux/tty.h +++ b/include/linux/tty.h @@ -10,6 +10,7 @@ #include #include #include +#include @@ -243,9 +244,9 @@ struct tty_struct { struct mutex atomic_write_lock; struct mutex legacy_mutex; - struct mutex termios_mutex; + struct rw_semaphore termios_rwsem; spinlock_t ctrl_lock; - /* Termios values are protected by the termios mutex */ + /* Termios values are protected by the termios rwsem */ struct ktermios termios, termios_locked; struct termiox *termiox; /* May be NULL for unsupported */ char name[64]; @@ -253,7 +254,7 @@ struct tty_struct { struct pid *session; unsigned long flags; int count; - struct winsize winsize; /* termios mutex */ + struct winsize winsize; /* termios rwsem */ unsigned char stopped:1, hw_stopped:1, flow_stopped:1, packet:1; unsigned char ctrl_status; /* ctrl_lock */ unsigned int receive_room; /* Bytes free for queue */ -- cgit v1.2.3-70-g09d2 From d8c1f929aa8164cd8eaa830068d2fa3159c0764a Mon Sep 17 00:00:00 2001 From: Peter Hurley Date: Sat, 15 Jun 2013 09:14:31 -0400 Subject: tty: Only guarantee termios read safety for throttle/unthrottle No tty driver modifies termios during throttle() or unthrottle(). Therefore, only read safety is required. However, tty_throttle_safe and tty_unthrottle_safe must still be mutually exclusive; introduce throttle_mutex for that purpose. Signed-off-by: Peter Hurley Signed-off-by: Greg Kroah-Hartman --- drivers/tty/n_tty.c | 4 ---- drivers/tty/tty_io.c | 1 + drivers/tty/tty_ioctl.c | 8 ++++---- include/linux/tty.h | 1 + 4 files changed, 6 insertions(+), 8 deletions(-) (limited to 'include/linux/tty.h') diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c index d0c8805d813..b78ee464bd0 100644 --- a/drivers/tty/n_tty.c +++ b/drivers/tty/n_tty.c @@ -1518,9 +1518,7 @@ static void __receive_buf(struct tty_struct *tty, const unsigned char *cp, tty_set_flow_change(tty, TTY_THROTTLE_SAFE); if (receive_room(tty) >= TTY_THRESHOLD_THROTTLE) break; - up_read(&tty->termios_rwsem); throttled = tty_throttle_safe(tty); - down_read(&tty->termios_rwsem); if (!throttled) break; } @@ -2086,9 +2084,7 @@ do_it_again: if (!tty->count) break; n_tty_set_room(tty); - up_read(&tty->termios_rwsem); unthrottled = tty_unthrottle_safe(tty); - down_read(&tty->termios_rwsem); if (!unthrottled) break; } diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c index 639e24ade9b..1b32da6f6cd 100644 --- a/drivers/tty/tty_io.c +++ b/drivers/tty/tty_io.c @@ -3015,6 +3015,7 @@ void initialize_tty_struct(struct tty_struct *tty, tty->session = NULL; tty->pgrp = NULL; mutex_init(&tty->legacy_mutex); + mutex_init(&tty->throttle_mutex); init_rwsem(&tty->termios_rwsem); init_ldsem(&tty->ldisc_sem); init_waitqueue_head(&tty->write_wait); diff --git a/drivers/tty/tty_ioctl.c b/drivers/tty/tty_ioctl.c index 9ce20df8a2c..03ba081c577 100644 --- a/drivers/tty/tty_ioctl.c +++ b/drivers/tty/tty_ioctl.c @@ -151,7 +151,7 @@ int tty_throttle_safe(struct tty_struct *tty) { int ret = 0; - down_write(&tty->termios_rwsem); + mutex_lock(&tty->throttle_mutex); if (!test_bit(TTY_THROTTLED, &tty->flags)) { if (tty->flow_change != TTY_THROTTLE_SAFE) ret = 1; @@ -161,7 +161,7 @@ int tty_throttle_safe(struct tty_struct *tty) tty->ops->throttle(tty); } } - up_write(&tty->termios_rwsem); + mutex_unlock(&tty->throttle_mutex); return ret; } @@ -182,7 +182,7 @@ int tty_unthrottle_safe(struct tty_struct *tty) { int ret = 0; - down_write(&tty->termios_rwsem); + mutex_lock(&tty->throttle_mutex); if (test_bit(TTY_THROTTLED, &tty->flags)) { if (tty->flow_change != TTY_UNTHROTTLE_SAFE) ret = 1; @@ -192,7 +192,7 @@ int tty_unthrottle_safe(struct tty_struct *tty) tty->ops->unthrottle(tty); } } - up_write(&tty->termios_rwsem); + mutex_unlock(&tty->throttle_mutex); return ret; } diff --git a/include/linux/tty.h b/include/linux/tty.h index d3042076d16..57a70d1d041 100644 --- a/include/linux/tty.h +++ b/include/linux/tty.h @@ -244,6 +244,7 @@ struct tty_struct { struct mutex atomic_write_lock; struct mutex legacy_mutex; + struct mutex throttle_mutex; struct rw_semaphore termios_rwsem; spinlock_t ctrl_lock; /* Termios values are protected by the termios rwsem */ -- cgit v1.2.3-70-g09d2 From 1fc359fc3ea72314cc3ebdfa94c60e020c152cd2 Mon Sep 17 00:00:00 2001 From: Peter Hurley Date: Sat, 15 Jun 2013 09:36:01 -0400 Subject: tty: Compute flip buffer ptrs The char_buf_ptr and flag_buf_ptr values are trivially derived from the .data field offset; compute values as needed. Fixes a long-standing type-mismatch with the char and flag ptrs. Signed-off-by: Peter Hurley Signed-off-by: Greg Kroah-Hartman --- drivers/tty/tty_buffer.c | 22 ++++++++++------------ include/linux/tty.h | 12 ++++++++++-- include/linux/tty_flip.h | 4 ++-- 3 files changed, 22 insertions(+), 16 deletions(-) (limited to 'include/linux/tty.h') diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c index ff1b2e37c3c..170674cb68f 100644 --- a/drivers/tty/tty_buffer.c +++ b/drivers/tty/tty_buffer.c @@ -71,8 +71,6 @@ static struct tty_buffer *tty_buffer_alloc(struct tty_port *port, size_t size) p->next = NULL; p->commit = 0; p->read = 0; - p->char_buf_ptr = (char *)(p->data); - p->flag_buf_ptr = (unsigned char *)p->char_buf_ptr + size; port->buf.memory_used += size; return p; } @@ -265,8 +263,8 @@ int tty_insert_flip_string_fixed_flag(struct tty_port *port, if (unlikely(space == 0)) { break; } - memcpy(tb->char_buf_ptr + tb->used, chars, space); - memset(tb->flag_buf_ptr + tb->used, flag, space); + memcpy(char_buf_ptr(tb, tb->used), chars, space); + memset(flag_buf_ptr(tb, tb->used), flag, space); tb->used += space; copied += space; chars += space; @@ -303,8 +301,8 @@ int tty_insert_flip_string_flags(struct tty_port *port, if (unlikely(space == 0)) { break; } - memcpy(tb->char_buf_ptr + tb->used, chars, space); - memcpy(tb->flag_buf_ptr + tb->used, flags, space); + memcpy(char_buf_ptr(tb, tb->used), chars, space); + memcpy(flag_buf_ptr(tb, tb->used), flags, space); tb->used += space; copied += space; chars += space; @@ -364,8 +362,8 @@ int tty_prepare_flip_string(struct tty_port *port, unsigned char **chars, int space = tty_buffer_request_room(port, size); if (likely(space)) { struct tty_buffer *tb = port->buf.tail; - *chars = tb->char_buf_ptr + tb->used; - memset(tb->flag_buf_ptr + tb->used, TTY_NORMAL, space); + *chars = char_buf_ptr(tb, tb->used); + memset(flag_buf_ptr(tb, tb->used), TTY_NORMAL, space); tb->used += space; } return space; @@ -394,8 +392,8 @@ int tty_prepare_flip_string_flags(struct tty_port *port, int space = tty_buffer_request_room(port, size); if (likely(space)) { struct tty_buffer *tb = port->buf.tail; - *chars = tb->char_buf_ptr + tb->used; - *flags = tb->flag_buf_ptr + tb->used; + *chars = char_buf_ptr(tb, tb->used); + *flags = flag_buf_ptr(tb, tb->used); tb->used += space; } return space; @@ -407,8 +405,8 @@ static int receive_buf(struct tty_struct *tty, struct tty_buffer *head, int count) { struct tty_ldisc *disc = tty->ldisc; - char *p = head->char_buf_ptr + head->read; - unsigned char *f = head->flag_buf_ptr + head->read; + unsigned char *p = char_buf_ptr(head, head->read); + char *f = flag_buf_ptr(head, head->read); if (disc->ops->receive_buf2) count = disc->ops->receive_buf2(tty, p, f, count); diff --git a/include/linux/tty.h b/include/linux/tty.h index 57a70d1d041..87bbaa31ebf 100644 --- a/include/linux/tty.h +++ b/include/linux/tty.h @@ -31,8 +31,6 @@ struct tty_buffer { struct tty_buffer *next; - char *char_buf_ptr; - unsigned char *flag_buf_ptr; int used; int size; int commit; @@ -41,6 +39,16 @@ struct tty_buffer { unsigned long data[0]; }; +static inline unsigned char *char_buf_ptr(struct tty_buffer *b, int ofs) +{ + return ((unsigned char *)b->data) + ofs; +} + +static inline char *flag_buf_ptr(struct tty_buffer *b, int ofs) +{ + return (char *)char_buf_ptr(b, ofs) + b->size; +} + /* * We default to dicing tty buffer allocations to this many characters * in order to avoid multiple page allocations. We know the size of diff --git a/include/linux/tty_flip.h b/include/linux/tty_flip.h index e0f252633b4..ad0303994c3 100644 --- a/include/linux/tty_flip.h +++ b/include/linux/tty_flip.h @@ -18,8 +18,8 @@ static inline int tty_insert_flip_char(struct tty_port *port, { struct tty_buffer *tb = port->buf.tail; if (tb && tb->used < tb->size) { - tb->flag_buf_ptr[tb->used] = flag; - tb->char_buf_ptr[tb->used++] = ch; + *flag_buf_ptr(tb, tb->used) = flag; + *char_buf_ptr(tb, tb->used++) = ch; return 1; } return tty_insert_flip_string_flags(port, &ch, &flag, 1); -- cgit v1.2.3-70-g09d2 From 809850b7a5fcc0a96d023e1171a7944c60fd5a71 Mon Sep 17 00:00:00 2001 From: Peter Hurley Date: Sat, 15 Jun 2013 09:36:06 -0400 Subject: tty: Use lockless flip buffer free list In preparation for lockless flip buffers, make the flip buffer free list lockless. NB: using llist is not the optimal solution, as the driver and buffer work may contend over the llist head unnecessarily. However, test measurements indicate this contention is low. Signed-off-by: Peter Hurley Signed-off-by: Greg Kroah-Hartman --- drivers/tty/tty_buffer.c | 29 ++++++++++++----------------- include/linux/llist.h | 23 +++++++++++++++++++++++ include/linux/tty.h | 8 ++++++-- 3 files changed, 41 insertions(+), 19 deletions(-) (limited to 'include/linux/tty.h') diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c index 0259a766b87..069640e5b9c 100644 --- a/drivers/tty/tty_buffer.c +++ b/drivers/tty/tty_buffer.c @@ -44,16 +44,17 @@ static void tty_buffer_reset(struct tty_buffer *p, size_t size) void tty_buffer_free_all(struct tty_port *port) { struct tty_bufhead *buf = &port->buf; - struct tty_buffer *p; + struct tty_buffer *p, *next; + struct llist_node *llist; while ((p = buf->head) != NULL) { buf->head = p->next; kfree(p); } - while ((p = buf->free) != NULL) { - buf->free = p->next; + llist = llist_del_all(&buf->free); + llist_for_each_entry_safe(p, next, llist, free) kfree(p); - } + buf->tail = NULL; buf->memory_used = 0; } @@ -68,22 +69,20 @@ void tty_buffer_free_all(struct tty_port *port) * allocation behaviour. * Return NULL if out of memory or the allocation would exceed the * per device queue - * - * Locking: Caller must hold tty->buf.lock */ static struct tty_buffer *tty_buffer_alloc(struct tty_port *port, size_t size) { - struct tty_buffer **tbh = &port->buf.free; + struct llist_node *free; struct tty_buffer *p; /* Round the buffer size out */ size = __ALIGN_MASK(size, TTYB_ALIGN_MASK); if (size <= MIN_TTYB_SIZE) { - if (*tbh) { - p = *tbh; - *tbh = p->next; + free = llist_del_first(&port->buf.free); + if (free) { + p = llist_entry(free, struct tty_buffer, free); goto found; } } @@ -109,8 +108,6 @@ found: * * Free a tty buffer, or add it to the free list according to our * internal strategy - * - * Locking: Caller must hold tty->buf.lock */ static void tty_buffer_free(struct tty_port *port, struct tty_buffer *b) @@ -123,10 +120,8 @@ static void tty_buffer_free(struct tty_port *port, struct tty_buffer *b) if (b->size > MIN_TTYB_SIZE) kfree(b); - else { - b->next = buf->free; - buf->free = b; - } + else + llist_add(&b->free, &buf->free); } /** @@ -542,7 +537,7 @@ void tty_buffer_init(struct tty_port *port) spin_lock_init(&buf->lock); buf->head = NULL; buf->tail = NULL; - buf->free = NULL; + init_llist_head(&buf->free); buf->memory_used = 0; INIT_WORK(&buf->work, flush_to_ldisc); } diff --git a/include/linux/llist.h b/include/linux/llist.h index cdaa7f02389..8828a78dec9 100644 --- a/include/linux/llist.h +++ b/include/linux/llist.h @@ -124,6 +124,29 @@ static inline void init_llist_head(struct llist_head *list) &(pos)->member != NULL; \ (pos) = llist_entry((pos)->member.next, typeof(*(pos)), member)) +/** + * llist_for_each_entry_safe - iterate over some deleted entries of lock-less list of given type + * safe against removal of list entry + * @pos: the type * to use as a loop cursor. + * @n: another type * to use as temporary storage + * @node: the first entry of deleted list entries. + * @member: the name of the llist_node with the struct. + * + * In general, some entries of the lock-less list can be traversed + * safely only after being removed from list, so start with an entry + * instead of list head. + * + * If being used on entries deleted from lock-less list directly, the + * traverse order is from the newest to the oldest added entry. If + * you want to traverse from the oldest to the newest, you must + * reverse the order by yourself before traversing. + */ +#define llist_for_each_entry_safe(pos, n, node, member) \ + for (pos = llist_entry((node), typeof(*pos), member); \ + &pos->member != NULL && \ + (n = llist_entry(pos->member.next, typeof(*n), member), true); \ + pos = n) + /** * llist_empty - tests whether a lock-less list is empty * @head: the list to test diff --git a/include/linux/tty.h b/include/linux/tty.h index 87bbaa31ebf..5043b12f23e 100644 --- a/include/linux/tty.h +++ b/include/linux/tty.h @@ -11,6 +11,7 @@ #include #include #include +#include @@ -30,7 +31,10 @@ #define __DISABLED_CHAR '\0' struct tty_buffer { - struct tty_buffer *next; + union { + struct tty_buffer *next; + struct llist_node free; + }; int used; int size; int commit; @@ -65,7 +69,7 @@ struct tty_bufhead { spinlock_t lock; struct tty_buffer *head; /* Queue head */ struct tty_buffer *tail; /* Active buffer */ - struct tty_buffer *free; /* Free queue head */ + struct llist_head free; /* Free queue head */ int memory_used; /* Buffer space used excluding free queue */ }; -- cgit v1.2.3-70-g09d2 From 7391ee16950e772076d321792d9fbf030f921345 Mon Sep 17 00:00:00 2001 From: Peter Hurley Date: Sat, 15 Jun 2013 09:36:07 -0400 Subject: tty: Simplify flip buffer list with 0-sized sentinel Use a 0-sized sentinel to avoid assigning the head ptr from the driver side thread. This also eliminates testing head/tail for NULL. When the sentinel is first 'consumed' by the buffer work (or by tty_buffer_flush()), it is detached from the list but not freed nor added to the free list. Both buffer work and tty_buffer_flush() continue to preserve at least 1 flip buffer to which head & tail is pointed. Signed-off-by: Peter Hurley Signed-off-by: Greg Kroah-Hartman --- drivers/tty/tty_buffer.c | 49 ++++++++++++++++++------------------------------ include/linux/tty.h | 1 + 2 files changed, 19 insertions(+), 31 deletions(-) (limited to 'include/linux/tty.h') diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c index 069640e5b9c..231b7a8710f 100644 --- a/drivers/tty/tty_buffer.c +++ b/drivers/tty/tty_buffer.c @@ -49,13 +49,16 @@ void tty_buffer_free_all(struct tty_port *port) while ((p = buf->head) != NULL) { buf->head = p->next; - kfree(p); + if (p->size > 0) + kfree(p); } llist = llist_del_all(&buf->free); llist_for_each_entry_safe(p, next, llist, free) kfree(p); - buf->tail = NULL; + tty_buffer_reset(&buf->sentinel, 0); + buf->head = &buf->sentinel; + buf->tail = &buf->sentinel; buf->memory_used = 0; } @@ -120,7 +123,7 @@ static void tty_buffer_free(struct tty_port *port, struct tty_buffer *b) if (b->size > MIN_TTYB_SIZE) kfree(b); - else + else if (b->size > 0) llist_add(&b->free, &buf->free); } @@ -140,8 +143,6 @@ static void __tty_buffer_flush(struct tty_port *port) struct tty_bufhead *buf = &port->buf; struct tty_buffer *next; - if (unlikely(buf->head == NULL)) - return; while ((next = buf->head->next) != NULL) { tty_buffer_free(port, buf->head); buf->head = next; @@ -200,23 +201,14 @@ int tty_buffer_request_room(struct tty_port *port, size_t size) int left; unsigned long flags; spin_lock_irqsave(&buf->lock, flags); - /* OPTIMISATION: We could keep a per tty "zero" sized buffer to - remove this conditional if its worth it. This would be invisible - to the callers */ b = buf->tail; - if (b != NULL) - left = b->size - b->used; - else - left = 0; + left = b->size - b->used; if (left < size) { /* This is the slow path - looking for new buffers to use */ if ((n = tty_buffer_alloc(port, size)) != NULL) { - if (b != NULL) { - b->next = n; - b->commit = b->used; - } else - buf->head = n; + b->next = n; + b->commit = b->used; buf->tail = n; } else size = left; @@ -247,10 +239,8 @@ int tty_insert_flip_string_fixed_flag(struct tty_port *port, int goal = min_t(size_t, size - copied, TTY_BUFFER_PAGE); int space = tty_buffer_request_room(port, goal); struct tty_buffer *tb = port->buf.tail; - /* If there is no space then tb may be NULL */ - if (unlikely(space == 0)) { + if (unlikely(space == 0)) break; - } memcpy(char_buf_ptr(tb, tb->used), chars, space); memset(flag_buf_ptr(tb, tb->used), flag, space); tb->used += space; @@ -285,10 +275,8 @@ int tty_insert_flip_string_flags(struct tty_port *port, int goal = min_t(size_t, size - copied, TTY_BUFFER_PAGE); int space = tty_buffer_request_room(port, goal); struct tty_buffer *tb = port->buf.tail; - /* If there is no space then tb may be NULL */ - if (unlikely(space == 0)) { + if (unlikely(space == 0)) break; - } memcpy(char_buf_ptr(tb, tb->used), chars, space); memcpy(flag_buf_ptr(tb, tb->used), flags, space); tb->used += space; @@ -322,8 +310,7 @@ void tty_schedule_flip(struct tty_port *port) WARN_ON(port->low_latency); spin_lock_irqsave(&buf->lock, flags); - if (buf->tail != NULL) - buf->tail->commit = buf->tail->used; + buf->tail->commit = buf->tail->used; spin_unlock_irqrestore(&buf->lock, flags); schedule_work(&buf->work); } @@ -438,8 +425,8 @@ static void flush_to_ldisc(struct work_struct *work) spin_lock_irqsave(&buf->lock, flags); if (!test_and_set_bit(TTYP_FLUSHING, &port->iflags)) { - struct tty_buffer *head; - while ((head = buf->head) != NULL) { + while (1) { + struct tty_buffer *head = buf->head; int count; count = head->commit - head->read; @@ -509,8 +496,7 @@ void tty_flip_buffer_push(struct tty_port *port) unsigned long flags; spin_lock_irqsave(&buf->lock, flags); - if (buf->tail != NULL) - buf->tail->commit = buf->tail->used; + buf->tail->commit = buf->tail->used; spin_unlock_irqrestore(&buf->lock, flags); if (port->low_latency) @@ -535,8 +521,9 @@ void tty_buffer_init(struct tty_port *port) struct tty_bufhead *buf = &port->buf; spin_lock_init(&buf->lock); - buf->head = NULL; - buf->tail = NULL; + tty_buffer_reset(&buf->sentinel, 0); + buf->head = &buf->sentinel; + buf->tail = &buf->sentinel; init_llist_head(&buf->free); buf->memory_used = 0; INIT_WORK(&buf->work, flush_to_ldisc); diff --git a/include/linux/tty.h b/include/linux/tty.h index 5043b12f23e..2e93eb831c6 100644 --- a/include/linux/tty.h +++ b/include/linux/tty.h @@ -67,6 +67,7 @@ static inline char *flag_buf_ptr(struct tty_buffer *b, int ofs) struct tty_bufhead { struct work_struct work; spinlock_t lock; + struct tty_buffer sentinel; struct tty_buffer *head; /* Queue head */ struct tty_buffer *tail; /* Active buffer */ struct llist_head free; /* Free queue head */ -- cgit v1.2.3-70-g09d2 From 7bfe0b7116be207cf2204ae06335cc89d8f8ee02 Mon Sep 17 00:00:00 2001 From: Peter Hurley Date: Sat, 15 Jun 2013 09:36:08 -0400 Subject: tty: Track flip buffer memory limit atomically Lockless flip buffers require atomically updating the bytes-in-use watermark. The pty driver also peeks at the watermark value to limit memory consumption to a much lower value than the default; query the watermark with new fn, tty_buffer_space_avail(). Signed-off-by: Peter Hurley Signed-off-by: Greg Kroah-Hartman --- drivers/tty/pty.c | 10 +++------- drivers/tty/tty_buffer.c | 37 +++++++++++++++++++++++++++++++------ include/linux/tty.h | 3 +-- include/linux/tty_flip.h | 1 + 4 files changed, 36 insertions(+), 15 deletions(-) (limited to 'include/linux/tty.h') diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c index 1b39dd639ee..b38a28bd951 100644 --- a/drivers/tty/pty.c +++ b/drivers/tty/pty.c @@ -89,17 +89,13 @@ static void pty_unthrottle(struct tty_struct *tty) * pty_space - report space left for writing * @to: tty we are writing into * - * The tty buffers allow 64K but we sneak a peak and clip at 8K this - * allows a lot of overspill room for echo and other fun messes to - * be handled properly + * Limit the buffer space used by ptys to 8k. */ static int pty_space(struct tty_struct *to) { - int n = 8192 - to->port->buf.memory_used; - if (n < 0) - return 0; - return n; + int n = tty_buffer_space_avail(to->port); + return min(n, 8192); } /** diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c index 231b7a8710f..5d5a56407aa 100644 --- a/drivers/tty/tty_buffer.c +++ b/drivers/tty/tty_buffer.c @@ -22,6 +22,31 @@ #define MIN_TTYB_SIZE 256 #define TTYB_ALIGN_MASK 255 +/* + * Byte threshold to limit memory consumption for flip buffers. + * The actual memory limit is > 2x this amount. + */ +#define TTYB_MEM_LIMIT 65536 + + +/** + * tty_buffer_space_avail - return unused buffer space + * @port - tty_port owning the flip buffer + * + * Returns the # of bytes which can be written by the driver without + * reaching the buffer limit. + * + * Note: this does not guarantee that memory is available to write + * the returned # of bytes (use tty_prepare_flip_string_xxx() to + * pre-allocate if memory guarantee is required). + */ + +int tty_buffer_space_avail(struct tty_port *port) +{ + int space = TTYB_MEM_LIMIT - atomic_read(&port->buf.memory_used); + return max(space, 0); +} + static void tty_buffer_reset(struct tty_buffer *p, size_t size) { p->used = 0; @@ -59,7 +84,8 @@ void tty_buffer_free_all(struct tty_port *port) tty_buffer_reset(&buf->sentinel, 0); buf->head = &buf->sentinel; buf->tail = &buf->sentinel; - buf->memory_used = 0; + + atomic_set(&buf->memory_used, 0); } /** @@ -92,7 +118,7 @@ static struct tty_buffer *tty_buffer_alloc(struct tty_port *port, size_t size) /* Should possibly check if this fails for the largest buffer we have queued and recycle that ? */ - if (port->buf.memory_used + size > 65536) + if (atomic_read(&port->buf.memory_used) > TTYB_MEM_LIMIT) return NULL; p = kmalloc(sizeof(struct tty_buffer) + 2 * size, GFP_ATOMIC); if (p == NULL) @@ -100,7 +126,7 @@ static struct tty_buffer *tty_buffer_alloc(struct tty_port *port, size_t size) found: tty_buffer_reset(p, size); - port->buf.memory_used += size; + atomic_add(size, &port->buf.memory_used); return p; } @@ -118,8 +144,7 @@ static void tty_buffer_free(struct tty_port *port, struct tty_buffer *b) struct tty_bufhead *buf = &port->buf; /* Dumb strategy for now - should keep some stats */ - buf->memory_used -= b->size; - WARN_ON(buf->memory_used < 0); + WARN_ON(atomic_sub_return(b->size, &buf->memory_used) < 0); if (b->size > MIN_TTYB_SIZE) kfree(b); @@ -525,7 +550,7 @@ void tty_buffer_init(struct tty_port *port) buf->head = &buf->sentinel; buf->tail = &buf->sentinel; init_llist_head(&buf->free); - buf->memory_used = 0; + atomic_set(&buf->memory_used, 0); INIT_WORK(&buf->work, flush_to_ldisc); } diff --git a/include/linux/tty.h b/include/linux/tty.h index 2e93eb831c6..7c124541f01 100644 --- a/include/linux/tty.h +++ b/include/linux/tty.h @@ -71,8 +71,7 @@ struct tty_bufhead { struct tty_buffer *head; /* Queue head */ struct tty_buffer *tail; /* Active buffer */ struct llist_head free; /* Free queue head */ - int memory_used; /* Buffer space used excluding - free queue */ + atomic_t memory_used; /* In-use buffers excluding free list */ }; /* * When a break, frame error, or parity error happens, these codes are diff --git a/include/linux/tty_flip.h b/include/linux/tty_flip.h index ad0303994c3..6944ed2ce69 100644 --- a/include/linux/tty_flip.h +++ b/include/linux/tty_flip.h @@ -1,6 +1,7 @@ #ifndef _LINUX_TTY_FLIP_H #define _LINUX_TTY_FLIP_H +extern int tty_buffer_space_avail(struct tty_port *port); extern int tty_buffer_request_room(struct tty_port *port, size_t size); extern int tty_insert_flip_string_flags(struct tty_port *port, const unsigned char *chars, const char *flags, size_t size); -- cgit v1.2.3-70-g09d2 From e9975fdec0138f1b2a85b9624e41660abd9865d4 Mon Sep 17 00:00:00 2001 From: Peter Hurley Date: Sat, 15 Jun 2013 09:36:10 -0400 Subject: tty: Ensure single-threaded flip buffer consumer with mutex The buffer work may race with parallel tty_buffer_flush. Use a mutex to guarantee exclusive modify access to the head flip buffer. Remove the unneeded spin lock. Signed-off-by: Peter Hurley Signed-off-by: Greg Kroah-Hartman --- drivers/staging/dgrp/dgrp_tty.c | 2 ++ drivers/tty/tty_buffer.c | 40 +++++++++++++++++++--------------------- include/linux/tty.h | 2 +- 3 files changed, 22 insertions(+), 22 deletions(-) (limited to 'include/linux/tty.h') diff --git a/drivers/staging/dgrp/dgrp_tty.c b/drivers/staging/dgrp/dgrp_tty.c index 654f6010b47..0d52de3729c 100644 --- a/drivers/staging/dgrp/dgrp_tty.c +++ b/drivers/staging/dgrp/dgrp_tty.c @@ -1120,7 +1120,9 @@ static void dgrp_tty_close(struct tty_struct *tty, struct file *file) if (!sent_printer_offstr) dgrp_tty_flush_buffer(tty); + spin_unlock_irqrestore(&nd->nd_lock, lock_flags); tty_ldisc_flush(tty); + spin_lock_irqsave(&nd->nd_lock, lock_flags); break; } diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c index 685757c6ce8..c3c606c5272 100644 --- a/drivers/tty/tty_buffer.c +++ b/drivers/tty/tty_buffer.c @@ -157,8 +157,6 @@ static void tty_buffer_free(struct tty_port *port, struct tty_buffer *b) * flush all the buffers containing receive data. Caller must * hold the buffer lock and must have ensured no parallel flush to * ldisc is running. - * - * Locking: Caller must hold tty->buf.lock */ static void __tty_buffer_flush(struct tty_port *port) @@ -182,29 +180,29 @@ static void __tty_buffer_flush(struct tty_port *port) * being processed by flush_to_ldisc then we defer the processing * to that function * - * Locking: none + * Locking: takes flush_mutex to ensure single-threaded flip buffer + * 'consumer' */ void tty_buffer_flush(struct tty_struct *tty) { struct tty_port *port = tty->port; struct tty_bufhead *buf = &port->buf; - unsigned long flags; - - spin_lock_irqsave(&buf->lock, flags); + mutex_lock(&buf->flush_mutex); /* If the data is being pushed to the tty layer then we can't process it here. Instead set a flag and the flush_to_ldisc path will process the flush request before it exits */ if (test_bit(TTYP_FLUSHING, &port->iflags)) { set_bit(TTYP_FLUSHPENDING, &port->iflags); - spin_unlock_irqrestore(&buf->lock, flags); + mutex_unlock(&buf->flush_mutex); wait_event(tty->read_wait, test_bit(TTYP_FLUSHPENDING, &port->iflags) == 0); return; - } else - __tty_buffer_flush(port); - spin_unlock_irqrestore(&buf->lock, flags); + } + + __tty_buffer_flush(port); + mutex_unlock(&buf->flush_mutex); } /** @@ -408,9 +406,10 @@ receive_buf(struct tty_struct *tty, struct tty_buffer *head, int count) * This routine is called out of the software interrupt to flush data * from the buffer chain to the line discipline. * - * Locking: holds tty->buf.lock to guard buffer list. Drops the lock - * while invoking the line discipline receive_buf method. The - * receive_buf method is single threaded for each tty instance. + * The receive_buf method is single threaded for each tty instance. + * + * Locking: takes flush_mutex to ensure single-threaded flip buffer + * 'consumer' */ static void flush_to_ldisc(struct work_struct *work) @@ -418,7 +417,6 @@ static void flush_to_ldisc(struct work_struct *work) struct tty_port *port = container_of(work, struct tty_port, buf.work); struct tty_bufhead *buf = &port->buf; struct tty_struct *tty; - unsigned long flags; struct tty_ldisc *disc; tty = port->itty; @@ -429,7 +427,7 @@ static void flush_to_ldisc(struct work_struct *work) if (disc == NULL) return; - spin_lock_irqsave(&buf->lock, flags); + mutex_lock(&buf->flush_mutex); if (!test_and_set_bit(TTYP_FLUSHING, &port->iflags)) { while (1) { @@ -444,11 +442,13 @@ static void flush_to_ldisc(struct work_struct *work) tty_buffer_free(port, head); continue; } - spin_unlock_irqrestore(&buf->lock, flags); + + mutex_unlock(&buf->flush_mutex); count = receive_buf(tty, head, count); - spin_lock_irqsave(&buf->lock, flags); + mutex_lock(&buf->flush_mutex); + /* Ldisc or user is trying to flush the buffers. We may have a deferred request to flush the input buffer, if so pull the chain under the lock @@ -464,7 +464,7 @@ static void flush_to_ldisc(struct work_struct *work) clear_bit(TTYP_FLUSHING, &port->iflags); } - spin_unlock_irqrestore(&buf->lock, flags); + mutex_unlock(&buf->flush_mutex); tty_ldisc_deref(disc); } @@ -514,15 +514,13 @@ EXPORT_SYMBOL(tty_flip_buffer_push); * * Set up the initial state of the buffer management for a tty device. * Must be called before the other tty buffer functions are used. - * - * Locking: none */ void tty_buffer_init(struct tty_port *port) { struct tty_bufhead *buf = &port->buf; - spin_lock_init(&buf->lock); + mutex_init(&buf->flush_mutex); tty_buffer_reset(&buf->sentinel, 0); buf->head = &buf->sentinel; buf->tail = &buf->sentinel; diff --git a/include/linux/tty.h b/include/linux/tty.h index 7c124541f01..1c8fef0e3ff 100644 --- a/include/linux/tty.h +++ b/include/linux/tty.h @@ -66,7 +66,7 @@ static inline char *flag_buf_ptr(struct tty_buffer *b, int ofs) struct tty_bufhead { struct work_struct work; - spinlock_t lock; + struct mutex flush_mutex; struct tty_buffer sentinel; struct tty_buffer *head; /* Queue head */ struct tty_buffer *tail; /* Active buffer */ -- cgit v1.2.3-70-g09d2 From d7a68be4f265be10e24be931c257af30ca55566b Mon Sep 17 00:00:00 2001 From: Peter Hurley Date: Sat, 15 Jun 2013 09:36:11 -0400 Subject: tty: Only perform flip buffer flush from tty_buffer_flush() Now that dropping the buffer lock is not necessary (as result of converting the spin lock to a mutex), the flip buffer flush no longer needs to be handled by the buffer work. Simply signal a flush is required; the buffer work will exit the i/o loop, which allows tty_buffer_flush() to proceed. Signed-off-by: Peter Hurley Signed-off-by: Greg Kroah-Hartman --- drivers/tty/tty_buffer.c | 63 ++++++++++++++++-------------------------------- include/linux/tty.h | 1 - 2 files changed, 21 insertions(+), 43 deletions(-) (limited to 'include/linux/tty.h') diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c index c3c606c5272..39cae611fe5 100644 --- a/drivers/tty/tty_buffer.c +++ b/drivers/tty/tty_buffer.c @@ -189,19 +189,11 @@ void tty_buffer_flush(struct tty_struct *tty) struct tty_port *port = tty->port; struct tty_bufhead *buf = &port->buf; - mutex_lock(&buf->flush_mutex); - /* If the data is being pushed to the tty layer then we can't - process it here. Instead set a flag and the flush_to_ldisc - path will process the flush request before it exits */ - if (test_bit(TTYP_FLUSHING, &port->iflags)) { - set_bit(TTYP_FLUSHPENDING, &port->iflags); - mutex_unlock(&buf->flush_mutex); - wait_event(tty->read_wait, - test_bit(TTYP_FLUSHPENDING, &port->iflags) == 0); - return; - } + set_bit(TTYP_FLUSHPENDING, &port->iflags); + mutex_lock(&buf->flush_mutex); __tty_buffer_flush(port); + clear_bit(TTYP_FLUSHPENDING, &port->iflags); mutex_unlock(&buf->flush_mutex); } @@ -429,39 +421,26 @@ static void flush_to_ldisc(struct work_struct *work) mutex_lock(&buf->flush_mutex); - if (!test_and_set_bit(TTYP_FLUSHING, &port->iflags)) { - while (1) { - struct tty_buffer *head = buf->head; - int count; - - count = head->commit - head->read; - if (!count) { - if (head->next == NULL) - break; - buf->head = head->next; - tty_buffer_free(port, head); - continue; - } - - mutex_unlock(&buf->flush_mutex); - - count = receive_buf(tty, head, count); - - mutex_lock(&buf->flush_mutex); - - /* Ldisc or user is trying to flush the buffers. - We may have a deferred request to flush the - input buffer, if so pull the chain under the lock - and empty the queue */ - if (test_bit(TTYP_FLUSHPENDING, &port->iflags)) { - __tty_buffer_flush(port); - clear_bit(TTYP_FLUSHPENDING, &port->iflags); - wake_up(&tty->read_wait); - break; - } else if (!count) + while (1) { + struct tty_buffer *head = buf->head; + int count; + + /* Ldisc or user is trying to flush the buffers. */ + if (test_bit(TTYP_FLUSHPENDING, &port->iflags)) + break; + + count = head->commit - head->read; + if (!count) { + if (head->next == NULL) break; + buf->head = head->next; + tty_buffer_free(port, head); + continue; } - clear_bit(TTYP_FLUSHING, &port->iflags); + + count = receive_buf(tty, head, count); + if (!count) + break; } mutex_unlock(&buf->flush_mutex); diff --git a/include/linux/tty.h b/include/linux/tty.h index 1c8fef0e3ff..1d5bacca365 100644 --- a/include/linux/tty.h +++ b/include/linux/tty.h @@ -213,7 +213,6 @@ struct tty_port { wait_queue_head_t delta_msr_wait; /* Modem status change */ unsigned long flags; /* TTY flags ASY_*/ unsigned long iflags; /* TTYP_ internal flags */ -#define TTYP_FLUSHING 1 /* Flushing to ldisc in progress */ #define TTYP_FLUSHPENDING 2 /* Queued buffer flush pending */ unsigned char console:1, /* port is a console */ low_latency:1; /* direct buffer flush */ -- cgit v1.2.3-70-g09d2 From 8c1fb49ba107c7db9441ef6ec0ab5830d112cc2a Mon Sep 17 00:00:00 2001 From: Peter Hurley Date: Sat, 15 Jun 2013 09:36:12 -0400 Subject: tty: Avoid false-sharing flip buffer ptrs Separate the head and tail ptrs to avoid cache-line contention (so called 'false-sharing') between concurrent threads. Signed-off-by: Peter Hurley Signed-off-by: Greg Kroah-Hartman --- include/linux/tty.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'include/linux/tty.h') diff --git a/include/linux/tty.h b/include/linux/tty.h index 1d5bacca365..b8e8adf95bf 100644 --- a/include/linux/tty.h +++ b/include/linux/tty.h @@ -65,13 +65,13 @@ static inline char *flag_buf_ptr(struct tty_buffer *b, int ofs) struct tty_bufhead { + struct tty_buffer *head; /* Queue head */ struct work_struct work; struct mutex flush_mutex; struct tty_buffer sentinel; - struct tty_buffer *head; /* Queue head */ - struct tty_buffer *tail; /* Active buffer */ struct llist_head free; /* Free queue head */ atomic_t memory_used; /* In-use buffers excluding free list */ + struct tty_buffer *tail; /* Active buffer */ }; /* * When a break, frame error, or parity error happens, these codes are -- cgit v1.2.3-70-g09d2 From 0f56bd2f6a97d8b0eb5c8f9bc04b83a6c16d1d48 Mon Sep 17 00:00:00 2001 From: Peter Hurley Date: Sat, 15 Jun 2013 09:36:13 -0400 Subject: tty: Use non-atomic state to signal flip buffer flush pending Atomic bit ops are no longer required to indicate a flip buffer flush is pending, as the flush_mutex is sufficient barrier. Remove the unnecessary port .iflags field and localize flip buffer state to struct tty_bufhead. Signed-off-by: Peter Hurley Signed-off-by: Greg Kroah-Hartman --- drivers/tty/tty_buffer.c | 7 ++++--- include/linux/tty.h | 3 +-- 2 files changed, 5 insertions(+), 5 deletions(-) (limited to 'include/linux/tty.h') diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c index 39cae611fe5..fb042b9a8d6 100644 --- a/drivers/tty/tty_buffer.c +++ b/drivers/tty/tty_buffer.c @@ -189,11 +189,11 @@ void tty_buffer_flush(struct tty_struct *tty) struct tty_port *port = tty->port; struct tty_bufhead *buf = &port->buf; - set_bit(TTYP_FLUSHPENDING, &port->iflags); + buf->flushpending = 1; mutex_lock(&buf->flush_mutex); __tty_buffer_flush(port); - clear_bit(TTYP_FLUSHPENDING, &port->iflags); + buf->flushpending = 0; mutex_unlock(&buf->flush_mutex); } @@ -426,7 +426,7 @@ static void flush_to_ldisc(struct work_struct *work) int count; /* Ldisc or user is trying to flush the buffers. */ - if (test_bit(TTYP_FLUSHPENDING, &port->iflags)) + if (buf->flushpending) break; count = head->commit - head->read; @@ -505,6 +505,7 @@ void tty_buffer_init(struct tty_port *port) buf->tail = &buf->sentinel; init_llist_head(&buf->free); atomic_set(&buf->memory_used, 0); + buf->flushpending = 0; INIT_WORK(&buf->work, flush_to_ldisc); } diff --git a/include/linux/tty.h b/include/linux/tty.h index b8e8adf95bf..991575fe345 100644 --- a/include/linux/tty.h +++ b/include/linux/tty.h @@ -68,6 +68,7 @@ struct tty_bufhead { struct tty_buffer *head; /* Queue head */ struct work_struct work; struct mutex flush_mutex; + unsigned int flushpending:1; struct tty_buffer sentinel; struct llist_head free; /* Free queue head */ atomic_t memory_used; /* In-use buffers excluding free list */ @@ -212,8 +213,6 @@ struct tty_port { wait_queue_head_t close_wait; /* Close waiters */ wait_queue_head_t delta_msr_wait; /* Modem status change */ unsigned long flags; /* TTY flags ASY_*/ - unsigned long iflags; /* TTYP_ internal flags */ -#define TTYP_FLUSHPENDING 2 /* Queued buffer flush pending */ unsigned char console:1, /* port is a console */ low_latency:1; /* direct buffer flush */ struct mutex mutex; /* Locking */ -- cgit v1.2.3-70-g09d2 From a7c8d58c79853adeebf0a1ddc9c63e433b4d97f1 Mon Sep 17 00:00:00 2001 From: Peter Hurley Date: Sat, 15 Jun 2013 09:36:15 -0400 Subject: tty: Fix unsafe vt paste_selection() Convert the tty_buffer_flush() exclusion mechanism to a public interface - tty_buffer_lock/unlock_exclusive() - and use the interface to safely write the paste selection to the line discipline. Signed-off-by: Peter Hurley Signed-off-by: Greg Kroah-Hartman --- drivers/tty/tty_buffer.c | 61 ++++++++++++++++++++++++++++++++++++---------- drivers/tty/vt/selection.c | 4 ++- include/linux/tty.h | 4 +-- include/linux/tty_flip.h | 3 +++ 4 files changed, 56 insertions(+), 16 deletions(-) (limited to 'include/linux/tty.h') diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c index dbe4a718e2d..f22e116db10 100644 --- a/drivers/tty/tty_buffer.c +++ b/drivers/tty/tty_buffer.c @@ -29,6 +29,42 @@ #define TTYB_MEM_LIMIT 65536 +/** + * tty_buffer_lock_exclusive - gain exclusive access to buffer + * tty_buffer_unlock_exclusive - release exclusive access + * + * @port - tty_port owning the flip buffer + * + * Guarantees safe use of the line discipline's receive_buf() method by + * excluding the buffer work and any pending flush from using the flip + * buffer. Data can continue to be added concurrently to the flip buffer + * from the driver side. + * + * On release, the buffer work is restarted if there is data in the + * flip buffer + */ + +void tty_buffer_lock_exclusive(struct tty_port *port) +{ + struct tty_bufhead *buf = &port->buf; + + atomic_inc(&buf->priority); + mutex_lock(&buf->lock); +} + +void tty_buffer_unlock_exclusive(struct tty_port *port) +{ + struct tty_bufhead *buf = &port->buf; + int restart; + + restart = buf->head->commit != buf->head->read; + + atomic_dec(&buf->priority); + mutex_unlock(&buf->lock); + if (restart) + queue_work(system_unbound_wq, &buf->work); +} + /** * tty_buffer_space_avail - return unused buffer space * @port - tty_port owning the flip buffer @@ -158,7 +194,7 @@ static void tty_buffer_free(struct tty_port *port, struct tty_buffer *b) * being processed by flush_to_ldisc then we defer the processing * to that function * - * Locking: takes flush_mutex to ensure single-threaded flip buffer + * Locking: takes buffer lock to ensure single-threaded flip buffer * 'consumer' */ @@ -168,16 +204,16 @@ void tty_buffer_flush(struct tty_struct *tty) struct tty_bufhead *buf = &port->buf; struct tty_buffer *next; - buf->flushpending = 1; + atomic_inc(&buf->priority); - mutex_lock(&buf->flush_mutex); + mutex_lock(&buf->lock); while ((next = buf->head->next) != NULL) { tty_buffer_free(port, buf->head); buf->head = next; } buf->head->read = buf->head->commit; - buf->flushpending = 0; - mutex_unlock(&buf->flush_mutex); + atomic_dec(&buf->priority); + mutex_unlock(&buf->lock); } /** @@ -383,7 +419,7 @@ receive_buf(struct tty_struct *tty, struct tty_buffer *head, int count) * * The receive_buf method is single threaded for each tty instance. * - * Locking: takes flush_mutex to ensure single-threaded flip buffer + * Locking: takes buffer lock to ensure single-threaded flip buffer * 'consumer' */ @@ -402,14 +438,14 @@ static void flush_to_ldisc(struct work_struct *work) if (disc == NULL) return; - mutex_lock(&buf->flush_mutex); + mutex_lock(&buf->lock); while (1) { struct tty_buffer *head = buf->head; int count; - /* Ldisc or user is trying to flush the buffers. */ - if (buf->flushpending) + /* Ldisc or user is trying to gain exclusive access */ + if (atomic_read(&buf->priority)) break; count = head->commit - head->read; @@ -426,7 +462,7 @@ static void flush_to_ldisc(struct work_struct *work) break; } - mutex_unlock(&buf->flush_mutex); + mutex_unlock(&buf->lock); tty_ldisc_deref(disc); } @@ -482,13 +518,12 @@ void tty_buffer_init(struct tty_port *port) { struct tty_bufhead *buf = &port->buf; - mutex_init(&buf->flush_mutex); + mutex_init(&buf->lock); tty_buffer_reset(&buf->sentinel, 0); buf->head = &buf->sentinel; buf->tail = &buf->sentinel; init_llist_head(&buf->free); atomic_set(&buf->memory_used, 0); - buf->flushpending = 0; + atomic_set(&buf->priority, 0); INIT_WORK(&buf->work, flush_to_ldisc); } - diff --git a/drivers/tty/vt/selection.c b/drivers/tty/vt/selection.c index 2ca8d6b6514..ea27804d87a 100644 --- a/drivers/tty/vt/selection.c +++ b/drivers/tty/vt/selection.c @@ -24,6 +24,7 @@ #include #include #include +#include /* Don't take this from : 011-015 on the screen aren't spaces */ #define isspace(c) ((c) == ' ') @@ -346,8 +347,8 @@ int paste_selection(struct tty_struct *tty) console_unlock(); ld = tty_ldisc_ref_wait(tty); + tty_buffer_lock_exclusive(&vc->port); - /* FIXME: this is completely unsafe */ add_wait_queue(&vc->paste_wait, &wait); while (sel_buffer && sel_buffer_lth > pasted) { set_current_state(TASK_INTERRUPTIBLE); @@ -363,6 +364,7 @@ int paste_selection(struct tty_struct *tty) remove_wait_queue(&vc->paste_wait, &wait); __set_current_state(TASK_RUNNING); + tty_buffer_unlock_exclusive(&vc->port); tty_ldisc_deref(ld); return 0; } diff --git a/include/linux/tty.h b/include/linux/tty.h index 991575fe345..7a9a3b0a6b5 100644 --- a/include/linux/tty.h +++ b/include/linux/tty.h @@ -67,8 +67,8 @@ static inline char *flag_buf_ptr(struct tty_buffer *b, int ofs) struct tty_bufhead { struct tty_buffer *head; /* Queue head */ struct work_struct work; - struct mutex flush_mutex; - unsigned int flushpending:1; + struct mutex lock; + atomic_t priority; struct tty_buffer sentinel; struct llist_head free; /* Free queue head */ atomic_t memory_used; /* In-use buffers excluding free list */ diff --git a/include/linux/tty_flip.h b/include/linux/tty_flip.h index 6944ed2ce69..21ddd7d9ea1 100644 --- a/include/linux/tty_flip.h +++ b/include/linux/tty_flip.h @@ -32,4 +32,7 @@ static inline int tty_insert_flip_string(struct tty_port *port, return tty_insert_flip_string_fixed_flag(port, chars, TTY_NORMAL, size); } +extern void tty_buffer_lock_exclusive(struct tty_port *port); +extern void tty_buffer_unlock_exclusive(struct tty_port *port); + #endif /* _LINUX_TTY_FLIP_H */ -- cgit v1.2.3-70-g09d2 From 9114fe8ccf1871f630d2c14cd60e5f455b015459 Mon Sep 17 00:00:00 2001 From: Peter Hurley Date: Sat, 15 Jun 2013 09:36:16 -0400 Subject: tty: Remove private constant from global namespace TTY_BUFFER_PAGE is only used within drivers/tty/tty_buffer.c; relocate to that file scope. Signed-off-by: Peter Hurley Signed-off-by: Greg Kroah-Hartman --- drivers/tty/tty_buffer.c | 10 ++++++++++ include/linux/tty.h | 11 ----------- 2 files changed, 10 insertions(+), 11 deletions(-) (limited to 'include/linux/tty.h') diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c index f22e116db10..c043136fbe5 100644 --- a/drivers/tty/tty_buffer.c +++ b/drivers/tty/tty_buffer.c @@ -28,6 +28,16 @@ */ #define TTYB_MEM_LIMIT 65536 +/* + * We default to dicing tty buffer allocations to this many characters + * in order to avoid multiple page allocations. We know the size of + * tty_buffer itself but it must also be taken into account that the + * the buffer is 256 byte aligned. See tty_buffer_find for the allocation + * logic this must match + */ + +#define TTY_BUFFER_PAGE (((PAGE_SIZE - sizeof(struct tty_buffer)) / 2) & ~0xFF) + /** * tty_buffer_lock_exclusive - gain exclusive access to buffer diff --git a/include/linux/tty.h b/include/linux/tty.h index 7a9a3b0a6b5..5fd5d6f1ebc 100644 --- a/include/linux/tty.h +++ b/include/linux/tty.h @@ -53,17 +53,6 @@ static inline char *flag_buf_ptr(struct tty_buffer *b, int ofs) return (char *)char_buf_ptr(b, ofs) + b->size; } -/* - * We default to dicing tty buffer allocations to this many characters - * in order to avoid multiple page allocations. We know the size of - * tty_buffer itself but it must also be taken into account that the - * the buffer is 256 byte aligned. See tty_buffer_find for the allocation - * logic this must match - */ - -#define TTY_BUFFER_PAGE (((PAGE_SIZE - sizeof(struct tty_buffer)) / 2) & ~0xFF) - - struct tty_bufhead { struct tty_buffer *head; /* Queue head */ struct work_struct work; -- cgit v1.2.3-70-g09d2 From 40d5e0905a03601d40cd4e46b8690093c2355d03 Mon Sep 17 00:00:00 2001 From: Peter Hurley Date: Sat, 15 Jun 2013 10:21:17 -0400 Subject: n_tty: Fix EOF push handling In canonical mode, an EOF which is not the first character of the line causes read() to complete and return the number of characters read so far (commonly referred to as EOF push). However, if the previous read() returned because the user buffer was full _and_ the next character is an EOF not at the beginning of the line, read() must not return 0, thus mistakenly indicating the end-of-file condition. The TTY_PUSH flag is used to indicate an EOF was received which is not at the beginning of the line. Because the EOF push condition is evaluated by a thread other than the read(), multiple EOF pushes can cause a premature end-of-file to be indicated. Instead, discover the 'EOF push as first read character' condition from the read() thread itself, and restart the i/o loop if detected. Signed-off-by: Peter Hurley Signed-off-by: Greg Kroah-Hartman --- drivers/tty/n_tty.c | 34 +++++++++++++++++----------------- drivers/tty/tty_io.c | 1 - include/linux/tty.h | 1 - 3 files changed, 17 insertions(+), 19 deletions(-) (limited to 'include/linux/tty.h') diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c index d4d71350a71..25aaf1afa22 100644 --- a/drivers/tty/n_tty.c +++ b/drivers/tty/n_tty.c @@ -113,6 +113,7 @@ struct n_tty_data { /* consumer-published */ size_t read_tail; + size_t line_start; /* protected by output lock */ unsigned int column; @@ -337,6 +338,7 @@ static void reset_buffer_flags(struct n_tty_data *ldata) { ldata->read_head = ldata->canon_head = ldata->read_tail = 0; ldata->echo_head = ldata->echo_tail = ldata->echo_commit = 0; + ldata->line_start = 0; ldata->erasing = 0; bitmap_zero(ldata->read_flags, N_TTY_BUF_SIZE); @@ -1396,8 +1398,6 @@ send_signal: if (c == EOF_CHAR(tty)) { if (read_cnt(ldata) >= N_TTY_BUF_SIZE) return; - if (ldata->canon_head != ldata->read_head) - set_bit(TTY_PUSH, &tty->flags); c = __DISABLED_CHAR; goto handle_newline; } @@ -1604,6 +1604,7 @@ static void n_tty_set_termios(struct tty_struct *tty, struct ktermios *old) canon_change = (old->c_lflag ^ tty->termios.c_lflag) & ICANON; if (canon_change) { bitmap_zero(ldata->read_flags, N_TTY_BUF_SIZE); + ldata->line_start = 0; ldata->canon_head = ldata->read_tail; ldata->erasing = 0; ldata->lnext = 0; @@ -1837,6 +1838,7 @@ static int canon_copy_from_read_buf(struct tty_struct *tty, size_t eol; size_t tail; int ret, found = 0; + bool eof_push = 0; /* N.B. avoid overrun if nr == 0 */ n = min(*nr, read_cnt(ldata)); @@ -1863,8 +1865,10 @@ static int canon_copy_from_read_buf(struct tty_struct *tty, n = (found + eol + size) & (N_TTY_BUF_SIZE - 1); c = n; - if (found && read_buf(ldata, eol) == __DISABLED_CHAR) + if (found && read_buf(ldata, eol) == __DISABLED_CHAR) { n--; + eof_push = !n && ldata->read_tail != ldata->line_start; + } n_tty_trace("%s: eol:%zu found:%d n:%zu c:%zu size:%zu more:%zu\n", __func__, eol, found, n, c, size, more); @@ -1887,9 +1891,11 @@ static int canon_copy_from_read_buf(struct tty_struct *tty, smp_mb__after_clear_bit(); ldata->read_tail += c; - if (found) + if (found) { + ldata->line_start = ldata->read_tail; tty_audit_push(tty); - return 0; + } + return eof_push ? -EAGAIN : 0; } extern ssize_t redirected_tty_write(struct file *, const char __user *, @@ -1964,12 +1970,10 @@ static ssize_t n_tty_read(struct tty_struct *tty, struct file *file, int c; int minimum, time; ssize_t retval = 0; - ssize_t size; long timeout; unsigned long flags; int packet; -do_it_again: c = job_control(tty, file); if (c < 0) return c; @@ -2076,7 +2080,10 @@ do_it_again: if (ldata->icanon && !L_EXTPROC(tty)) { retval = canon_copy_from_read_buf(tty, &b, &nr); - if (retval) + if (retval == -EAGAIN) { + retval = 0; + continue; + } else if (retval) break; } else { int uncopied; @@ -2104,15 +2111,8 @@ do_it_again: ldata->minimum_to_wake = minimum; __set_current_state(TASK_RUNNING); - size = b - buf; - if (size) { - retval = size; - if (nr) - clear_bit(TTY_PUSH, &tty->flags); - } else if (test_and_clear_bit(TTY_PUSH, &tty->flags)) { - up_read(&tty->termios_rwsem); - goto do_it_again; - } + if (b - buf) + retval = b - buf; n_tty_set_room(tty); up_read(&tty->termios_rwsem); diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c index 1b32da6f6cd..2174698dd6f 100644 --- a/drivers/tty/tty_io.c +++ b/drivers/tty/tty_io.c @@ -664,7 +664,6 @@ static void __tty_hangup(struct tty_struct *tty, int exit_session) spin_lock_irq(&tty->ctrl_lock); clear_bit(TTY_THROTTLED, &tty->flags); - clear_bit(TTY_PUSH, &tty->flags); clear_bit(TTY_DO_WRITE_WAKEUP, &tty->flags); put_pid(tty->session); put_pid(tty->pgrp); diff --git a/include/linux/tty.h b/include/linux/tty.h index 5fd5d6f1ebc..554b732d8b5 100644 --- a/include/linux/tty.h +++ b/include/linux/tty.h @@ -304,7 +304,6 @@ struct tty_file_private { #define TTY_EXCLUSIVE 3 /* Exclusive open mode */ #define TTY_DEBUG 4 /* Debugging */ #define TTY_DO_WRITE_WAKEUP 5 /* Call write_wakeup after queuing new */ -#define TTY_PUSH 6 /* n_tty private */ #define TTY_CLOSING 7 /* ->close() in progress */ #define TTY_LDISC_OPEN 11 /* Line discipline is open */ #define TTY_PTY_LOCK 16 /* pty private */ -- cgit v1.2.3-70-g09d2 From dee4a0be69c0e2996188e0c46478eadc280a8954 Mon Sep 17 00:00:00 2001 From: Peter Hurley Date: Wed, 24 Jul 2013 16:43:51 -0400 Subject: tty: Fix lock order in tty_do_resize() Commits 6a1c0680cf3ba94356ecd58833e1540c93472a57 and 9356b535fcb71db494fc434acceb79f56d15bda2, respectively 'tty: Convert termios_mutex to termios_rwsem' and 'n_tty: Access termios values safely' introduced a circular lock dependency with console_lock and termios_rwsem. The lockdep report [1] shows that n_tty_write() will attempt to claim console_lock while holding the termios_rwsem, whereas tty_do_resize() may already hold the console_lock while claiming the termios_rwsem. Since n_tty_write() and tty_do_resize() do not contend over the same data -- the tty->winsize structure -- correct the lock dependency by introducing a new lock which specifically serializes access to tty->winsize only. [1] Lockdep report ====================================================== [ INFO: possible circular locking dependency detected ] 3.10.0-0+tip-xeon+lockdep #0+tip Not tainted ------------------------------------------------------- modprobe/277 is trying to acquire lock: (&tty->termios_rwsem){++++..}, at: [] tty_do_resize+0x36/0xe0 but task is already holding lock: ((fb_notifier_list).rwsem){.+.+.+}, at: [] __blocking_notifier_call_chain+0x56/0xc0 which lock already depends on the new lock. the existing dependency chain (in reverse order) is: -> #2 ((fb_notifier_list).rwsem){.+.+.+}: [] lock_acquire+0x92/0x1f0 [] down_read+0x47/0x5c [] __blocking_notifier_call_chain+0x56/0xc0 [] blocking_notifier_call_chain+0x16/0x20 [] fb_notifier_call_chain+0x1b/0x20 [] register_framebuffer+0x1e2/0x320 [] drm_fb_helper_initial_config+0x371/0x540 [drm_kms_helper] [] nouveau_fbcon_init+0x105/0x140 [nouveau] [] nouveau_drm_load+0x43f/0x610 [nouveau] [] drm_get_pci_dev+0x17e/0x2a0 [drm] [] nouveau_drm_probe+0x25a/0x2a0 [nouveau] [] local_pci_probe+0x4b/0x80 [] pci_device_probe+0x111/0x120 [] driver_probe_device+0x8b/0x3a0 [] __driver_attach+0xab/0xb0 [] bus_for_each_dev+0x5d/0xa0 [] driver_attach+0x1e/0x20 [] bus_add_driver+0x111/0x290 [] driver_register+0x77/0x170 [] __pci_register_driver+0x64/0x70 [] drm_pci_init+0x11a/0x130 [drm] [] nouveau_drm_init+0x4d/0x1000 [nouveau] [] do_one_initcall+0xea/0x1a0 [] load_module+0x123b/0x1bf0 [] SyS_init_module+0xd7/0x120 [] system_call_fastpath+0x16/0x1b -> #1 (console_lock){+.+.+.}: [] lock_acquire+0x92/0x1f0 [] console_lock+0x77/0x80 [] con_flush_chars+0x31/0x50 [] n_tty_write+0x1ec/0x4d0 [] tty_write+0x159/0x2e0 [] redirected_tty_write+0xb5/0xc0 [] vfs_write+0xc5/0x1f0 [] SyS_write+0x55/0xa0 [] system_call_fastpath+0x16/0x1b -> #0 (&tty->termios_rwsem){++++..}: [] __lock_acquire+0x1c43/0x1d30 [] lock_acquire+0x92/0x1f0 [] down_write+0x44/0x70 [] tty_do_resize+0x36/0xe0 [] vc_do_resize+0x3e1/0x4c0 [] vc_resize+0x1f/0x30 [] fbcon_init+0x385/0x5a0 [] visual_init+0xbc/0x120 [] do_bind_con_driver+0x163/0x320 [] do_take_over_console+0x61/0x70 [] do_fbcon_takeover+0x63/0xc0 [] fbcon_event_notify+0x715/0x820 [] notifier_call_chain+0x5d/0x110 [] __blocking_notifier_call_chain+0x6c/0xc0 [] blocking_notifier_call_chain+0x16/0x20 [] fb_notifier_call_chain+0x1b/0x20 [] register_framebuffer+0x1e2/0x320 [] drm_fb_helper_initial_config+0x371/0x540 [drm_kms_helper] [] nouveau_fbcon_init+0x105/0x140 [nouveau] [] nouveau_drm_load+0x43f/0x610 [nouveau] [] drm_get_pci_dev+0x17e/0x2a0 [drm] [] nouveau_drm_probe+0x25a/0x2a0 [nouveau] [] local_pci_probe+0x4b/0x80 [] pci_device_probe+0x111/0x120 [] driver_probe_device+0x8b/0x3a0 [] __driver_attach+0xab/0xb0 [] bus_for_each_dev+0x5d/0xa0 [] driver_attach+0x1e/0x20 [] bus_add_driver+0x111/0x290 [] driver_register+0x77/0x170 [] __pci_register_driver+0x64/0x70 [] drm_pci_init+0x11a/0x130 [drm] [] nouveau_drm_init+0x4d/0x1000 [nouveau] [] do_one_initcall+0xea/0x1a0 [] load_module+0x123b/0x1bf0 [] SyS_init_module+0xd7/0x120 [] system_call_fastpath+0x16/0x1b other info that might help us debug this: Chain exists of: &tty->termios_rwsem --> console_lock --> (fb_notifier_list).rwsem Possible unsafe locking scenario: CPU0 CPU1 ---- ---- lock((fb_notifier_list).rwsem); lock(console_lock); lock((fb_notifier_list).rwsem); lock(&tty->termios_rwsem); *** DEADLOCK *** 7 locks held by modprobe/277: #0: (&__lockdep_no_validate__){......}, at: [] __driver_attach+0x5b/0xb0 #1: (&__lockdep_no_validate__){......}, at: [] __driver_attach+0x69/0xb0 #2: (drm_global_mutex){+.+.+.}, at: [] drm_get_pci_dev+0xbd/0x2a0 [drm] #3: (registration_lock){+.+.+.}, at: [] register_framebuffer+0x25/0x320 #4: (&fb_info->lock){+.+.+.}, at: [] lock_fb_info+0x26/0x60 #5: (console_lock){+.+.+.}, at: [] register_framebuffer+0x1d4/0x320 #6: ((fb_notifier_list).rwsem){.+.+.+}, at: [] __blocking_notifier_call_chain+0x56/0xc0 stack backtrace: CPU: 0 PID: 277 Comm: modprobe Not tainted 3.10.0-0+tip-xeon+lockdep #0+tip Hardware name: Dell Inc. Precision WorkStation T5400 /0RW203, BIOS A11 04/30/2012 ffffffff8213e5e0 ffff8802aa2fb298 ffffffff81755f19 ffff8802aa2fb2e8 ffffffff8174f506 ffff8802aa2fa000 ffff8802aa2fb378 ffff8802aa2ea8e8 ffff8802aa2ea910 ffff8802aa2ea8e8 0000000000000006 0000000000000007 Call Trace: [] dump_stack+0x19/0x1b [] print_circular_bug+0x1fb/0x20c [] __lock_acquire+0x1c43/0x1d30 [] ? mark_held_locks+0xae/0x120 [] ? trace_hardirqs_on_caller+0x105/0x1d0 [] lock_acquire+0x92/0x1f0 [] ? tty_do_resize+0x36/0xe0 [] down_write+0x44/0x70 [] ? tty_do_resize+0x36/0xe0 [] tty_do_resize+0x36/0xe0 [] vc_do_resize+0x3e1/0x4c0 [] vc_resize+0x1f/0x30 [] fbcon_init+0x385/0x5a0 [] visual_init+0xbc/0x120 [] do_bind_con_driver+0x163/0x320 [] do_take_over_console+0x61/0x70 [] do_fbcon_takeover+0x63/0xc0 [] fbcon_event_notify+0x715/0x820 [] notifier_call_chain+0x5d/0x110 [] __blocking_notifier_call_chain+0x6c/0xc0 [] blocking_notifier_call_chain+0x16/0x20 [] fb_notifier_call_chain+0x1b/0x20 [] register_framebuffer+0x1e2/0x320 [] drm_fb_helper_initial_config+0x371/0x540 [drm_kms_helper] [] ? kmemleak_alloc+0x5b/0xc0 [] ? kmem_cache_alloc_trace+0x104/0x290 [] ? drm_fb_helper_single_add_all_connectors+0x81/0xf0 [drm_kms_helper] [] nouveau_fbcon_init+0x105/0x140 [nouveau] [] nouveau_drm_load+0x43f/0x610 [nouveau] [] drm_get_pci_dev+0x17e/0x2a0 [drm] [] nouveau_drm_probe+0x25a/0x2a0 [nouveau] [] ? _raw_spin_unlock_irqrestore+0x42/0x80 [] local_pci_probe+0x4b/0x80 [] pci_device_probe+0x111/0x120 [] driver_probe_device+0x8b/0x3a0 [] __driver_attach+0xab/0xb0 [] ? driver_probe_device+0x3a0/0x3a0 [] bus_for_each_dev+0x5d/0xa0 [] driver_attach+0x1e/0x20 [] bus_add_driver+0x111/0x290 [] ? 0xffffffffa0229fff [] driver_register+0x77/0x170 [] ? 0xffffffffa0229fff [] __pci_register_driver+0x64/0x70 [] drm_pci_init+0x11a/0x130 [drm] [] ? 0xffffffffa0229fff [] ? 0xffffffffa0229fff [] nouveau_drm_init+0x4d/0x1000 [nouveau] [] do_one_initcall+0xea/0x1a0 [] load_module+0x123b/0x1bf0 [] ? ddebug_proc_open+0xb0/0xb0 [] ? trace_hardirqs_on_thunk+0x3a/0x3f [] SyS_init_module+0xd7/0x120 [] system_call_fastpath+0x16/0x1b Signed-off-by: Peter Hurley Signed-off-by: Greg Kroah-Hartman --- drivers/tty/pty.c | 4 ++-- drivers/tty/tty_io.c | 11 ++++++----- include/linux/tty.h | 3 ++- 3 files changed, 10 insertions(+), 8 deletions(-) (limited to 'include/linux/tty.h') diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c index b940127ba1c..25c9bc78372 100644 --- a/drivers/tty/pty.c +++ b/drivers/tty/pty.c @@ -281,7 +281,7 @@ static 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 */ - down_write(&tty->termios_rwsem); + mutex_lock(&tty->winsize_mutex); if (!memcmp(ws, &tty->winsize, sizeof(*ws))) goto done; @@ -308,7 +308,7 @@ static int pty_resize(struct tty_struct *tty, struct winsize *ws) tty->winsize = *ws; pty->winsize = *ws; /* Never used so will go away soon */ done: - up_write(&tty->termios_rwsem); + mutex_unlock(&tty->winsize_mutex); return 0; } diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c index 2174698dd6f..26bb78c30a0 100644 --- a/drivers/tty/tty_io.c +++ b/drivers/tty/tty_io.c @@ -2229,7 +2229,7 @@ static int tiocsti(struct tty_struct *tty, char __user *p) * * Copies the kernel idea of the window size into the user buffer. * - * Locking: tty->termios_rwsem is taken to ensure the winsize data + * Locking: tty->winsize_mutex is taken to ensure the winsize data * is consistent. */ @@ -2237,9 +2237,9 @@ static int tiocgwinsz(struct tty_struct *tty, struct winsize __user *arg) { int err; - down_read(&tty->termios_rwsem); + mutex_lock(&tty->winsize_mutex); err = copy_to_user(arg, &tty->winsize, sizeof(*arg)); - up_read(&tty->termios_rwsem); + mutex_unlock(&tty->winsize_mutex); return err ? -EFAULT: 0; } @@ -2260,7 +2260,7 @@ int tty_do_resize(struct tty_struct *tty, struct winsize *ws) unsigned long flags; /* Lock the tty */ - down_write(&tty->termios_rwsem); + mutex_lock(&tty->winsize_mutex); if (!memcmp(ws, &tty->winsize, sizeof(*ws))) goto done; /* Get the PID values and reference them so we can @@ -2275,7 +2275,7 @@ int tty_do_resize(struct tty_struct *tty, struct winsize *ws) tty->winsize = *ws; done: - up_write(&tty->termios_rwsem); + mutex_unlock(&tty->winsize_mutex); return 0; } EXPORT_SYMBOL(tty_do_resize); @@ -3016,6 +3016,7 @@ void initialize_tty_struct(struct tty_struct *tty, mutex_init(&tty->legacy_mutex); mutex_init(&tty->throttle_mutex); init_rwsem(&tty->termios_rwsem); + mutex_init(&tty->winsize_mutex); init_ldsem(&tty->ldisc_sem); init_waitqueue_head(&tty->write_wait); init_waitqueue_head(&tty->read_wait); diff --git a/include/linux/tty.h b/include/linux/tty.h index 554b732d8b5..64f864651d8 100644 --- a/include/linux/tty.h +++ b/include/linux/tty.h @@ -245,6 +245,7 @@ struct tty_struct { struct mutex legacy_mutex; struct mutex throttle_mutex; struct rw_semaphore termios_rwsem; + struct mutex winsize_mutex; spinlock_t ctrl_lock; /* Termios values are protected by the termios rwsem */ struct ktermios termios, termios_locked; @@ -254,7 +255,7 @@ struct tty_struct { struct pid *session; unsigned long flags; int count; - struct winsize winsize; /* termios rwsem */ + struct winsize winsize; /* winsize_mutex */ unsigned char stopped:1, hw_stopped:1, flow_stopped:1, packet:1; unsigned char ctrl_status; /* ctrl_lock */ unsigned int receive_room; /* Bytes free for queue */ -- cgit v1.2.3-70-g09d2