diff options
author | Frederic Weisbecker <fweisbec@gmail.com> | 2009-12-13 17:32:06 -0500 |
---|---|---|
committer | Frederic Weisbecker <fweisbec@gmail.com> | 2009-12-14 05:47:11 -0500 |
commit | cb1c2e51c5a72f093b5af384b11d2f1c2abd6c13 (patch) | |
tree | e4cb094df0a729eaac4eb1229e0f9ee0ab39aeaf /fs | |
parent | 500f5a0bf5f0624dae34307010e240ec090e4cde (diff) |
reiserfs: Fix reiserfs lock and journal lock inversion dependency
When we were using the bkl, we didn't care about dependencies against
other locks, but the mutex conversion created new ones, which is why
we have reiserfs_mutex_lock_safe(), which unlocks the reiserfs lock
before acquiring another mutex.
But this trick actually fails if we have acquired the reiserfs lock
recursively, as we try to unlock it to acquire the new mutex without
inverted dependency, but we eventually only decrease its depth.
This happens in the case of a nested inode creation/deletion.
Say we have no space left on the device, we create an inode
and tak the lock but fail to create its entry, then we release the
inode using iput(), which calls reiserfs_delete_inode() that takes
the reiserfs lock recursively. The path eventually ends up in
journal_begin() where we try to take the journal safely but we
fail because of the reiserfs lock recursion:
[ INFO: possible circular locking dependency detected ]
2.6.32-06486-g053fe57 #2
-------------------------------------------------------
vi/23454 is trying to acquire lock:
(&journal->j_mutex){+.+...}, at: [<c110dac4>] do_journal_begin_r+0x64/0x2f0
but task is already holding lock:
(&REISERFS_SB(s)->lock){+.+.+.}, at: [<c11106a8>] reiserfs_write_lock+0x28/0x40
which lock already depends on the new lock.
the existing dependency chain (in reverse order) is:
-> #1 (&REISERFS_SB(s)->lock){+.+.+.}:
[<c104f8f3>] validate_chain+0xa23/0xf70
[<c1050325>] __lock_acquire+0x4e5/0xa70
[<c105092a>] lock_acquire+0x7a/0xa0
[<c134c78f>] mutex_lock_nested+0x5f/0x2b0
[<c11106a8>] reiserfs_write_lock+0x28/0x40
[<c110dacb>] do_journal_begin_r+0x6b/0x2f0
[<c110ddcf>] journal_begin+0x7f/0x120
[<c10f76c2>] reiserfs_remount+0x212/0x4d0
[<c1093997>] do_remount_sb+0x67/0x140
[<c10a9ca6>] do_mount+0x436/0x6b0
[<c10a9f86>] sys_mount+0x66/0xa0
[<c1002c50>] sysenter_do_call+0x12/0x36
-> #0 (&journal->j_mutex){+.+...}:
[<c104fe38>] validate_chain+0xf68/0xf70
[<c1050325>] __lock_acquire+0x4e5/0xa70
[<c105092a>] lock_acquire+0x7a/0xa0
[<c134c78f>] mutex_lock_nested+0x5f/0x2b0
[<c110dac4>] do_journal_begin_r+0x64/0x2f0
[<c110ddcf>] journal_begin+0x7f/0x120
[<c10ef52f>] reiserfs_delete_inode+0x9f/0x140
[<c10a55fc>] generic_delete_inode+0x9c/0x150
[<c10a56ed>] generic_drop_inode+0x3d/0x60
[<c10a4607>] iput+0x47/0x50
[<c10e915c>] reiserfs_create+0x16c/0x1c0
[<c109a9c1>] vfs_create+0xc1/0x130
[<c109dbec>] do_filp_open+0x81c/0x920
[<c109004f>] do_sys_open+0x4f/0x110
[<c1090179>] sys_open+0x29/0x40
[<c1002c50>] sysenter_do_call+0x12/0x36
other info that might help us debug this:
2 locks held by vi/23454:
#0: (&sb->s_type->i_mutex_key#5){+.+.+.}, at: [<c109d64e>]
do_filp_open+0x27e/0x920
#1: (&REISERFS_SB(s)->lock){+.+.+.}, at: [<c11106a8>]
reiserfs_write_lock+0x28/0x40
stack backtrace:
Pid: 23454, comm: vi Not tainted 2.6.32-06486-g053fe57 #2
Call Trace:
[<c134b202>] ? printk+0x18/0x1e
[<c104e960>] print_circular_bug+0xc0/0xd0
[<c104fe38>] validate_chain+0xf68/0xf70
[<c104ca9b>] ? trace_hardirqs_off+0xb/0x10
[<c1050325>] __lock_acquire+0x4e5/0xa70
[<c105092a>] lock_acquire+0x7a/0xa0
[<c110dac4>] ? do_journal_begin_r+0x64/0x2f0
[<c134c78f>] mutex_lock_nested+0x5f/0x2b0
[<c110dac4>] ? do_journal_begin_r+0x64/0x2f0
[<c110dac4>] ? do_journal_begin_r+0x64/0x2f0
[<c110ff80>] ? delete_one_xattr+0x0/0x1c0
[<c110dac4>] do_journal_begin_r+0x64/0x2f0
[<c110ddcf>] journal_begin+0x7f/0x120
[<c11105b5>] ? reiserfs_delete_xattrs+0x15/0x50
[<c10ef52f>] reiserfs_delete_inode+0x9f/0x140
[<c10a55bf>] ? generic_delete_inode+0x5f/0x150
[<c10ef490>] ? reiserfs_delete_inode+0x0/0x140
[<c10a55fc>] generic_delete_inode+0x9c/0x150
[<c10a56ed>] generic_drop_inode+0x3d/0x60
[<c10a4607>] iput+0x47/0x50
[<c10e915c>] reiserfs_create+0x16c/0x1c0
[<c1099a5d>] ? inode_permission+0x7d/0xa0
[<c109a9c1>] vfs_create+0xc1/0x130
[<c10e8ff0>] ? reiserfs_create+0x0/0x1c0
[<c109dbec>] do_filp_open+0x81c/0x920
[<c104ca9b>] ? trace_hardirqs_off+0xb/0x10
[<c134dc0d>] ? _spin_unlock+0x1d/0x20
[<c10a6eea>] ? alloc_fd+0xba/0xf0
[<c109004f>] do_sys_open+0x4f/0x110
[<c1090179>] sys_open+0x29/0x40
[<c1002c50>] sysenter_do_call+0x12/0x36
To fix this, use reiserfs_lock_once() from reiserfs_delete_inode()
which prevents from adding reiserfs lock recursion.
Reported-by: Alexander Beregalov <a.beregalov@gmail.com>
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Chris Mason <chris.mason@oracle.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Thomas Gleixner <tglx@linutronix.de>
Diffstat (limited to 'fs')
-rw-r--r-- | fs/reiserfs/inode.c | 5 |
1 files changed, 3 insertions, 2 deletions
diff --git a/fs/reiserfs/inode.c b/fs/reiserfs/inode.c index 3a28e7751b3c..bd615dfe4ec7 100644 --- a/fs/reiserfs/inode.c +++ b/fs/reiserfs/inode.c | |||
@@ -31,11 +31,12 @@ void reiserfs_delete_inode(struct inode *inode) | |||
31 | JOURNAL_PER_BALANCE_CNT * 2 + | 31 | JOURNAL_PER_BALANCE_CNT * 2 + |
32 | 2 * REISERFS_QUOTA_INIT_BLOCKS(inode->i_sb); | 32 | 2 * REISERFS_QUOTA_INIT_BLOCKS(inode->i_sb); |
33 | struct reiserfs_transaction_handle th; | 33 | struct reiserfs_transaction_handle th; |
34 | int depth; | ||
34 | int err; | 35 | int err; |
35 | 36 | ||
36 | truncate_inode_pages(&inode->i_data, 0); | 37 | truncate_inode_pages(&inode->i_data, 0); |
37 | 38 | ||
38 | reiserfs_write_lock(inode->i_sb); | 39 | depth = reiserfs_write_lock_once(inode->i_sb); |
39 | 40 | ||
40 | /* The = 0 happens when we abort creating a new inode for some reason like lack of space.. */ | 41 | /* The = 0 happens when we abort creating a new inode for some reason like lack of space.. */ |
41 | if (!(inode->i_state & I_NEW) && INODE_PKEY(inode)->k_objectid != 0) { /* also handles bad_inode case */ | 42 | if (!(inode->i_state & I_NEW) && INODE_PKEY(inode)->k_objectid != 0) { /* also handles bad_inode case */ |
@@ -74,7 +75,7 @@ void reiserfs_delete_inode(struct inode *inode) | |||
74 | out: | 75 | out: |
75 | clear_inode(inode); /* note this must go after the journal_end to prevent deadlock */ | 76 | clear_inode(inode); /* note this must go after the journal_end to prevent deadlock */ |
76 | inode->i_blocks = 0; | 77 | inode->i_blocks = 0; |
77 | reiserfs_write_unlock(inode->i_sb); | 78 | reiserfs_write_unlock_once(inode->i_sb, depth); |
78 | } | 79 | } |
79 | 80 | ||
80 | static void _make_cpu_key(struct cpu_key *key, int version, __u32 dirid, | 81 | static void _make_cpu_key(struct cpu_key *key, int version, __u32 dirid, |