From 3f14fea6bbd3444dd46a2af3a2e219e792616645 Mon Sep 17 00:00:00 2001 From: Frederic Weisbecker Date: Wed, 30 Dec 2009 07:03:53 +0100 Subject: [PATCH] reiserfs: Relax lock before open xattr dir in reiserfs_xattr_set_handle() We call xattr_lookup() from reiserfs_xattr_get(). We then hold the reiserfs lock when we grab the i_mutex. But later, we may relax the reiserfs lock, creating dependency inversion between both locks. The lookups and creation jobs ar already protected by the inode mutex, so we can safely relax the reiserfs lock, dropping the unwanted reiserfs lock -> i_mutex dependency, as shown in 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: [] reiserfs_write_lock_once+0x29/0x50 but task is already holding lock: (&sb->s_type->i_mutex_key#4/3){+.+.+.}, at: [] 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){+.+.+.}: [] __lock_acquire+0x11ff/0x19e0 [] lock_acquire+0x68/0x90 [] mutex_lock_nested+0x5b/0x340 [] open_xa_dir+0x43/0x1b0 [] reiserfs_for_each_xattr+0x62/0x260 [] reiserfs_delete_xattrs+0x1a/0x60 [] reiserfs_delete_inode+0x9f/0x150 [] generic_delete_inode+0xa2/0x170 [] generic_drop_inode+0x4f/0x70 [] iput+0x47/0x50 [] do_unlinkat+0xd5/0x160 [] sys_unlink+0x10/0x20 [] sysenter_do_call+0x12/0x32 -> #0 (&REISERFS_SB(s)->lock){+.+.+.}: [] __lock_acquire+0x18f6/0x19e0 [] lock_acquire+0x68/0x90 [] mutex_lock_nested+0x5b/0x340 [] reiserfs_write_lock_once+0x29/0x50 [] reiserfs_lookup+0x62/0x140 [] __lookup_hash+0xef/0x110 [] lookup_one_len+0x8d/0xc0 [] open_xa_dir+0xea/0x1b0 [] xattr_lookup+0x15/0x160 [] reiserfs_xattr_get+0x56/0x2a0 [] reiserfs_get_acl+0xa2/0x360 [] reiserfs_cache_default_acl+0x3a/0x160 [] reiserfs_mkdir+0x6c/0x2c0 [] vfs_mkdir+0xd6/0x180 [] sys_mkdirat+0xc0/0xd0 [] sys_mkdir+0x20/0x30 [] 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: [] lookup_create+0x26/0xa0 #1: (&sb->s_type->i_mutex_key#4/3){+.+.+.}, at: [] open_xa_dir+0xd8/0x1b0 stack backtrace: Pid: 3204, comm: cp Not tainted 2.6.32-atom #173 Call Trace: [] ? printk+0x18/0x1a [] print_circular_bug+0xca/0xd0 [] __lock_acquire+0x18f6/0x19e0 [] ? check_usage+0x6a/0x460 [] lock_acquire+0x68/0x90 [] ? reiserfs_write_lock_once+0x29/0x50 [] ? reiserfs_write_lock_once+0x29/0x50 [] mutex_lock_nested+0x5b/0x340 [] ? reiserfs_write_lock_once+0x29/0x50 [] reiserfs_write_lock_once+0x29/0x50 [] reiserfs_lookup+0x62/0x140 [] ? debug_check_no_locks_freed+0x8a/0x140 [] ? trace_hardirqs_on_caller+0x124/0x170 [] __lookup_hash+0xef/0x110 [] lookup_one_len+0x8d/0xc0 [] open_xa_dir+0xea/0x1b0 [] xattr_lookup+0x15/0x160 [] reiserfs_xattr_get+0x56/0x2a0 [] reiserfs_get_acl+0xa2/0x360 [] ? new_inode+0x27/0xa0 [] reiserfs_cache_default_acl+0x3a/0x160 [] ? _spin_unlock+0x27/0x40 [] reiserfs_mkdir+0x6c/0x2c0 [] ? __d_lookup+0x108/0x190 [] ? mark_held_locks+0x62/0x80 [] ? mutex_lock_nested+0x2bd/0x340 [] ? generic_permission+0x1a/0xa0 [] ? security_inode_permission+0x1e/0x20 [] vfs_mkdir+0xd6/0x180 [] sys_mkdirat+0xc0/0xd0 [] ? up_read+0x16/0x30 [] ? restore_all_notrace+0x0/0x18 [] sys_mkdir+0x20/0x30 [] sysenter_do_call+0x12/0x32 Signed-off-by: Frederic Weisbecker Tested-by: Christian Kujau Cc: Alexander Beregalov Cc: Chris Mason Cc: Ingo Molnar --- fs/reiserfs/xattr.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/fs/reiserfs/xattr.c b/fs/reiserfs/xattr.c index c320c7792c75..78a3f246295c 100644 --- a/fs/reiserfs/xattr.c +++ b/fs/reiserfs/xattr.c @@ -485,11 +485,16 @@ reiserfs_xattr_set_handle(struct reiserfs_transaction_handle *th, if (!buffer) return lookup_and_delete_xattr(inode, name); + reiserfs_write_unlock(inode->i_sb); dentry = xattr_lookup(inode, name, flags); - if (IS_ERR(dentry)) + if (IS_ERR(dentry)) { + reiserfs_write_lock(inode->i_sb); return PTR_ERR(dentry); + } - reiserfs_down_read_safe(&REISERFS_I(inode)->i_xattr_sem, inode->i_sb); + down_read(&REISERFS_I(inode)->i_xattr_sem); + + reiserfs_write_lock(inode->i_sb); xahash = xattr_hash(buffer, buffer_size); while (buffer_pos < buffer_size || buffer_pos == 0) { -- 2.20.1