Re: Lockdep warning for sys_tee system call

Peter Zijlstra
2009-07-21T08:08:41Z

On Tue, 2009-07-21 at 08:59 +0200, Eric Sesterhenn wrote:
> Hi,
>
> On one of my systems I get the following lockdep warning, after
> running ./testcases/bin/tee01 from current LTP
> multiple times.
>
> [ 2000.324359] =======================================================
> [ 2000.324658] [ INFO: possible circular locking dependency detected ]
> [ 2000.324804] 2.6.31-rc3 #10
> [ 2000.324916] -------------------------------------------------------
> [ 2000.325135] tee01/18578 is trying to acquire lock:
> [ 2000.325265] (&sb->s_type->i_mutex_key#7/2){+.+...}, at: [<c01a5f5b>] pipe_double_lock+0x3b/0x80
> [ 2000.325843]
> [ 2000.325846] but task is already holding lock:
> [ 2000.326124] (&sb->s_type->i_mutex_key#7/1){+.+.+.}, at: [<c01a5f45>] pipe_double_lock+0x25/0x80
> [ 2000.326681]
> [ 2000.326684] which lock already depends on the new lock.
> [ 2000.326689]
> [ 2000.327062]
> [ 2000.327066] the existing dependency chain (in reverse order) is:
> [ 2000.327354]
> [ 2000.327357] -> #1 (&sb->s_type->i_mutex_key#7/1){+.+.+.}:
> [ 2000.327943] [<c014e9ff>] __lock_acquire+0xbbf/0x1040
> [ 2000.328048] [<c014eef4>] lock_acquire+0x74/0xa0
> [ 2000.328048] [<c05ce891>] mutex_lock_nested+0x51/0x280
> [ 2000.328048] [<c01a5f8c>] pipe_double_lock+0x6c/0x80
> [ 2000.328048] [<c01bd1cf>] sys_tee+0x12f/0x2c0
> [ 2000.328048] [<c010305b>] sysenter_do_call+0x12/0x32
> [ 2000.328048] [<ffffffff>] 0xffffffff
> [ 2000.328048]
> [ 2000.328048] -> #0 (&sb->s_type->i_mutex_key#7/2){+.+...}:
> [ 2000.328048] [<c014eab0>] __lock_acquire+0xc70/0x1040
> [ 2000.328048] [<c014eef4>] lock_acquire+0x74/0xa0
> [ 2000.328048] [<c05ce891>] mutex_lock_nested+0x51/0x280
> [ 2000.328048] [<c01a5f5b>] pipe_double_lock+0x3b/0x80
> [ 2000.328048] [<c01bd1cf>] sys_tee+0x12f/0x2c0
> [ 2000.328048] [<c010305b>] sysenter_do_call+0x12/0x32
> [ 2000.328048] [<ffffffff>] 0xffffffff
> [ 2000.328048]
> [ 2000.328048] other info that might help us debug this:
> [ 2000.328048]
> [ 2000.328048] 1 lock held by tee01/18578:
> [ 2000.328048] #0: (&sb->s_type->i_mutex_key#7/1){+.+.+.}, at: [<c01a5f45>] pipe_double_lock+0x25/0x80
> [ 2000.328048]
> [ 2000.328048] stack backtrace:
> [ 2000.328048] Pid: 18578, comm: tee01 Not tainted 2.6.31-rc3 #10
> [ 2000.328048] Call Trace:
> [ 2000.328048] [<c05cd998>] ? printk+0x18/0x20
> [ 2000.328048] [<c014d494>] print_circular_bug_tail+0x84/0xd0
> [ 2000.328048] [<c014eab0>] __lock_acquire+0xc70/0x1040
> [ 2000.328048] [<c014eef4>] lock_acquire+0x74/0xa0
> [ 2000.328048] [<c01a5f5b>] ? pipe_double_lock+0x3b/0x80
> [ 2000.328048] [<c05ce891>] mutex_lock_nested+0x51/0x280
> [ 2000.328048] [<c01a5f5b>] ? pipe_double_lock+0x3b/0x80
> [ 2000.328048] [<c01a5f5b>] pipe_double_lock+0x3b/0x80
> [ 2000.328048] [<c01bd1cf>] sys_tee+0x12f/0x2c0
> [ 2000.328048] [<c014dbbc>] ? trace_hardirqs_on_caller+0x12c/0x180
> [ 2000.328048] [<c010305b>] sysenter_do_call+0x12/0x32

void pipe_double_lock(struct pipe_inode_info *pipe1,
struct pipe_inode_info *pipe2)
{
BUG_ON(pipe1 == pipe2);

if (pipe1 < pipe2) {
pipe_lock_nested(pipe1, I_MUTEX_PARENT);
pipe_lock_nested(pipe2, I_MUTEX_CHILD);
} else {
pipe_lock_nested(pipe2, I_MUTEX_CHILD);
pipe_lock_nested(pipe1, I_MUTEX_PARENT);
}
}

That's an obvious FAIL right there.

Miklos?

---
Subject: fs/pipe: rectify a lockdep annotation

The presumed use of the pipe_double_lock() routine is to lock 2 locks in
a deadlock free way by ordering the locks by their address. However it
fails to keep the specified lock classes in order and explicitly
annotates a deadlock.

Rectify this.

Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
---
fs/pipe.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/pipe.c b/fs/pipe.c
index f7dd21a..52c4151 100644
--- a/fs/pipe.c
+++ b/fs/pipe.c
@@ -68,8 +68,8 @@ void pipe_double_lock(struct pipe_inode_info *pipe1,
pipe_lock_nested(pipe1, I_MUTEX_PARENT);
pipe_lock_nested(pipe2, I_MUTEX_CHILD);
} else {
- pipe_lock_nested(pipe2, I_MUTEX_CHILD);
- pipe_lock_nested(pipe1, I_MUTEX_PARENT);
+ pipe_lock_nested(pipe2, I_MUTEX_PARENT);
+ pipe_lock_nested(pipe1, I_MUTEX_CHILD);
}
}