diff options
author | Frederic Weisbecker <fweisbec@gmail.com> | 2009-12-29 23:06:21 -0500 |
---|---|---|
committer | Frederic Weisbecker <fweisbec@gmail.com> | 2010-01-01 19:54:47 -0500 |
commit | 27026a05bb805866a3b9068dda8153b72cb942f4 (patch) | |
tree | 45fbdb663ad5165b2b5cbc08f11f09e06d502a72 /fs/reiserfs | |
parent | c4a62ca362258d98f42efb282cfbf9b61caffdbe (diff) |
reiserfs: Fix reiserfs lock <-> i_mutex dependency inversion on xattr
While deleting the xattrs of an inode, we hold the reiserfs lock
and grab the inode->i_mutex of the targeted inode and the root
private xattr directory.
Later on, we may relax the reiserfs lock for various reasons, this
creates inverted dependencies.
We can remove the reiserfs lock -> i_mutex dependency by relaxing
the former before calling open_xa_dir(). This is fine because the
lookup and creation of xattr private directories done in
open_xa_dir() are covered by the targeted inode mutexes. And deeper
operations in the tree are still done under the write lock.
This fixes the following lockdep report:
=======================================================
[ INFO: possible circular locking dependency detected ]
2.6.32-atom #173
-------------------------------------------------------
cp/3204 is trying to acquire lock:
(&REISERFS_SB(s)->lock){+.+.+.}, at: [<c11432b9>] reiserfs_write_lock_once+0x29/0x50
but task is already holding lock:
(&sb->s_type->i_mutex_key#4/3){+.+.+.}, at: [<c1141e18>] open_xa_dir+0xd8/0x1b0
which lock already depends on the new lock.
the existing dependency chain (in reverse order) is:
-> #1 (&sb->s_type->i_mutex_key#4/3){+.+.+.}:
[<c105ea7f>] __lock_acquire+0x11ff/0x19e0
[<c105f2c8>] lock_acquire+0x68/0x90
[<c1401a2b>] mutex_lock_nested+0x5b/0x340
[<c1141d83>] open_xa_dir+0x43/0x1b0
[<c1142722>] reiserfs_for_each_xattr+0x62/0x260
[<c114299a>] reiserfs_delete_xattrs+0x1a/0x60
[<c111ea1f>] reiserfs_delete_inode+0x9f/0x150
[<c10c9c32>] generic_delete_inode+0xa2/0x170
[<c10c9d4f>] generic_drop_inode+0x4f/0x70
[<c10c8b07>] iput+0x47/0x50
[<c10c0965>] do_unlinkat+0xd5/0x160
[<c10c0a00>] sys_unlink+0x10/0x20
[<c1002ec4>] sysenter_do_call+0x12/0x32
-> #0 (&REISERFS_SB(s)->lock){+.+.+.}:
[<c105f176>] __lock_acquire+0x18f6/0x19e0
[<c105f2c8>] lock_acquire+0x68/0x90
[<c1401a2b>] mutex_lock_nested+0x5b/0x340
[<c11432b9>] reiserfs_write_lock_once+0x29/0x50
[<c1117012>] reiserfs_lookup+0x62/0x140
[<c10bd85f>] __lookup_hash+0xef/0x110
[<c10bf21d>] lookup_one_len+0x8d/0xc0
[<c1141e2a>] open_xa_dir+0xea/0x1b0
[<c1141fe5>] xattr_lookup+0x15/0x160
[<c1142476>] reiserfs_xattr_get+0x56/0x2a0
[<c1144042>] reiserfs_get_acl+0xa2/0x360
[<c114461a>] reiserfs_cache_default_acl+0x3a/0x160
[<c111789c>] reiserfs_mkdir+0x6c/0x2c0
[<c10bea96>] vfs_mkdir+0xd6/0x180
[<c10c0c10>] sys_mkdirat+0xc0/0xd0
[<c10c0c40>] sys_mkdir+0x20/0x30
[<c1002ec4>] sysenter_do_call+0x12/0x32
other info that might help us debug this:
2 locks held by cp/3204:
#0: (&sb->s_type->i_mutex_key#4/1){+.+.+.}, at: [<c10bd8d6>] lookup_create+0x26/0xa0
#1: (&sb->s_type->i_mutex_key#4/3){+.+.+.}, at: [<c1141e18>] open_xa_dir+0xd8/0x1b0
stack backtrace:
Pid: 3204, comm: cp Not tainted 2.6.32-atom #173
Call Trace:
[<c13ff993>] ? printk+0x18/0x1a
[<c105d33a>] print_circular_bug+0xca/0xd0
[<c105f176>] __lock_acquire+0x18f6/0x19e0
[<c105d3aa>] ? check_usage+0x6a/0x460
[<c105f2c8>] lock_acquire+0x68/0x90
[<c11432b9>] ? reiserfs_write_lock_once+0x29/0x50
[<c11432b9>] ? reiserfs_write_lock_once+0x29/0x50
[<c1401a2b>] mutex_lock_nested+0x5b/0x340
[<c11432b9>] ? reiserfs_write_lock_once+0x29/0x50
[<c11432b9>] reiserfs_write_lock_once+0x29/0x50
[<c1117012>] reiserfs_lookup+0x62/0x140
[<c105ccca>] ? debug_check_no_locks_freed+0x8a/0x140
[<c105cbe4>] ? trace_hardirqs_on_caller+0x124/0x170
[<c10bd85f>] __lookup_hash+0xef/0x110
[<c10bf21d>] lookup_one_len+0x8d/0xc0
[<c1141e2a>] open_xa_dir+0xea/0x1b0
[<c1141fe5>] xattr_lookup+0x15/0x160
[<c1142476>] reiserfs_xattr_get+0x56/0x2a0
[<c1144042>] reiserfs_get_acl+0xa2/0x360
[<c10ca2e7>] ? new_inode+0x27/0xa0
[<c114461a>] reiserfs_cache_default_acl+0x3a/0x160
[<c1402eb7>] ? _spin_unlock+0x27/0x40
[<c111789c>] reiserfs_mkdir+0x6c/0x2c0
[<c10c7cb8>] ? __d_lookup+0x108/0x190
[<c105c932>] ? mark_held_locks+0x62/0x80
[<c1401c8d>] ? mutex_lock_nested+0x2bd/0x340
[<c10bd17a>] ? generic_permission+0x1a/0xa0
[<c11788fe>] ? security_inode_permission+0x1e/0x20
[<c10bea96>] vfs_mkdir+0xd6/0x180
[<c10c0c10>] sys_mkdirat+0xc0/0xd0
[<c10505c6>] ? up_read+0x16/0x30
[<c1002fd8>] ? restore_all_notrace+0x0/0x18
[<c10c0c40>] sys_mkdir+0x20/0x30
[<c1002ec4>] sysenter_do_call+0x12/0x32
v2: Don't drop reiserfs_mutex_lock_nested_safe() as we'll still
need it later
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Tested-by: Christian Kujau <lists@nerdbynature.de>
Cc: Alexander Beregalov <a.beregalov@gmail.com>
Cc: Chris Mason <chris.mason@oracle.com>
Cc: Ingo Molnar <mingo@elte.hu>
Diffstat (limited to 'fs/reiserfs')
-rw-r--r-- | fs/reiserfs/xattr.c | 9 |
1 files changed, 7 insertions, 2 deletions
diff --git a/fs/reiserfs/xattr.c b/fs/reiserfs/xattr.c index a0e2e7acdc75..c320c7792c75 100644 --- a/fs/reiserfs/xattr.c +++ b/fs/reiserfs/xattr.c | |||
@@ -234,17 +234,22 @@ static int reiserfs_for_each_xattr(struct inode *inode, | |||
234 | if (IS_PRIVATE(inode) || get_inode_sd_version(inode) == STAT_DATA_V1) | 234 | if (IS_PRIVATE(inode) || get_inode_sd_version(inode) == STAT_DATA_V1) |
235 | return 0; | 235 | return 0; |
236 | 236 | ||
237 | reiserfs_write_unlock(inode->i_sb); | ||
237 | dir = open_xa_dir(inode, XATTR_REPLACE); | 238 | dir = open_xa_dir(inode, XATTR_REPLACE); |
238 | if (IS_ERR(dir)) { | 239 | if (IS_ERR(dir)) { |
239 | err = PTR_ERR(dir); | 240 | err = PTR_ERR(dir); |
241 | reiserfs_write_lock(inode->i_sb); | ||
240 | goto out; | 242 | goto out; |
241 | } else if (!dir->d_inode) { | 243 | } else if (!dir->d_inode) { |
242 | err = 0; | 244 | err = 0; |
245 | reiserfs_write_lock(inode->i_sb); | ||
243 | goto out_dir; | 246 | goto out_dir; |
244 | } | 247 | } |
245 | 248 | ||
246 | reiserfs_mutex_lock_nested_safe(&dir->d_inode->i_mutex, I_MUTEX_XATTR, | 249 | mutex_lock_nested(&dir->d_inode->i_mutex, I_MUTEX_XATTR); |
247 | inode->i_sb); | 250 | |
251 | reiserfs_write_lock(inode->i_sb); | ||
252 | |||
248 | buf.xadir = dir; | 253 | buf.xadir = dir; |
249 | err = reiserfs_readdir_dentry(dir, &buf, fill_with_dentries, &pos); | 254 | err = reiserfs_readdir_dentry(dir, &buf, fill_with_dentries, &pos); |
250 | while ((err == 0 || err == -ENOSPC) && buf.count) { | 255 | while ((err == 0 || err == -ENOSPC) && buf.count) { |