Re: tty related lockdep trace during bootup on 3.2-rc2

From: Cong Wang
Date: Wed Nov 23 2011 - 02:29:33 EST


On Wed, Nov 23, 2011 at 11:38 AM, Dave Jones <davej@xxxxxxxxxx> wrote:
> From Linus' current tree...
>
> related to 5dc2470c602da8851907ec18942cd876c3b4ecc1 maybe ?
>
> Â Â Â ÂDave
>
> [ Â 40.778011] ======================================================
> [ Â 40.780010] [ INFO: possible circular locking dependency detected ]
> [ Â 40.780010] 3.2.0-rc2+ #8
> [ Â 40.780010] -------------------------------------------------------
> [ Â 40.780010] modem-manager/1141 is trying to acquire lock:
> [ Â 40.780010] Â(big_tty_mutex){+.+.+.}, at: [<ffffffff81682807>] tty_lock+0x17/0x19
> [ Â 40.780010]
> [ Â 40.780010] but task is already holding lock:
> [ Â 40.780010] Â(open_mutex){+.+...}, at: [<ffffffffa032b321>] acm_tty_close+0x41/0xc0 [cdc_acm]
> [ Â 40.780010]
> [ Â 40.780010] which lock already depends on the new lock.
> [ Â 40.780010]
> [ Â 40.780010]
> [ Â 40.780010] the existing dependency chain (in reverse order) is:
> [ Â 40.780010]
> [ Â 40.780010] -> #1 (open_mutex){+.+...}:
> [ Â 40.780010] Â Â Â Â[<ffffffff810c1d9d>] lock_acquire+0x9d/0x210
> [ Â 40.780010] Â Â Â Â[<ffffffff8168003e>] __mutex_lock_common+0x5e/0x4f0
> [ Â 40.780010] Â Â Â Â[<ffffffff81680604>] mutex_lock_nested+0x44/0x50
> [ Â 40.780010] Â Â Â Â[<ffffffffa032b715>] acm_tty_open+0x35/0x230 [cdc_acm]
> [ Â 40.780010] Â Â Â Â[<ffffffff813c1087>] tty_open+0x247/0x5d0
> [ Â 40.780010] Â Â Â Â[<ffffffff811b5408>] chrdev_open+0x258/0x350
> [ Â 40.780010] Â Â Â Â[<ffffffff811ad9a4>] __dentry_open+0x384/0x550
> [ Â 40.780010] Â Â Â Â[<ffffffff811af194>] nameidata_to_filp+0x74/0x80
> [ Â 40.780010] Â Â Â Â[<ffffffff811c0b6c>] do_last+0x26c/0x920
> [ Â 40.780010] Â Â Â Â[<ffffffff811c1335>] path_openat+0xd5/0x3e0
> [ Â 40.780010] Â Â Â Â[<ffffffff811c1762>] do_filp_open+0x42/0xa0
> [ Â 40.780010] Â Â Â Â[<ffffffff811af297>] do_sys_open+0xf7/0x1d0
> [ Â 40.780010] Â Â Â Â[<ffffffff811af390>] sys_open+0x20/0x30
> [ Â 40.780010] Â Â Â Â[<ffffffff81689f82>] system_call_fastpath+0x16/0x1b
> [ Â 40.780010]
> [ Â 40.780010] -> #0 (big_tty_mutex){+.+.+.}:
> [ Â 40.780010] Â Â Â Â[<ffffffff810c112e>] __lock_acquire+0x16ce/0x1c40
> [ Â 40.780010] Â Â Â Â[<ffffffff810c1d9d>] lock_acquire+0x9d/0x210
> [ Â 40.780010] Â Â Â Â[<ffffffff8168003e>] __mutex_lock_common+0x5e/0x4f0
> [ Â 40.780010] Â Â Â Â[<ffffffff81680604>] mutex_lock_nested+0x44/0x50
> [ Â 40.780010] Â Â Â Â[<ffffffff81682807>] tty_lock+0x17/0x19
> [ Â 40.780010] Â Â Â Â[<ffffffff813c91ad>] tty_port_close_start+0x17d/0x210
> [ Â 40.780010] Â Â Â Â[<ffffffffa032b32f>] acm_tty_close+0x4f/0xc0 [cdc_acm]
> [ Â 40.780010] Â Â Â Â[<ffffffff813c09e7>] tty_release+0x167/0x5c0
> [ Â 40.780010] Â Â Â Â[<ffffffff811b252e>] fput+0xfe/0x2d0
> [ Â 40.780010] Â Â Â Â[<ffffffff811adee9>] filp_close+0x69/0x90
> [ Â 40.780010] Â Â Â Â[<ffffffff811ae1d0>] sys_close+0xc0/0x1a0
> [ Â 40.780010] Â Â Â Â[<ffffffff81689f82>] system_call_fastpath+0x16/0x1b
> [ Â 40.780010]
> [ Â 40.780010] other info that might help us debug this:
> [ Â 40.780010]
> [ Â 40.780010] ÂPossible unsafe locking scenario:
> [ Â 40.780010]
> [ Â 40.780010] Â Â Â ÂCPU0 Â Â Â Â Â Â Â Â Â ÂCPU1
> [ Â 40.780010] Â Â Â Â---- Â Â Â Â Â Â Â Â Â Â----
> [ Â 40.780010] Â lock(open_mutex);
> [ Â 40.780010] Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âlock(big_tty_mutex);
> [ Â 40.780010] Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âlock(open_mutex);
> [ Â 40.780010] Â lock(big_tty_mutex);
> [ Â 40.780010]
> [ Â 40.780010] Â*** DEADLOCK ***
> [ Â 40.780010]
> [ Â 40.780010] 1 lock held by modem-manager/1141:
> [ Â 40.780010] Â#0: Â(open_mutex){+.+...}, at: [<ffffffffa032b321>] acm_tty_close+0x41/0xc0 [cdc_acm]
> [ Â 40.780010]
> [ Â 40.780010] stack backtrace:
> [ Â 40.780010] Pid: 1141, comm: modem-manager Not tainted 3.2.0-rc2+ #8
> [ Â 40.780010] Call Trace:
> [ Â 40.780010] Â[<ffffffff81675378>] print_circular_bug+0x202/0x213
> [ Â 40.780010] Â[<ffffffff810c112e>] __lock_acquire+0x16ce/0x1c40
> [ Â 40.780010] Â[<ffffffff81021f62>] ? native_sched_clock+0x22/0x70
> [ Â 40.780010] Â[<ffffffff810ae2c5>] ? sched_clock_local+0x25/0x90
> [ Â 40.780010] Â[<ffffffff810c1d9d>] lock_acquire+0x9d/0x210
> [ Â 40.780010] Â[<ffffffff81682807>] ? tty_lock+0x17/0x19
> [ Â 40.780010] Â[<ffffffff810c180a>] ? lock_release_non_nested+0x16a/0x350
> [ Â 40.780010] Â[<ffffffff8168003e>] __mutex_lock_common+0x5e/0x4f0
> [ Â 40.780010] Â[<ffffffff81682807>] ? tty_lock+0x17/0x19
> [ Â 40.780010] Â[<ffffffff810c2796>] ? mark_held_locks+0x86/0x150
> [ Â 40.780010] Â[<ffffffff816807ae>] ? mutex_unlock+0xe/0x10
> [ Â 40.780010] Â[<ffffffff81682807>] ? tty_lock+0x17/0x19
> [ Â 40.780010] Â[<ffffffff81680604>] mutex_lock_nested+0x44/0x50
> [ Â 40.780010] Â[<ffffffff81682807>] tty_lock+0x17/0x19
> [ Â 40.780010] Â[<ffffffff813c91ad>] tty_port_close_start+0x17d/0x210
> [ Â 40.780010] Â[<ffffffffa032b32f>] acm_tty_close+0x4f/0xc0 [cdc_acm]
> [ Â 40.780010] Â[<ffffffff813c09e7>] tty_release+0x167/0x5c0
> [ Â 40.780010] Â[<ffffffff81021fb9>] ? sched_clock+0x9/0x10
> [ Â 40.780010] Â[<ffffffff810ae2c5>] ? sched_clock_local+0x25/0x90
> [ Â 40.780010] Â[<ffffffff811b252e>] fput+0xfe/0x2d0
> [ Â 40.780010] Â[<ffffffff811adee9>] filp_close+0x69/0x90
> [ Â 40.780010] Â[<ffffffff811ae1d0>] sys_close+0xc0/0x1a0
> [ Â 40.780010] Â[<ffffffff81689f82>] system_call_fastpath+0x16/0x1b
>

Will the following untested patch fix this problem?

------->
tty_port_close_start() will acquire big_tty_mutex, so don't
call it with open_mutex held.

Signed-off-by: WANG Cong <xiyou.wangcong@xxxxxxxxx>
diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
index e8c564a..3a97fec 100644
--- a/drivers/usb/class/cdc-acm.c
+++ b/drivers/usb/class/cdc-acm.c
@@ -570,8 +570,8 @@ static void acm_tty_close(struct tty_struct *tty, struct file *filp)
if (!acm)
return;

- mutex_lock(&open_mutex);
if (tty_port_close_start(&acm->port, tty, filp) == 0) {
+ mutex_lock(&open_mutex);
if (!acm->dev) {
tty_port_tty_set(&acm->port, NULL);
acm_tty_unregister(acm);
@@ -580,6 +580,7 @@ static void acm_tty_close(struct tty_struct *tty, struct file *filp)
mutex_unlock(&open_mutex);
return;
}
+ mutex_lock(&open_mutex);
acm_port_down(acm);
tty_port_close_end(&acm->port, tty);
tty_port_tty_set(&acm->port, NULL);