tty changes for the locking warnings.
cover tty changes for the locking warnings.
Fix the tty locking warnings. This doesn't include the patches to the lock markup as I'm not sure which are the final valid ones there.
---
Alan Cox (2):
tty: fix ldisc lock inversion trace
pty: Fix lock inversion
drivers/tty/pty.c | 2 -- drivers/tty/tty_ldisc.c | 41 +++++++++++++++++++++++++---------------- 2 files changed, 25 insertions(+), 18 deletions(-)
PATCH 1/2 pty: Fix lock inversion
From: Alan Cox <[email protected]>
The ptmx_open path takes the tty and devpts locks in the wrong order because tty_init_dev locks and returns a locked tty. As far as I can tell this is actually safe anyway because the tty being returned is new so nobody can get a reference to lock it at this point.
However we don't even need the devpts lock at this point, it's only held as a byproduct of the way the locks were pushed down.
---
drivers/tty/pty.c | 2 -- 1 files changed, 0 insertions(+), 2 deletions(-)
drivers/tty/pty.c +0 -2
@@ -633,7 +633,6 @@ static int ptmx_open(struct inode *inode, struct file *filp)mutex_unlock (&devpts_mutex);mutex_lock (&tty_mutex); -mutex_lock (&devpts_mutex); tty =tty_init_dev (ptm_driver, index);if (IS_ERR (tty)) {
@@ -643,7 +642,6 @@ static int ptmx_open(struct inode *inode, struct file *filp)/* The tty returned here is locked so we can safely drop the mutex */ -mutex_unlock (&devpts_mutex);mutex_unlock (&tty_mutex);set_bit (TTY_PTY_LOCK , &tty->flags);/* LOCK THE SLAVE */
PATCH 2/2 tty: fix ldisc lock inversion trace
In reply to Alan CoxFrom: Alan Cox <[email protected]>
This is caused by tty_release using tty_lock_pair to lock both sides of the pty/tty pair, and then tty_ldisc_release dropping and relocking one side only. We can drop both fine, so drop both to avoid any lock ordering concerns.
Rework the release path to fix the new locking model.
---
drivers/tty/tty_ldisc.c | 41 +++++++++++++++++++++++++---------------- 1 files changed, 25 insertions(+), 16 deletions(-)
drivers/tty/tty_ldisc.c +25 -16
@@ -894,6 +894,23 @@ int tty_ldisc_setup(struct tty_struct *tty, struct tty_struct *o_tty)tty_ldisc_enable (tty);return 0 ; } + +static void tty_ldisc_kill (struct tty_struct *tty) +{ +mutex_lock (&tty->ldisc_mutex); +/* +* Now kill off the ldisc +*/ +tty_ldisc_close (tty, tty->ldisc); +tty_ldisc_put (tty->ldisc); +/* Force an oops if we mess this up */ + tty->ldisc =NULL ; + +/* Ensure the next open requests the N_TTY ldisc */ +tty_set_termios_ldisc (tty,N_TTY ); +mutex_unlock (&tty->ldisc_mutex); +} +/** * tty_ldisc_release - release line discipline * @tty: tty being shut down
@@ -912,27 +929,19 @@ void tty_ldisc_release(struct tty_struct *tty, struct tty_struct *o_tty)* race with the set_ldisc code path. */ -tty_unlock (tty); +tty_unlock_pair (tty, o_tty);tty_ldisc_halt (tty);tty_ldisc_flush_works (tty); -tty_lock (tty); - -mutex_lock (&tty->ldisc_mutex); -/* -* Now kill off the ldisc -*/ -tty_ldisc_close (tty, tty->ldisc); -tty_ldisc_put (tty->ldisc); -/* Force an oops if we mess this up */ - tty->ldisc =NULL ; +if (o_tty) { +tty_ldisc_halt (o_tty); +tty_ldisc_flush_works (o_tty); + } +tty_lock_pair (tty, o_tty); -/* Ensure the next open requests the N_TTY ldisc */ -tty_set_termios_ldisc (tty,N_TTY ); -mutex_unlock (&tty->ldisc_mutex); -/* This will need doing differently if we need to lock */ +tty_ldisc_kill (tty);if (o_tty) -tty_ldisc_release (o_tty,NULL ); +tty_ldisc_kill (o_tty);/* And the memory resources remaining (buffers, termios) will be disposed of when the kref hits zero */
Quoted 2 lines · Alan Cox
Fix the tty locking warnings. This doesn't include the patches to the lock markup as I'm not sure which are the final valid ones there.
So this should take care of the issues people have reported? Or are Peter's patches still needed?
thanks,
greg k-h
Quoted 6 lines · On Mon, 28 May 2012 20:29:06 +0900 Greg KH <[email protected]> wrote:
Quoted 2 lines · Alan Cox
Fix the tty locking warnings. This doesn't include the patches to the lock markup as I'm not sure which are the final valid ones there.
So this should take care of the issues people have reported? Or are Peter's patches still needed?
These fix all the known actual locking issues. I'm not clear if one of the other various mark up patches are also needed to babysit the lockdep analyser
Quoted 7 lines · Alan Cox
Quoted 4 lines · On Mon, 28 May 2012 20:29:06 +0900 Greg KH <[email protected]> wrote:
Quoted 2 lines · Alan Cox
Fix the tty locking warnings. This doesn't include the patches to the lock markup as I'm not sure which are the final valid ones there.
Quoted 7 lines
Quoted 2 lines
So this should take care of the issues people have reported? Or are Peter's patches still needed?
These fix all the known actual locking issues. I'm not clear if one of the other various mark up patches are also needed to babysit the lockdep analyser
Ok, great, thanks for fixing this up.
But, it turns out I don't think I'll have internet access at all for the next 4 days, due to travel arrangements, so can you send these directly to Linus before 3.5-rc1 is out so they get in and we don't see a zillion people complaining about it?
Feel free to add:
to them.
thanks,
greg k-h