* 2.6.17-rc5-mm3-lockdep - locking error in quotaon
@ 2006-06-05 17:00 Valdis.Kletnieks
2006-06-05 17:25 ` Arjan van de Ven
0 siblings, 1 reply; 11+ messages in thread
From: Valdis.Kletnieks @ 2006-06-05 17:00 UTC (permalink / raw)
To: Ingo Molnar, Andrew Morton, jack; +Cc: linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1646 bytes --]
Using the -lockdep patch that Ingo had a few days ago, plus Stefan Richter's
two patches for ieee1394 (which fixed the *first* locking issue I hit)....
[ 219.535000] ( quotaon-1374 |#0): new 222056071 us user-latency.
[ 219.535000] stopped custom tracer.
[ 219.535000]
[ 219.535000] ======================================
[ 219.535000] [ BUG: bad unlock ordering detected! ]
[ 219.535000] --------------------------------------
[ 219.535000] quotaon/1374 is trying to release lock (&inode->i_mutex) at:
[ 219.535000] [<c0382ba2>] mutex_unlock+0xd/0xf
[ 219.535000] but the next lock to release is:
[ 219.535000] (&s->s_dquot.dqonoff_mutex){--..}, at: [<c0382a09>] mutex_lock+0xd/0xf
[ 219.535000]
[ 219.535000] other info that might help us debug this:
[ 219.535000] 2 locks held by quotaon/1374:
[ 219.535000] #0: (&s->s_umount#16){----}, at: [<c016828f>] get_super+0x4a/0xd4
[ 219.535000] #1: (&inode->i_mutex){--..}, at: [<c0382a09>] mutex_lock+0xd/0xf
[ 219.535000]
[ 219.535000] stack backtrace:
[ 219.535000] [<c01032d6>] show_trace_log_lvl+0x64/0x125
[ 219.535000] [<c0103865>] show_trace+0x1b/0x20
[ 219.535000] [<c010391c>] dump_stack+0x1f/0x24
[ 219.535000] [<c012dee7>] lockdep_release+0x192/0x35e
[ 219.535000] [<c0382b53>] __mutex_unlock_slowpath+0x29/0x6b
[ 219.535000] [<c0382ba2>] mutex_unlock+0xd/0xf
[ 219.535000] [<c019243c>] vfs_quota_on_inode+0x35e/0x538
[ 219.535000] [<c019334b>] vfs_quota_on+0x55/0x6b
[ 219.535000] [<c01ac3f7>] ext3_quota_on+0x47/0xb6
[ 219.535000] [<c0195a11>] sys_quotactl+0x3b5/0x695
[ 219.535000] [<c03845b2>] sysenter_past_esp+0x63/0xa1
[-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: 2.6.17-rc5-mm3-lockdep - locking error in quotaon 2006-06-05 17:00 2.6.17-rc5-mm3-lockdep - locking error in quotaon Valdis.Kletnieks @ 2006-06-05 17:25 ` Arjan van de Ven 2006-06-05 17:35 ` Jan Kara 2006-06-05 19:20 ` Valdis.Kletnieks 0 siblings, 2 replies; 11+ messages in thread From: Arjan van de Ven @ 2006-06-05 17:25 UTC (permalink / raw) To: Valdis.Kletnieks; +Cc: linux-kernel, jack, Andrew Morton, Ingo Molnar On Mon, 2006-06-05 at 13:00 -0400, Valdis.Kletnieks@vt.edu wrote: > Using the -lockdep patch that Ingo had a few days ago, plus Stefan Richter's > two patches for ieee1394 (which fixed the *first* locking issue I hit).... > [ 219.535000] quotaon/1374 is trying to release lock (&inode->i_mutex) at: > [ 219.535000] [<c0382ba2>] mutex_unlock+0xd/0xf > [ 219.535000] but the next lock to release is: > [ 219.535000] (&s->s_dquot.dqonoff_mutex){--..}, at: [<c0382a09>] mutex_lock+0xd/0xf ok the quota code is playing fun games with unlock order.. The quota code nests 3 mutexes but releases them in a totally different order; mark this as such in the code. Signed-off-by: Arjan van de Ven <arjan@linux.intel.com> --- fs/dquot.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Index: linux-2.6.17-rc5-mm3/fs/dquot.c =================================================================== --- linux-2.6.17-rc5-mm3.orig/fs/dquot.c +++ linux-2.6.17-rc5-mm3/fs/dquot.c @@ -1475,7 +1475,7 @@ static int vfs_quota_on_inode(struct ino goto out_file_init; } mutex_unlock(&dqopt->dqio_mutex); - mutex_unlock(&inode->i_mutex); + mutex_unlock_non_nested(&inode->i_mutex); set_enable_flags(dqopt, type); add_dquot_ref(sb, type); ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: 2.6.17-rc5-mm3-lockdep - locking error in quotaon 2006-06-05 17:25 ` Arjan van de Ven @ 2006-06-05 17:35 ` Jan Kara 2006-06-05 19:20 ` Valdis.Kletnieks 1 sibling, 0 replies; 11+ messages in thread From: Jan Kara @ 2006-06-05 17:35 UTC (permalink / raw) To: Arjan van de Ven Cc: Valdis.Kletnieks, linux-kernel, Andrew Morton, Ingo Molnar > On Mon, 2006-06-05 at 13:00 -0400, Valdis.Kletnieks@vt.edu wrote: > > Using the -lockdep patch that Ingo had a few days ago, plus Stefan Richter's > > two patches for ieee1394 (which fixed the *first* locking issue I hit).... > > > [ 219.535000] quotaon/1374 is trying to release lock (&inode->i_mutex) at: > > [ 219.535000] [<c0382ba2>] mutex_unlock+0xd/0xf > > [ 219.535000] but the next lock to release is: > > [ 219.535000] (&s->s_dquot.dqonoff_mutex){--..}, at: [<c0382a09>] mutex_lock+0xd/0xf > > > ok the quota code is playing fun games with unlock order.. > > > > The quota code nests 3 mutexes but releases them in a totally different > order; mark this as such in the code. Yes, it does. It was hard enough to get it to *lock* everything in the right order and thus avoid all those nasty lock inversion problems so unlocking is sometimes a bit arbitrary :).. > Signed-off-by: Arjan van de Ven <arjan@linux.intel.com> Signed-off-by: Jan Kara <jack@suse.cz> > --- > fs/dquot.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > Index: linux-2.6.17-rc5-mm3/fs/dquot.c > =================================================================== > --- linux-2.6.17-rc5-mm3.orig/fs/dquot.c > +++ linux-2.6.17-rc5-mm3/fs/dquot.c > @@ -1475,7 +1475,7 @@ static int vfs_quota_on_inode(struct ino > goto out_file_init; > } > mutex_unlock(&dqopt->dqio_mutex); > - mutex_unlock(&inode->i_mutex); > + mutex_unlock_non_nested(&inode->i_mutex); > set_enable_flags(dqopt, type); > > add_dquot_ref(sb, type); > -- Jan Kara <jack@suse.cz> SuSE CR Labs ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: 2.6.17-rc5-mm3-lockdep - locking error in quotaon 2006-06-05 17:25 ` Arjan van de Ven 2006-06-05 17:35 ` Jan Kara @ 2006-06-05 19:20 ` Valdis.Kletnieks 2006-06-05 19:35 ` Jan Kara 1 sibling, 1 reply; 11+ messages in thread From: Valdis.Kletnieks @ 2006-06-05 19:20 UTC (permalink / raw) To: Arjan van de Ven; +Cc: linux-kernel, jack, Andrew Morton, Ingo Molnar [-- Attachment #1: Type: text/plain, Size: 8782 bytes --] On Mon, 05 Jun 2006 19:25:39 +0200, Arjan van de Ven said: > The quota code nests 3 mutexes but releases them in a totally different > order; mark this as such in the code. > =================================================================== > --- linux-2.6.17-rc5-mm3.orig/fs/dquot.c > +++ linux-2.6.17-rc5-mm3/fs/dquot.c > @@ -1475,7 +1475,7 @@ static int vfs_quota_on_inode(struct ino > goto out_file_init; > } > mutex_unlock(&dqopt->dqio_mutex); > - mutex_unlock(&inode->i_mutex); > + mutex_unlock_non_nested(&inode->i_mutex); > set_enable_flags(dqopt, type); > > add_dquot_ref(sb, type); Obviously, there be bigger and nastier dragons lurking in the quota code. This one made it past quotaon, but not as far as the swapon command in the Fedora /etc/rc.sysinit. There's a bunch of rm all over the place in that section of the script, and I'm not sure at all which one triggered it. [ 228.335000] ( rm-1399 |#0): new 231527956 us user-latency. [ 228.335000] stopped custom tracer. [ 228.335000] [ 228.335000] ===================================================== [ 228.335000] [ BUG: possible circular locking deadlock detected! ] [ 228.335000] ----------------------------------------------------- [ 228.335000] rm/1399 is trying to acquire lock: [ 228.335000] (&inode->i_mutex){--..}, at: [<c0382a09>] mutex_lock+0xd/0xf [ 228.335000] [ 228.335000] but task is already holding lock: [ 228.335000] (&s->s_dquot.dqio_mutex){--..}, at: [<c0382a09>] mutex_lock+0xd/0xf [ 228.335000] [ 228.335000] which lock already depends on the new lock, [ 228.335000] which could lead to circular deadlocks! [ 228.335000] [ 228.335000] the existing dependency chain (in reverse order) is: [ 228.335000] [ 228.335000] -> #5 (&s->s_dquot.dqio_mutex){--..}: [ 228.335000] [<c012dd3d>] lockdep_acquire+0x67/0x7f [ 228.335000] [<c0382919>] __mutex_lock_slowpath+0x3a/0x11d [ 228.335000] [<c0382a09>] mutex_lock+0xd/0xf [ 228.335000] [<c0192fbd>] dquot_acquire+0x2d/0xeb [ 228.335000] [<c01aded9>] ext3_acquire_dquot+0x63/0x89 [ 228.335000] [<c0191d46>] dqget+0x264/0x2b0 [ 228.335000] [<c01934c7>] dquot_initialize+0x92/0xdb [ 228.335000] [<c01addda>] ext3_dquot_initialize+0x53/0x79 [ 228.335000] [<c01702e6>] may_open+0x1c9/0x243 [ 228.335000] [<c0172a52>] open_namei+0x28f/0x710 [ 228.335000] [<c015f937>] do_filp_open+0x2b/0x42 [ 228.335000] [<c015f9af>] do_sys_open+0x61/0xe8 [ 228.335000] [<c015fa69>] sys_open+0x18/0x1a [ 228.335000] [<c03845b2>] sysenter_past_esp+0x63/0xa1 [ 228.335000] [ 228.335000] -> #4 (&dquot->dq_lock){--..}: [ 228.335000] [<c012dd3d>] lockdep_acquire+0x67/0x7f [ 228.335000] [<c0382919>] __mutex_lock_slowpath+0x3a/0x11d [ 228.335000] [<c0382a09>] mutex_lock+0xd/0xf [ 228.335000] [<c0191d2b>] dqget+0x249/0x2b0 [ 228.335000] [<c01934c7>] dquot_initialize+0x92/0xdb [ 228.335000] [<c01addda>] ext3_dquot_initialize+0x53/0x79 [ 228.335000] [<c01702e6>] may_open+0x1c9/0x243 [ 228.335000] [<c0172a52>] open_namei+0x28f/0x710 [ 228.335000] [<c015f937>] do_filp_open+0x2b/0x42 [ 228.335000] [<c015f9af>] do_sys_open+0x61/0xe8 [ 228.335000] [<c015fa69>] sys_open+0x18/0x1a [ 228.335000] [<c03845b2>] sysenter_past_esp+0x63/0xa1 [ 228.335000] [ 228.335000] -> #3 (&s->s_dquot.dqptr_sem){----}: [ 228.335000] [<c012dd3d>] lockdep_acquire+0x67/0x7f [ 228.335000] [<c01939b6>] dquot_free_space+0x4b/0x229 [ 228.335000] [<c01a1953>] ext3_free_blocks+0x75/0x94 [ 228.335000] [<c01a5915>] ext3_clear_blocks+0xcb/0xf8 [ 228.335000] [<c01a59e1>] ext3_free_data+0x9f/0xd4 [ 228.335000] [<c01a604c>] ext3_truncate+0x4a3/0x770 [ 228.335000] [<c015178a>] vmtruncate+0x126/0x151 [ 228.335000] [<c017bb0e>] inode_setattr+0x77/0x1a5 [ 228.335000] [<c01a71bb>] ext3_setattr+0x1b5/0x21d [ 228.335000] [<c017bdc1>] notify_change+0x185/0x358 [ 228.335000] [<c015fc28>] do_truncate+0x5d/0x78 [ 228.335000] [<c01702f8>] may_open+0x1db/0x243 [ 228.335000] [<c0172a52>] open_namei+0x28f/0x710 [ 228.335000] [<c015f937>] do_filp_open+0x2b/0x42 [ 228.335000] [<c015f9af>] do_sys_open+0x61/0xe8 [ 228.335000] [<c015fa69>] sys_open+0x18/0x1a [ 228.335000] [<c03845b2>] sysenter_past_esp+0x63/0xa1 [ 228.335000] [ 228.335000] -> #2 (&ei->truncate_mutex){--..}: [ 228.335000] [<c012dd3d>] lockdep_acquire+0x67/0x7f [ 228.335000] [<c0382919>] __mutex_lock_slowpath+0x3a/0x11d [ 228.335000] [<c0382a09>] mutex_lock+0xd/0xf [ 228.335000] [<c01a602d>] ext3_truncate+0x484/0x770 [ 228.335000] [<c015178a>] vmtruncate+0x126/0x151 [ 228.335000] [<c017bb0e>] inode_setattr+0x77/0x1a5 [ 228.335000] [<c01a71bb>] ext3_setattr+0x1b5/0x21d [ 228.335000] [<c017bdc1>] notify_change+0x185/0x358 [ 228.335000] [<c015fc28>] do_truncate+0x5d/0x78 [ 228.335000] [<c01702f8>] may_open+0x1db/0x243 [ 228.335000] [<c0172a52>] open_namei+0x28f/0x710 [ 228.335000] [<c015f937>] do_filp_open+0x2b/0x42 [ 228.335000] [<c015f9af>] do_sys_open+0x61/0xe8 [ 228.335000] [<c015fa69>] sys_open+0x18/0x1a [ 228.335000] [<c03845b2>] sysenter_past_esp+0x63/0xa1 [ 228.335000] [ 228.335000] -> #1 (&inode->i_alloc_sem){--..}: [ 228.335000] [<c012dd3d>] lockdep_acquire+0x67/0x7f [ 228.335000] [<c017bd4c>] notify_change+0x110/0x358 [ 228.335000] [<c015fc28>] do_truncate+0x5d/0x78 [ 228.335000] [<c01702f8>] may_open+0x1db/0x243 [ 228.335000] [<c0172a52>] open_namei+0x28f/0x710 [ 228.335000] [<c015f937>] do_filp_open+0x2b/0x42 [ 228.335000] [<c015f9af>] do_sys_open+0x61/0xe8 [ 228.335000] [<c015fa69>] sys_open+0x18/0x1a [ 228.335000] [<c0384644>] syscall_call+0x7/0xb [ 228.335000] [ 228.335000] -> #0 (&inode->i_mutex){--..}: [ 228.335000] [<c012dd3d>] lockdep_acquire+0x67/0x7f [ 228.335000] [<c0382919>] __mutex_lock_slowpath+0x3a/0x11d [ 228.335000] [<c0382a09>] mutex_lock+0xd/0xf [ 228.335000] [<c01adb93>] ext3_quota_write+0x80/0x274 [ 228.335000] [<c019521d>] v2_write_dquot+0x112/0x152 [ 228.335000] [<c01933e9>] dquot_commit+0x88/0xd4 [ 228.335000] [<c01abd84>] ext3_write_dquot+0x63/0x89 [ 228.335000] [<c01919b4>] dqput+0x118/0x1bb [ 228.335000] [<c0193565>] dquot_drop+0x55/0x9c [ 228.335000] [<c01ade50>] ext3_dquot_drop+0x50/0x76 [ 228.335000] [<c01a3ca7>] ext3_free_inode+0x100/0x32d [ 228.335000] [<c01a68d9>] ext3_delete_inode+0xc3/0xe2 [ 228.335000] [<c017ac28>] generic_delete_inode+0xdb/0x164 [ 228.335000] [<c017acca>] generic_drop_inode+0x19/0x14b [ 228.335000] [<c017a5d6>] iput+0x84/0x8c [ 228.335000] [<c0172010>] do_unlinkat+0xe7/0x159 [ 228.335000] [<c0172094>] sys_unlink+0x12/0x14 [ 228.335000] [<c03845b2>] sysenter_past_esp+0x63/0xa1 [ 228.335000] [ 228.335000] other info that might help us debug this: [ 228.335000] [ 228.335000] 2 locks held by rm/1399: [ 228.335000] #0: (&s->s_dquot.dqptr_sem){----}, at: [<c0193529>] dquot_drop+0x19/0x9c [ 228.335000] #1: (&s->s_dquot.dqio_mutex){--..}, at: [<c0382a09>] mutex_lock+0xd/0xf [ 228.335000] [ 228.335000] stack backtrace: [ 228.335000] [<c01032d6>] show_trace_log_lvl+0x64/0x125 [ 228.335000] [<c0103865>] show_trace+0x1b/0x20 [ 228.335000] [<c010391c>] dump_stack+0x1f/0x24 [ 228.335000] [<c012ce2e>] print_circular_bug_tail+0x5d/0x69 [ 228.335000] [<c012d6d0>] __lockdep_acquire+0x896/0xa91 [ 228.335000] [<c012dd3d>] lockdep_acquire+0x67/0x7f [ 228.335000] [<c0382919>] __mutex_lock_slowpath+0x3a/0x11d [ 228.335000] [<c0382a09>] mutex_lock+0xd/0xf [ 228.335000] [<c01adb93>] ext3_quota_write+0x80/0x274 [ 228.335000] [<c019521d>] v2_write_dquot+0x112/0x152 [ 228.335000] [<c01933e9>] dquot_commit+0x88/0xd4 [ 228.335000] [<c01abd84>] ext3_write_dquot+0x63/0x89 [ 228.335000] [<c01919b4>] dqput+0x118/0x1bb [ 228.335000] [<c0193565>] dquot_drop+0x55/0x9c [ 228.335000] [<c01ade50>] ext3_dquot_drop+0x50/0x76 [ 228.335000] [<c01a3ca7>] ext3_free_inode+0x100/0x32d [ 228.335000] [<c01a68d9>] ext3_delete_inode+0xc3/0xe2 [ 228.335000] [<c017ac28>] generic_delete_inode+0xdb/0x164 [ 228.335000] [<c017acca>] generic_drop_inode+0x19/0x14b [ 228.335000] [<c017a5d6>] iput+0x84/0x8c [ 228.335000] [<c0172010>] do_unlinkat+0xe7/0x159 [ 228.335000] [<c0172094>] sys_unlink+0x12/0x14 [ 228.335000] [<c03845b2>] sysenter_past_esp+0x63/0xa1 [-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: 2.6.17-rc5-mm3-lockdep - locking error in quotaon 2006-06-05 19:20 ` Valdis.Kletnieks @ 2006-06-05 19:35 ` Jan Kara 2006-06-05 19:52 ` Arjan van de Ven 0 siblings, 1 reply; 11+ messages in thread From: Jan Kara @ 2006-06-05 19:35 UTC (permalink / raw) To: Valdis.Kletnieks Cc: Arjan van de Ven, linux-kernel, Andrew Morton, Ingo Molnar > On Mon, 05 Jun 2006 19:25:39 +0200, Arjan van de Ven said: > > > The quota code nests 3 mutexes but releases them in a totally different > > order; mark this as such in the code. > > > =================================================================== > > --- linux-2.6.17-rc5-mm3.orig/fs/dquot.c > > +++ linux-2.6.17-rc5-mm3/fs/dquot.c > > @@ -1475,7 +1475,7 @@ static int vfs_quota_on_inode(struct ino > > goto out_file_init; > > } > > mutex_unlock(&dqopt->dqio_mutex); > > - mutex_unlock(&inode->i_mutex); > > + mutex_unlock_non_nested(&inode->i_mutex); > > set_enable_flags(dqopt, type); > > > > add_dquot_ref(sb, type); > > Obviously, there be bigger and nastier dragons lurking in the quota code. > > This one made it past quotaon, but not as far as the swapon command in the > Fedora /etc/rc.sysinit. There's a bunch of rm all over the place in that > section of the script, and I'm not sure at all which one triggered it. I have not analyzed the backtraces to much in detail but I guess the following confuses the code (and I agree that it's kind of ugly from quota code to do that): i_mutex of inode containing quota file is acquired after all other quota locks. i_mutex of all other inodes is acquired before quota locks. Quota code makes sure (by resetting inode operations and setting special flag on inode) that noone tries to enter quota code while holding i_mutex on a quota file... Anyways it's nice checker that it caches this kind of things ;) None of the previous ones did. Honza ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: 2.6.17-rc5-mm3-lockdep - locking error in quotaon 2006-06-05 19:35 ` Jan Kara @ 2006-06-05 19:52 ` Arjan van de Ven 2006-06-05 20:06 ` Jan Kara 0 siblings, 1 reply; 11+ messages in thread From: Arjan van de Ven @ 2006-06-05 19:52 UTC (permalink / raw) To: Jan Kara; +Cc: Valdis.Kletnieks, linux-kernel, Andrew Morton, Ingo Molnar > following confuses the code (and I agree that it's kind of ugly from > quota code to do that): > i_mutex of inode containing quota file is acquired after all other > quota locks. i_mutex of all other inodes is acquired before quota locks. > Quota code makes sure (by resetting inode operations and setting special > flag on inode) that noone tries to enter quota code while holding > i_mutex on a quota file... can you point this out in a bit more detail, eg where exactly is this happening? I think it is in this bit /* As we bypass the pagecache we must now flush the inode so that * we see all the changes from userspace... */ write_inode_now(inode, 1); /* And now flush the block cache so that kernel sees the changes */ invalidate_bdev(sb->s_bdev, 0); mutex_lock(&inode->i_mutex); mutex_lock(&dqopt->dqonoff_mutex); if (sb_has_quota_enabled(sb, type)) { error = -EBUSY; goto out_lock; but that doesn't quite match your description... ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: 2.6.17-rc5-mm3-lockdep - locking error in quotaon 2006-06-05 19:52 ` Arjan van de Ven @ 2006-06-05 20:06 ` Jan Kara 2006-06-05 20:15 ` Ingo Molnar 2006-06-05 20:26 ` Arjan van de Ven 0 siblings, 2 replies; 11+ messages in thread From: Jan Kara @ 2006-06-05 20:06 UTC (permalink / raw) To: Arjan van de Ven Cc: Valdis.Kletnieks, linux-kernel, Andrew Morton, Ingo Molnar > > > following confuses the code (and I agree that it's kind of ugly from > > quota code to do that): > > i_mutex of inode containing quota file is acquired after all other > > quota locks. i_mutex of all other inodes is acquired before quota locks. > > Quota code makes sure (by resetting inode operations and setting special > > flag on inode) that noone tries to enter quota code while holding > > i_mutex on a quota file... > > can you point this out in a bit more detail, eg where exactly is this > happening? I think it is in this bit > /* As we bypass the pagecache we must now flush the inode so that > * we see all the changes from userspace... */ > write_inode_now(inode, 1); > /* And now flush the block cache so that kernel sees the changes > */ > invalidate_bdev(sb->s_bdev, 0); > mutex_lock(&inode->i_mutex); > mutex_lock(&dqopt->dqonoff_mutex); > if (sb_has_quota_enabled(sb, type)) { > error = -EBUSY; > goto out_lock; > > but that doesn't quite match your description... This piece of code is there just because we avoid page cache when doing quota writes. That is a different story and should cause problems with your lock checker. Standard way of running quota is: - get i_mutex for data_inode - write some data to data_inode - requires allocation -> calls DQUOT_ALLOC_SPACE - DQUOT_ALLOC_SPACE acquires some quota locks, decides it wants to write out quota structure (e.g. because we are journaling quota and must preserve quota integrity) - acquires dqio_sem, calls filesystem specific quota writing function - e.g. ext3_quota_write() - this function acquires i_mutex for quota file I think this is the type of circle your checker has found. Honza ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: 2.6.17-rc5-mm3-lockdep - locking error in quotaon 2006-06-05 20:06 ` Jan Kara @ 2006-06-05 20:15 ` Ingo Molnar 2006-06-05 20:26 ` Arjan van de Ven 1 sibling, 0 replies; 11+ messages in thread From: Ingo Molnar @ 2006-06-05 20:15 UTC (permalink / raw) To: Jan Kara; +Cc: Arjan van de Ven, Valdis.Kletnieks, linux-kernel, Andrew Morton * Jan Kara <jack@suse.cz> wrote: > > but that doesn't quite match your description... > This piece of code is there just because we avoid page cache when > doing quota writes. That is a different story and should cause problems > with your lock checker. > Standard way of running quota is: > - get i_mutex for data_inode > - write some data to data_inode > - requires allocation -> calls DQUOT_ALLOC_SPACE > - DQUOT_ALLOC_SPACE acquires some quota locks, decides it wants to > write out quota structure (e.g. because we are journaling quota and > must preserve quota integrity) > - acquires dqio_sem, calls filesystem specific quota writing > function - e.g. ext3_quota_write() > - this function acquires i_mutex for quota file > > I think this is the type of circle your checker has found. the validator noticed a circular dependency (AB->BA, or AB->BC->CA, etc.) - while the nesting above it would report as: "BUG: possible deadlock detected!". Ingo ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: 2.6.17-rc5-mm3-lockdep - locking error in quotaon 2006-06-05 20:06 ` Jan Kara 2006-06-05 20:15 ` Ingo Molnar @ 2006-06-05 20:26 ` Arjan van de Ven 2006-06-06 2:22 ` Valdis.Kletnieks 2006-06-06 8:33 ` Ingo Molnar 1 sibling, 2 replies; 11+ messages in thread From: Arjan van de Ven @ 2006-06-05 20:26 UTC (permalink / raw) To: Jan Kara; +Cc: Valdis.Kletnieks, linux-kernel, Andrew Morton, Ingo Molnar > - acquires dqio_sem, calls filesystem specific quota writing > function - e.g. ext3_quota_write() > - this function acquires i_mutex for quota file > > I think this is the type of circle your checker has found. ok since you know this doesn't deadlock the patch below (concept; akpm please don't apply yet) ought to describe this special locking situation to lockdep; I would really appreciate someone who does use quota to test this out and see if it works.... --- fs/dquot.c | 2 +- fs/ext2/super.c | 2 +- fs/ext3/super.c | 2 +- fs/reiserfs/super.c | 2 +- fs/ufs/super.c | 2 +- include/linux/fs.h | 7 ++++++- 6 files changed, 11 insertions(+), 6 deletions(-) Index: linux-2.6.17-rc5-mm3/fs/dquot.c =================================================================== --- linux-2.6.17-rc5-mm3.orig/fs/dquot.c +++ linux-2.6.17-rc5-mm3/fs/dquot.c @@ -1475,7 +1475,7 @@ static int vfs_quota_on_inode(struct ino goto out_file_init; } mutex_unlock(&dqopt->dqio_mutex); - mutex_unlock(&inode->i_mutex); + mutex_unlock_non_nested(&inode->i_mutex); set_enable_flags(dqopt, type); add_dquot_ref(sb, type); Index: linux-2.6.17-rc5-mm3/fs/ext2/super.c =================================================================== --- linux-2.6.17-rc5-mm3.orig/fs/ext2/super.c +++ linux-2.6.17-rc5-mm3/fs/ext2/super.c @@ -1157,7 +1157,7 @@ static ssize_t ext2_quota_write(struct s struct buffer_head tmp_bh; struct buffer_head *bh; - mutex_lock(&inode->i_mutex); + mutex_lock_nested(&inode->i_mutex, I_MUTEX_QUOTA); while (towrite > 0) { tocopy = sb->s_blocksize - offset < towrite ? sb->s_blocksize - offset : towrite; Index: linux-2.6.17-rc5-mm3/fs/ext3/super.c =================================================================== --- linux-2.6.17-rc5-mm3.orig/fs/ext3/super.c +++ linux-2.6.17-rc5-mm3/fs/ext3/super.c @@ -2614,7 +2614,7 @@ static ssize_t ext3_quota_write(struct s struct buffer_head *bh; handle_t *handle = journal_current_handle(); - mutex_lock(&inode->i_mutex); + mutex_lock_nested(&inode->i_mutex, I_MUTEX_QUOTA); while (towrite > 0) { tocopy = sb->s_blocksize - offset < towrite ? sb->s_blocksize - offset : towrite; Index: linux-2.6.17-rc5-mm3/fs/reiserfs/super.c =================================================================== --- linux-2.6.17-rc5-mm3.orig/fs/reiserfs/super.c +++ linux-2.6.17-rc5-mm3/fs/reiserfs/super.c @@ -2204,7 +2204,7 @@ static ssize_t reiserfs_quota_write(stru size_t towrite = len; struct buffer_head tmp_bh, *bh; - mutex_lock(&inode->i_mutex); + mutex_lock_nested(&inode->i_mutex, I_MUTEX_QUOTA); while (towrite > 0) { tocopy = sb->s_blocksize - offset < towrite ? sb->s_blocksize - offset : towrite; Index: linux-2.6.17-rc5-mm3/fs/ufs/super.c =================================================================== --- linux-2.6.17-rc5-mm3.orig/fs/ufs/super.c +++ linux-2.6.17-rc5-mm3/fs/ufs/super.c @@ -1269,7 +1269,7 @@ static ssize_t ufs_quota_write(struct su size_t towrite = len; struct buffer_head *bh; - mutex_lock(&inode->i_mutex); + mutex_lock_nested(&inode->i_mutex, I_MUTEX_QUOTA); while (towrite > 0) { tocopy = sb->s_blocksize - offset < towrite ? sb->s_blocksize - offset : towrite; Index: linux-2.6.17-rc5-mm3/include/linux/fs.h =================================================================== --- linux-2.6.17-rc5-mm3.orig/include/linux/fs.h +++ linux-2.6.17-rc5-mm3/include/linux/fs.h @@ -563,12 +563,17 @@ struct inode { * 0: the object of the current VFS operation * 1: parent * 2: child/target + * 3: quota file + * + * The locking order between these types is + * parent -> child -> normal -> quota */ enum inode_i_mutex_lock_type { I_MUTEX_NORMAL, I_MUTEX_PARENT, - I_MUTEX_CHILD + I_MUTEX_CHILD, + I_MUTEX_QUOTA }; /* ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: 2.6.17-rc5-mm3-lockdep - locking error in quotaon 2006-06-05 20:26 ` Arjan van de Ven @ 2006-06-06 2:22 ` Valdis.Kletnieks 2006-06-06 8:33 ` Ingo Molnar 1 sibling, 0 replies; 11+ messages in thread From: Valdis.Kletnieks @ 2006-06-06 2:22 UTC (permalink / raw) To: Arjan van de Ven; +Cc: Jan Kara, linux-kernel, Andrew Morton, Ingo Molnar [-- Attachment #1: Type: text/plain, Size: 548 bytes --] On Mon, 05 Jun 2006 22:26:23 +0200, Arjan van de Ven said: > ok since you know this doesn't deadlock the patch below (concept; akpm > please don't apply yet) ought to describe this special locking situation > to lockdep; I would really appreciate someone who does use quota to test > this out and see if it works.... It boots, quotas work, it doesn't issue BUG about the quota system. I'm however not qualified to comment on whether the patch is in fact *correct*. But if Jan is happy with it to, it's probably ready to be pushed to Andrew.... [-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: 2.6.17-rc5-mm3-lockdep - locking error in quotaon 2006-06-05 20:26 ` Arjan van de Ven 2006-06-06 2:22 ` Valdis.Kletnieks @ 2006-06-06 8:33 ` Ingo Molnar 1 sibling, 0 replies; 11+ messages in thread From: Ingo Molnar @ 2006-06-06 8:33 UTC (permalink / raw) To: Arjan van de Ven; +Cc: Jan Kara, Valdis.Kletnieks, linux-kernel, Andrew Morton * Arjan van de Ven <arjan@linux.intel.com> wrote: > ok since you know this doesn't deadlock the patch below (concept; akpm > please don't apply yet) ought to describe this special locking > situation to lockdep; I would really appreciate someone who does use > quota to test this out and see if it works.... looks good to me and doesnt produce messages on my system either. Acked-by: Ingo Molnar <mingo@elte.hu> Ingo ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2006-06-06 8:33 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2006-06-05 17:00 2.6.17-rc5-mm3-lockdep - locking error in quotaon Valdis.Kletnieks 2006-06-05 17:25 ` Arjan van de Ven 2006-06-05 17:35 ` Jan Kara 2006-06-05 19:20 ` Valdis.Kletnieks 2006-06-05 19:35 ` Jan Kara 2006-06-05 19:52 ` Arjan van de Ven 2006-06-05 20:06 ` Jan Kara 2006-06-05 20:15 ` Ingo Molnar 2006-06-05 20:26 ` Arjan van de Ven 2006-06-06 2:22 ` Valdis.Kletnieks 2006-06-06 8:33 ` Ingo Molnar
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox