From aefceaf453024ef4cf8a0f93c4b1edf6c6f748bf Mon Sep 17 00:00:00 2001 From: Peter Hurley Date: Sun, 11 Aug 2013 08:04:23 -0400 Subject: [PATCH] n_tty: Fix termios_rwsem lockdep false positive Lockdep reports a circular lock dependency between atomic_read_lock and termios_rwsem [1]. However, a lock order deadlock is not possible since CPU1 only holds a read lock which cannot prevent CPU0 from also acquiring a read lock on the same r/w semaphore. Unfortunately, lockdep cannot currently distinguish whether the locks are read or write for any particular lock graph, merely that the locks _were_ previously read and/or write. Until lockdep is fixed, re-order atomic_read_lock so termios_rwsem can be dropped and reacquired without triggering lockdep. Patch based on original posted here https://lkml.org/lkml/2013/8/1/510 by Sergey Senozhatsky [1] Initial lockdep report from Artem Savkov ====================================================== [ INFO: possible circular locking dependency detected ] 3.11.0-rc3-next-20130730+ #140 Tainted: G W ------------------------------------------------------- bash/1198 is trying to acquire lock: (&tty->termios_rwsem){++++..}, at: [] n_tty_read+0x49b/0x660 but task is already holding lock: (&ldata->atomic_read_lock){+.+...}, at: [] n_tty_read+0x1d0/0x660 which lock already depends on the new lock. the existing dependency chain (in reverse order) is: -> #1 (&ldata->atomic_read_lock){+.+...}: [] validate_chain+0x73c/0x850 [] __lock_acquire+0x500/0x5d0 [] lock_acquire+0x179/0x1d0 [] mutex_lock_interruptible_nested+0x7c/0x540 [] n_tty_read+0x1d0/0x660 [] tty_read+0x86/0xf0 [] vfs_read+0xc3/0x130 [] SyS_read+0x62/0xa0 [] system_call_fastpath+0x16/0x1b -> #0 (&tty->termios_rwsem){++++..}: [] check_prev_add+0x14f/0x590 [] validate_chain+0x73c/0x850 [] __lock_acquire+0x500/0x5d0 [] lock_acquire+0x179/0x1d0 [] down_read+0x51/0xa0 [] n_tty_read+0x49b/0x660 [] tty_read+0x86/0xf0 [] vfs_read+0xc3/0x130 [] SyS_read+0x62/0xa0 [] system_call_fastpath+0x16/0x1b other info that might help us debug this: Possible unsafe locking scenario: CPU0 CPU1 ---- ---- lock(&ldata->atomic_read_lock); lock(&tty->termios_rwsem); lock(&ldata->atomic_read_lock); lock(&tty->termios_rwsem); *** DEADLOCK *** 2 locks held by bash/1198: #0: (&tty->ldisc_sem){.+.+.+}, at: [] tty_ldisc_ref_wait+0x24/0x60 #1: (&ldata->atomic_read_lock){+.+...}, at: [] n_tty_read+0x1d0/0x660 stack backtrace: CPU: 1 PID: 1198 Comm: bash Tainted: G W 3.11.0-rc3-next-20130730+ #140 Hardware name: Bochs Bochs, BIOS Bochs 01/01/2007 0000000000000000 ffff880019acdb28 ffffffff81d34074 0000000000000002 0000000000000000 ffff880019acdb78 ffffffff8110ed75 ffff880019acdb98 ffff880019fd0000 ffff880019acdb78 ffff880019fd0638 ffff880019fd0670 Call Trace: [] dump_stack+0x59/0x7d [] print_circular_bug+0x105/0x120 [] check_prev_add+0x14f/0x590 [] ? _raw_spin_unlock_irq+0x4f/0x70 [] validate_chain+0x73c/0x850 [] ? trace_hardirqs_off_caller+0x1f/0x190 [] __lock_acquire+0x500/0x5d0 [] lock_acquire+0x179/0x1d0 [] ? n_tty_read+0x49b/0x660 [] down_read+0x51/0xa0 [] ? n_tty_read+0x49b/0x660 [] n_tty_read+0x49b/0x660 [] ? try_to_wake_up+0x210/0x210 [] tty_read+0x86/0xf0 [] vfs_read+0xc3/0x130 [] SyS_read+0x62/0xa0 [] ? trace_hardirqs_on_thunk+0x3a/0x3f [] system_call_fastpath+0x16/0x1b Reported-by: Artem Savkov Reported-by: Sergey Senozhatsky Signed-off-by: Peter Hurley Signed-off-by: Greg Kroah-Hartman --- drivers/tty/n_tty.c | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c index dd8ae0cad1d5..c9a9ddd1d0bc 100644 --- a/drivers/tty/n_tty.c +++ b/drivers/tty/n_tty.c @@ -2122,6 +2122,17 @@ static ssize_t n_tty_read(struct tty_struct *tty, struct file *file, if (c < 0) return c; + /* + * Internal serialization of reads. + */ + if (file->f_flags & O_NONBLOCK) { + if (!mutex_trylock(&ldata->atomic_read_lock)) + return -EAGAIN; + } else { + if (mutex_lock_interruptible(&ldata->atomic_read_lock)) + return -ERESTARTSYS; + } + down_read(&tty->termios_rwsem); minimum = time = 0; @@ -2141,20 +2152,6 @@ static ssize_t n_tty_read(struct tty_struct *tty, struct file *file, } } - /* - * Internal serialization of reads. - */ - if (file->f_flags & O_NONBLOCK) { - if (!mutex_trylock(&ldata->atomic_read_lock)) { - up_read(&tty->termios_rwsem); - return -EAGAIN; - } - } else { - if (mutex_lock_interruptible(&ldata->atomic_read_lock)) { - up_read(&tty->termios_rwsem); - return -ERESTARTSYS; - } - } packet = tty->packet; add_wait_queue(&tty->read_wait, &wait);