* [Bug] possible circular locking in reiserfs_unpack @ 2010-09-05 11:31 Jarek Poplawski 2010-09-08 22:37 ` Andrew Morton 0 siblings, 1 reply; 8+ messages in thread From: Jarek Poplawski @ 2010-09-05 11:31 UTC (permalink / raw) To: linux-kernel; +Cc: reiserfs-devel Hi, I get this warning on every lilo write with 2.6.35.4 and a bit/git later too. Jarek P. [ 92.766639] ======================================================= [ 92.767222] [ INFO: possible circular locking dependency detected ] [ 92.767222] 2.6.35c #13 [ 92.767222] ------------------------------------------------------- [ 92.767222] lilo/1606 is trying to acquire lock: [ 92.767222] (&sb->s_type->i_mutex_key#8){+.+.+.}, at: [<d0329450>] reiserfs_unpack+0x60/0x110 [reiserfs] [ 92.767222] [ 92.767222] but task is already holding lock: [ 92.767222] (&REISERFS_SB(s)->lock){+.+.+.}, at: [<d032a268>] reiserfs_write_lock+0x28/0x40 [reiserfs] [ 92.767222] [ 92.767222] which lock already depends on the new lock. [ 92.767222] [ 92.767222] [ 92.767222] the existing dependency chain (in reverse order) is: [ 92.767222] [ 92.767222] -> #1 (&REISERFS_SB(s)->lock){+.+.+.}: [ 92.767222] [<c1056347>] lock_acquire+0x67/0x80 [ 92.767222] [<c12f083d>] __mutex_lock_common+0x4d/0x410 [ 92.767222] [<c12f0c58>] mutex_lock_nested+0x18/0x20 [ 92.767222] [<d032a268>] reiserfs_write_lock+0x28/0x40 [reiserfs] [ 92.767222] [<d0329e9a>] reiserfs_lookup_privroot+0x2a/0x90 [reiserfs] [ 92.767222] [<d0316b81>] reiserfs_fill_super+0x941/0xe60 [reiserfs] [ 92.767222] [<c10b7d17>] get_sb_bdev+0x117/0x170 [ 92.767222] [<d0313e21>] get_super_block+0x21/0x30 [reiserfs] [ 92.767222] [<c10b74ba>] vfs_kern_mount+0x6a/0x1b0 [ 92.767222] [<c10b7659>] do_kern_mount+0x39/0xe0 [ 92.767222] [<c10cebe0>] do_mount+0x340/0x790 [ 92.767222] [<c10cf0b4>] sys_mount+0x84/0xb0 [ 92.767222] [<c12f25cd>] syscall_call+0x7/0xb [ 92.767222] [ 92.767222] -> #0 (&sb->s_type->i_mutex_key#8){+.+.+.}: [ 92.767222] [<c1056186>] __lock_acquire+0x1026/0x1180 [ 92.767222] [<c1056347>] lock_acquire+0x67/0x80 [ 92.767222] [<c12f083d>] __mutex_lock_common+0x4d/0x410 [ 92.767222] [<c12f0c58>] mutex_lock_nested+0x18/0x20 [ 92.767222] [<d0329450>] reiserfs_unpack+0x60/0x110 [reiserfs] [ 92.767222] [<d0329772>] reiserfs_ioctl+0x272/0x320 [reiserfs] [ 92.767222] [<c10c3228>] vfs_ioctl+0x28/0xa0 [ 92.767222] [<c10c3c5d>] do_vfs_ioctl+0x32d/0x5c0 [ 92.767222] [<c10c3f53>] sys_ioctl+0x63/0x70 [ 92.767222] [<c12f25cd>] syscall_call+0x7/0xb [ 92.767222] [ 92.767222] other info that might help us debug this: [ 92.767222] [ 92.767222] 1 lock held by lilo/1606: [ 92.767222] #0: (&REISERFS_SB(s)->lock){+.+.+.}, at: [<d032a268>] reiserfs_write_lock+0x28/0x40 [reiserfs] [ 92.767222] [ 92.767222] stack backtrace: [ 92.767222] Pid: 1606, comm: lilo Not tainted 2.6.35c #13 [ 92.767222] Call Trace: [ 92.767222] [<c12ef64a>] ? printk+0x18/0x1e [ 92.767222] [<c1054212>] print_circular_bug+0xd2/0xe0 [ 92.767222] [<c1056186>] __lock_acquire+0x1026/0x1180 [ 92.767222] [<c1089489>] ? __generic_file_aio_write+0x1c9/0x550 [ 92.767222] [<c1056347>] lock_acquire+0x67/0x80 [ 92.767222] [<d0329450>] ? reiserfs_unpack+0x60/0x110 [reiserfs] [ 92.767222] [<c12f083d>] __mutex_lock_common+0x4d/0x410 [ 92.767222] [<d0329450>] ? reiserfs_unpack+0x60/0x110 [reiserfs] [ 92.767222] [<c12f0b08>] ? __mutex_lock_common+0x318/0x410 [ 92.767222] [<d032a268>] ? reiserfs_write_lock+0x28/0x40 [reiserfs] [ 92.767222] [<c12f0c58>] mutex_lock_nested+0x18/0x20 [ 92.767222] [<d0329450>] ? reiserfs_unpack+0x60/0x110 [reiserfs] [ 92.767222] [<d0329450>] reiserfs_unpack+0x60/0x110 [reiserfs] [ 92.767222] [<c12f0c58>] ? mutex_lock_nested+0x18/0x20 [ 92.767222] [<d0329772>] reiserfs_ioctl+0x272/0x320 [reiserfs] [ 92.767222] [<d0329500>] ? reiserfs_ioctl+0x0/0x320 [reiserfs] [ 92.767222] [<c10c3228>] vfs_ioctl+0x28/0xa0 [ 92.767222] [<c10c3c5d>] do_vfs_ioctl+0x32d/0x5c0 [ 92.767222] [<c109a428>] ? might_fault+0x88/0x90 [ 92.767222] [<c109a3e2>] ? might_fault+0x42/0x90 [ 92.767222] [<c10b6638>] ? fget_light+0xf8/0x2f0 [ 92.767222] [<c10c3f53>] sys_ioctl+0x63/0x70 [ 92.767222] [<c12f25cd>] syscall_call+0x7/0xb ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Bug] possible circular locking in reiserfs_unpack 2010-09-05 11:31 [Bug] possible circular locking in reiserfs_unpack Jarek Poplawski @ 2010-09-08 22:37 ` Andrew Morton 2010-09-09 1:53 ` Frederic Weisbecker 0 siblings, 1 reply; 8+ messages in thread From: Andrew Morton @ 2010-09-08 22:37 UTC (permalink / raw) To: Jarek Poplawski; +Cc: linux-kernel, reiserfs-devel, Christoph Hellwig, Al Viro On Sun, 5 Sep 2010 13:31:21 +0200 Jarek Poplawski <jarkao2@gmail.com> wrote: > Hi, > I get this warning on every lilo write with 2.6.35.4 and a bit/git > later too. > Can you tell us the latest kernel version which did *not* have this bug? That way we can narrow the problem down a bit. Thanks. > > [ 92.766639] ======================================================= > [ 92.767222] [ INFO: possible circular locking dependency detected ] > [ 92.767222] 2.6.35c #13 > [ 92.767222] ------------------------------------------------------- > [ 92.767222] lilo/1606 is trying to acquire lock: > [ 92.767222] (&sb->s_type->i_mutex_key#8){+.+.+.}, at: [<d0329450>] reiserfs_unpack+0x60/0x110 [reiserfs] > [ 92.767222] > [ 92.767222] but task is already holding lock: > [ 92.767222] (&REISERFS_SB(s)->lock){+.+.+.}, at: [<d032a268>] reiserfs_write_lock+0x28/0x40 [reiserfs] > [ 92.767222] > [ 92.767222] which lock already depends on the new lock. > [ 92.767222] > [ 92.767222] > [ 92.767222] the existing dependency chain (in reverse order) is: > [ 92.767222] > [ 92.767222] -> #1 (&REISERFS_SB(s)->lock){+.+.+.}: > [ 92.767222] [<c1056347>] lock_acquire+0x67/0x80 > [ 92.767222] [<c12f083d>] __mutex_lock_common+0x4d/0x410 > [ 92.767222] [<c12f0c58>] mutex_lock_nested+0x18/0x20 > [ 92.767222] [<d032a268>] reiserfs_write_lock+0x28/0x40 [reiserfs] > [ 92.767222] [<d0329e9a>] reiserfs_lookup_privroot+0x2a/0x90 [reiserfs] > [ 92.767222] [<d0316b81>] reiserfs_fill_super+0x941/0xe60 [reiserfs] > [ 92.767222] [<c10b7d17>] get_sb_bdev+0x117/0x170 > [ 92.767222] [<d0313e21>] get_super_block+0x21/0x30 [reiserfs] > [ 92.767222] [<c10b74ba>] vfs_kern_mount+0x6a/0x1b0 > [ 92.767222] [<c10b7659>] do_kern_mount+0x39/0xe0 > [ 92.767222] [<c10cebe0>] do_mount+0x340/0x790 > [ 92.767222] [<c10cf0b4>] sys_mount+0x84/0xb0 > [ 92.767222] [<c12f25cd>] syscall_call+0x7/0xb > [ 92.767222] > [ 92.767222] -> #0 (&sb->s_type->i_mutex_key#8){+.+.+.}: > [ 92.767222] [<c1056186>] __lock_acquire+0x1026/0x1180 > [ 92.767222] [<c1056347>] lock_acquire+0x67/0x80 > [ 92.767222] [<c12f083d>] __mutex_lock_common+0x4d/0x410 > [ 92.767222] [<c12f0c58>] mutex_lock_nested+0x18/0x20 > [ 92.767222] [<d0329450>] reiserfs_unpack+0x60/0x110 [reiserfs] > [ 92.767222] [<d0329772>] reiserfs_ioctl+0x272/0x320 [reiserfs] > [ 92.767222] [<c10c3228>] vfs_ioctl+0x28/0xa0 > [ 92.767222] [<c10c3c5d>] do_vfs_ioctl+0x32d/0x5c0 > [ 92.767222] [<c10c3f53>] sys_ioctl+0x63/0x70 > [ 92.767222] [<c12f25cd>] syscall_call+0x7/0xb > [ 92.767222] > [ 92.767222] other info that might help us debug this: > [ 92.767222] > [ 92.767222] 1 lock held by lilo/1606: > [ 92.767222] #0: (&REISERFS_SB(s)->lock){+.+.+.}, at: [<d032a268>] reiserfs_write_lock+0x28/0x40 [reiserfs] > [ 92.767222] > [ 92.767222] stack backtrace: > [ 92.767222] Pid: 1606, comm: lilo Not tainted 2.6.35c #13 > [ 92.767222] Call Trace: > [ 92.767222] [<c12ef64a>] ? printk+0x18/0x1e > [ 92.767222] [<c1054212>] print_circular_bug+0xd2/0xe0 > [ 92.767222] [<c1056186>] __lock_acquire+0x1026/0x1180 > [ 92.767222] [<c1089489>] ? __generic_file_aio_write+0x1c9/0x550 > [ 92.767222] [<c1056347>] lock_acquire+0x67/0x80 > [ 92.767222] [<d0329450>] ? reiserfs_unpack+0x60/0x110 [reiserfs] > [ 92.767222] [<c12f083d>] __mutex_lock_common+0x4d/0x410 > [ 92.767222] [<d0329450>] ? reiserfs_unpack+0x60/0x110 [reiserfs] > [ 92.767222] [<c12f0b08>] ? __mutex_lock_common+0x318/0x410 > [ 92.767222] [<d032a268>] ? reiserfs_write_lock+0x28/0x40 [reiserfs] > [ 92.767222] [<c12f0c58>] mutex_lock_nested+0x18/0x20 > [ 92.767222] [<d0329450>] ? reiserfs_unpack+0x60/0x110 [reiserfs] > [ 92.767222] [<d0329450>] reiserfs_unpack+0x60/0x110 [reiserfs] > [ 92.767222] [<c12f0c58>] ? mutex_lock_nested+0x18/0x20 > [ 92.767222] [<d0329772>] reiserfs_ioctl+0x272/0x320 [reiserfs] > [ 92.767222] [<d0329500>] ? reiserfs_ioctl+0x0/0x320 [reiserfs] > [ 92.767222] [<c10c3228>] vfs_ioctl+0x28/0xa0 > [ 92.767222] [<c10c3c5d>] do_vfs_ioctl+0x32d/0x5c0 > [ 92.767222] [<c109a428>] ? might_fault+0x88/0x90 > [ 92.767222] [<c109a3e2>] ? might_fault+0x42/0x90 > [ 92.767222] [<c10b6638>] ? fget_light+0xf8/0x2f0 > [ 92.767222] [<c10c3f53>] sys_ioctl+0x63/0x70 > [ 92.767222] [<c12f25cd>] syscall_call+0x7/0xb ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Bug] possible circular locking in reiserfs_unpack 2010-09-08 22:37 ` Andrew Morton @ 2010-09-09 1:53 ` Frederic Weisbecker 2010-09-09 6:07 ` Jarek Poplawski 2010-09-09 14:55 ` Jarek Poplawski 0 siblings, 2 replies; 8+ messages in thread From: Frederic Weisbecker @ 2010-09-09 1:53 UTC (permalink / raw) To: Andrew Morton Cc: Jarek Poplawski, linux-kernel, reiserfs-devel, Christoph Hellwig, Al Viro On Wed, Sep 08, 2010 at 03:37:30PM -0700, Andrew Morton wrote: > On Sun, 5 Sep 2010 13:31:21 +0200 > Jarek Poplawski <jarkao2@gmail.com> wrote: > > > Hi, > > I get this warning on every lilo write with 2.6.35.4 and a bit/git > > later too. > > > > Can you tell us the latest kernel version which did *not* have this > bug? That way we can narrow the problem down a bit. > > Thanks. Ah, when you see &REISERFS_SB(s)->lock in a bug report, don't hesitate to blame me :-) This is a problem resulting from the bkl conversion to a mutex that introduced a lot of new locking dependencies. Most of them have been fixed, but for less tested paths like ioctl, we hear about it later. Does the following patch fixes the issue? If so, I'll make a proper changelog and put the appropriate 2.6.33-35 stable tags for the backport. Thnaks! diff --git a/fs/reiserfs/ioctl.c b/fs/reiserfs/ioctl.c index f53505d..679d502 100644 --- a/fs/reiserfs/ioctl.c +++ b/fs/reiserfs/ioctl.c @@ -188,7 +188,7 @@ int reiserfs_unpack(struct inode *inode, struct file *filp) /* we need to make sure nobody is changing the file size beneath ** us */ - mutex_lock(&inode->i_mutex); + reiserfs_mutex_lock_safe(&inode->i_mutex, inode->i_sb); reiserfs_write_lock(inode->i_sb); write_from = inode->i_size & (blocksize - 1); ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Bug] possible circular locking in reiserfs_unpack 2010-09-09 1:53 ` Frederic Weisbecker @ 2010-09-09 6:07 ` Jarek Poplawski 2010-09-09 14:55 ` Jarek Poplawski 1 sibling, 0 replies; 8+ messages in thread From: Jarek Poplawski @ 2010-09-09 6:07 UTC (permalink / raw) To: Frederic Weisbecker Cc: Andrew Morton, linux-kernel, reiserfs-devel, Christoph Hellwig, Al Viro On Thu, Sep 09, 2010 at 03:53:52AM +0200, Frederic Weisbecker wrote: > On Wed, Sep 08, 2010 at 03:37:30PM -0700, Andrew Morton wrote: > > On Sun, 5 Sep 2010 13:31:21 +0200 > > Jarek Poplawski <jarkao2@gmail.com> wrote: > > > > > Hi, > > > I get this warning on every lilo write with 2.6.35.4 and a bit/git > > > later too. > > > > > > > Can you tell us the latest kernel version which did *not* have this > > bug? That way we can narrow the problem down a bit. I'll try if Frederic's patch doesn't help. But, generally, it seems with this kind of (not too long) lockdep info it should be much faster to have a look of somebody with a basic reiserfs locking knowledge.;-) > > > > Thanks. > > > > Ah, when you see &REISERFS_SB(s)->lock in a bug report, don't hesitate to blame me :-) > > This is a problem resulting from the bkl conversion to a mutex that introduced > a lot of new locking dependencies. Most of them have been fixed, but for less > tested paths like ioctl, we hear about it later. > > Does the following patch fixes the issue? > If so, I'll make a proper changelog and put the appropriate 2.6.33-35 stable > tags for the backport. I should be able to test it when back home (within 9 hours). Thanks, Jarek P. > > Thnaks! > > > diff --git a/fs/reiserfs/ioctl.c b/fs/reiserfs/ioctl.c > index f53505d..679d502 100644 > --- a/fs/reiserfs/ioctl.c > +++ b/fs/reiserfs/ioctl.c > @@ -188,7 +188,7 @@ int reiserfs_unpack(struct inode *inode, struct file *filp) > /* we need to make sure nobody is changing the file size beneath > ** us > */ > - mutex_lock(&inode->i_mutex); > + reiserfs_mutex_lock_safe(&inode->i_mutex, inode->i_sb); > reiserfs_write_lock(inode->i_sb); > > write_from = inode->i_size & (blocksize - 1); > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Re: [Bug] possible circular locking in reiserfs_unpack 2010-09-09 1:53 ` Frederic Weisbecker 2010-09-09 6:07 ` Jarek Poplawski @ 2010-09-09 14:55 ` Jarek Poplawski 2010-09-09 15:34 ` Frederic Weisbecker 2010-09-22 13:49 ` Frederic Weisbecker 1 sibling, 2 replies; 8+ messages in thread From: Jarek Poplawski @ 2010-09-09 14:55 UTC (permalink / raw) To: Frederic Weisbecker Cc: Andrew Morton, linux-kernel, reiserfs-devel, Christoph Hellwig, Al Viro Frederic Weisbecker wrote, On 12/23/-28158 08:59 PM: > On Wed, Sep 08, 2010 at 03:37:30PM -0700, Andrew Morton wrote: >> On Sun, 5 Sep 2010 13:31:21 +0200 >> Jarek Poplawski <jarkao2@gmail.com> wrote: >> >>> Hi, >>> I get this warning on every lilo write with 2.6.35.4 and a bit/git >>> later too. >>> >> Can you tell us the latest kernel version which did *not* have this >> bug? That way we can narrow the problem down a bit. >> >> Thanks. > > > > Ah, when you see &REISERFS_SB(s)->lock in a bug report, don't hesitate to blame me :-) > > This is a problem resulting from the bkl conversion to a mutex that introduced > a lot of new locking dependencies. Most of them have been fixed, but for less > tested paths like ioctl, we hear about it later. > > Does the following patch fixes the issue? > If so, I'll make a proper changelog and put the appropriate 2.6.33-35 stable > tags for the backport. > > Thnaks! > > > diff --git a/fs/reiserfs/ioctl.c b/fs/reiserfs/ioctl.c > index f53505d..679d502 100644 > --- a/fs/reiserfs/ioctl.c > +++ b/fs/reiserfs/ioctl.c > @@ -188,7 +188,7 @@ int reiserfs_unpack(struct inode *inode, struct file *filp) > /* we need to make sure nobody is changing the file size beneath > ** us > */ > - mutex_lock(&inode->i_mutex); > + reiserfs_mutex_lock_safe(&inode->i_mutex, inode->i_sb); > reiserfs_write_lock(inode->i_sb); > > write_from = inode->i_size & (blocksize - 1); > So, there is still a warning but a bit different now. Jarek P. [ 67.110273] ======================================================= [ 67.110313] [ INFO: possible circular locking dependency detected ] [ 67.110313] 2.6.35.4.4a #3 [ 67.110313] ------------------------------------------------------- [ 67.110313] lilo/1620 is trying to acquire lock: [ 67.110313] (&journal->j_mutex){+.+...}, at: [<d0325bff>] do_journal_begin_r+0x7f/0x340 [reiserfs] [ 67.110313] [ 67.110313] but task is already holding lock: [ 67.110313] (&REISERFS_SB(s)->lock){+.+.+.}, at: [<d032a278>] reiserfs_write_lock+0x28/0x40 [reiserfs] [ 67.110313] [ 67.110313] which lock already depends on the new lock. [ 67.110313] [ 67.110313] [ 67.110313] the existing dependency chain (in reverse order) is: [ 67.110313] [ 67.110313] -> #1 (&REISERFS_SB(s)->lock){+.+.+.}: [ 67.110313] [<c10562b7>] lock_acquire+0x67/0x80 [ 67.110313] [<c12facad>] __mutex_lock_common+0x4d/0x410 [ 67.110313] [<c12fb0c8>] mutex_lock_nested+0x18/0x20 [ 67.110313] [<d032a278>] reiserfs_write_lock+0x28/0x40 [reiserfs] [ 67.110313] [<d0325c06>] do_journal_begin_r+0x86/0x340 [reiserfs] [ 67.110313] [<d0325f77>] journal_begin+0x77/0x140 [reiserfs] [ 67.110313] [<d0315be4>] reiserfs_remount+0x224/0x530 [reiserfs] [ 67.110313] [<c10b6a20>] do_remount_sb+0x60/0x110 [ 67.110313] [<c10cee25>] do_mount+0x625/0x790 [ 67.110313] [<c10cf014>] sys_mount+0x84/0xb0 [ 67.110313] [<c12fca3d>] syscall_call+0x7/0xb [ 67.110313] [ 67.110313] -> #0 (&journal->j_mutex){+.+...}: [ 67.110313] [<c10560f6>] __lock_acquire+0x1026/0x1180 [ 67.110313] [<c10562b7>] lock_acquire+0x67/0x80 [ 67.110313] [<c12facad>] __mutex_lock_common+0x4d/0x410 [ 67.110313] [<c12fb0c8>] mutex_lock_nested+0x18/0x20 [ 67.110313] [<d0325bff>] do_journal_begin_r+0x7f/0x340 [reiserfs] [ 67.110313] [<d0325f77>] journal_begin+0x77/0x140 [reiserfs] [ 67.110313] [<d0326271>] reiserfs_persistent_transaction+0x41/0x90 [reiserfs] [ 67.110313] [<d030d06c>] reiserfs_get_block+0x22c/0x1530 [reiserfs] [ 67.110313] [<c10db9db>] __block_prepare_write+0x1bb/0x3a0 [ 67.110313] [<c10dbbe6>] block_prepare_write+0x26/0x40 [ 67.110313] [<d030b738>] reiserfs_prepare_write+0x88/0x170 [reiserfs] [ 67.110313] [<d03294d6>] reiserfs_unpack+0xe6/0x120 [reiserfs] [ 67.110313] [<d0329782>] reiserfs_ioctl+0x272/0x320 [reiserfs] [ 67.110313] [<c10c3188>] vfs_ioctl+0x28/0xa0 [ 67.110313] [<c10c3bbd>] do_vfs_ioctl+0x32d/0x5c0 [ 67.110313] [<c10c3eb3>] sys_ioctl+0x63/0x70 [ 67.110313] [<c12fca3d>] syscall_call+0x7/0xb [ 67.110313] [ 67.110313] other info that might help us debug this: [ 67.110313] [ 67.110313] 2 locks held by lilo/1620: [ 67.110313] #0: (&sb->s_type->i_mutex_key#8){+.+.+.}, at: [<d032945a>] reiserfs_unpack+0x6a/0x120 [reiserfs] [ 67.110313] #1: (&REISERFS_SB(s)->lock){+.+.+.}, at: [<d032a278>] reiserfs_write_lock+0x28/0x40 [reiserfs] [ 67.110313] [ 67.110313] stack backtrace: [ 67.110313] Pid: 1620, comm: lilo Not tainted 2.6.35.4.4a #3 [ 67.110313] Call Trace: [ 67.110313] [<c12f9aba>] ? printk+0x18/0x1e [ 67.110313] [<c1054182>] print_circular_bug+0xd2/0xe0 [ 67.110313] [<c10560f6>] __lock_acquire+0x1026/0x1180 [ 67.110313] [<c10562b7>] lock_acquire+0x67/0x80 [ 67.110313] [<d0325bff>] ? do_journal_begin_r+0x7f/0x340 [reiserfs] [ 67.110313] [<c12facad>] __mutex_lock_common+0x4d/0x410 [ 67.110313] [<d0325bff>] ? do_journal_begin_r+0x7f/0x340 [reiserfs] [ 67.110313] [<c1055275>] ? __lock_acquire+0x1a5/0x1180 [ 67.110313] [<c10897ae>] ? mempool_alloc_slab+0xe/0x10 [ 67.110313] [<c12fb0c8>] mutex_lock_nested+0x18/0x20 [ 67.110313] [<d0325bff>] ? do_journal_begin_r+0x7f/0x340 [reiserfs] [ 67.110313] [<d0325bff>] do_journal_begin_r+0x7f/0x340 [reiserfs] [ 67.110313] [<c1054bd2>] ? mark_held_locks+0x62/0x80 [ 67.110313] [<c10b163d>] ? kmem_cache_alloc+0x7d/0xb0 [ 67.110313] [<c1054e5c>] ? trace_hardirqs_on_caller+0x11c/0x160 [ 67.110313] [<d0325f77>] journal_begin+0x77/0x140 [reiserfs] [ 67.110313] [<d0326262>] ? reiserfs_persistent_transaction+0x32/0x90 [reiserfs] [ 67.110313] [<d0326271>] reiserfs_persistent_transaction+0x41/0x90 [reiserfs] [ 67.110313] [<d030d06c>] reiserfs_get_block+0x22c/0x1530 [reiserfs] [ 67.110313] [<c1055275>] ? __lock_acquire+0x1a5/0x1180 [ 67.110313] [<c12fc672>] ? _raw_spin_unlock_irq+0x22/0x50 [ 67.110313] [<c10b163d>] ? kmem_cache_alloc+0x7d/0xb0 [ 67.110313] [<c1054e5c>] ? trace_hardirqs_on_caller+0x11c/0x160 [ 67.110313] [<c102789b>] ? sub_preempt_count+0x7b/0xb0 [ 67.110313] [<c12fc457>] ? _raw_spin_unlock+0x27/0x40 [ 67.110313] [<c10db9db>] __block_prepare_write+0x1bb/0x3a0 [ 67.110313] [<c10dbbe6>] block_prepare_write+0x26/0x40 [ 67.110313] [<d030ce40>] ? reiserfs_get_block+0x0/0x1530 [reiserfs] [ 67.110313] [<d030b738>] reiserfs_prepare_write+0x88/0x170 [reiserfs] [ 67.110313] [<d030ce40>] ? reiserfs_get_block+0x0/0x1530 [reiserfs] [ 67.110313] [<d03294d6>] reiserfs_unpack+0xe6/0x120 [reiserfs] [ 67.110313] [<d0329782>] reiserfs_ioctl+0x272/0x320 [reiserfs] [ 67.110313] [<d0329510>] ? reiserfs_ioctl+0x0/0x320 [reiserfs] [ 67.110313] [<c10c3188>] vfs_ioctl+0x28/0xa0 [ 67.110313] [<c10c3bbd>] do_vfs_ioctl+0x32d/0x5c0 [ 67.110313] [<c109a228>] ? might_fault+0x88/0x90 [ 67.110313] [<c109a1e2>] ? might_fault+0x42/0x90 [ 67.110313] [<c10b6588>] ? fget_light+0xf8/0x2f0 [ 67.110313] [<c10c3eb3>] sys_ioctl+0x63/0x70 [ 67.110313] [<c12fca3d>] syscall_call+0x7/0xb [ 67.110313] [<c12f007b>] ? cookie_v6_check+0x44b/0x630 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Re: [Bug] possible circular locking in reiserfs_unpack 2010-09-09 14:55 ` Jarek Poplawski @ 2010-09-09 15:34 ` Frederic Weisbecker 2010-09-22 13:49 ` Frederic Weisbecker 1 sibling, 0 replies; 8+ messages in thread From: Frederic Weisbecker @ 2010-09-09 15:34 UTC (permalink / raw) To: Jarek Poplawski Cc: Andrew Morton, linux-kernel, reiserfs-devel, Christoph Hellwig, Al Viro On Thu, Sep 09, 2010 at 04:55:34PM +0200, Jarek Poplawski wrote: > Frederic Weisbecker wrote, On 12/23/-28158 08:59 PM: > > > On Wed, Sep 08, 2010 at 03:37:30PM -0700, Andrew Morton wrote: > >> On Sun, 5 Sep 2010 13:31:21 +0200 > >> Jarek Poplawski <jarkao2@gmail.com> wrote: > >> > >>> Hi, > >>> I get this warning on every lilo write with 2.6.35.4 and a bit/git > >>> later too. > >>> > >> Can you tell us the latest kernel version which did *not* have this > >> bug? That way we can narrow the problem down a bit. > >> > >> Thanks. > > > > > > > > Ah, when you see &REISERFS_SB(s)->lock in a bug report, don't hesitate to blame me :-) > > > > This is a problem resulting from the bkl conversion to a mutex that introduced > > a lot of new locking dependencies. Most of them have been fixed, but for less > > tested paths like ioctl, we hear about it later. > > > > Does the following patch fixes the issue? > > If so, I'll make a proper changelog and put the appropriate 2.6.33-35 stable > > tags for the backport. > > > > Thnaks! > > > > > > diff --git a/fs/reiserfs/ioctl.c b/fs/reiserfs/ioctl.c > > index f53505d..679d502 100644 > > --- a/fs/reiserfs/ioctl.c > > +++ b/fs/reiserfs/ioctl.c > > @@ -188,7 +188,7 @@ int reiserfs_unpack(struct inode *inode, struct file *filp) > > /* we need to make sure nobody is changing the file size beneath > > ** us > > */ > > - mutex_lock(&inode->i_mutex); > > + reiserfs_mutex_lock_safe(&inode->i_mutex, inode->i_sb); > > reiserfs_write_lock(inode->i_sb); > > > > write_from = inode->i_size & (blocksize - 1); > > > > > So, there is still a warning but a bit different now. > > Jarek P. That's another bug, due to the fact we sometimes recursively acquire the reiserfs big lock. Anyway, will have a look this evening. Thanks. > [ 67.110273] ======================================================= > [ 67.110313] [ INFO: possible circular locking dependency detected ] > [ 67.110313] 2.6.35.4.4a #3 > [ 67.110313] ------------------------------------------------------- > [ 67.110313] lilo/1620 is trying to acquire lock: > [ 67.110313] (&journal->j_mutex){+.+...}, at: [<d0325bff>] do_journal_begin_r+0x7f/0x340 [reiserfs] > [ 67.110313] > [ 67.110313] but task is already holding lock: > [ 67.110313] (&REISERFS_SB(s)->lock){+.+.+.}, at: [<d032a278>] reiserfs_write_lock+0x28/0x40 [reiserfs] > [ 67.110313] > [ 67.110313] which lock already depends on the new lock. > [ 67.110313] > [ 67.110313] > [ 67.110313] the existing dependency chain (in reverse order) is: > [ 67.110313] > [ 67.110313] -> #1 (&REISERFS_SB(s)->lock){+.+.+.}: > [ 67.110313] [<c10562b7>] lock_acquire+0x67/0x80 > [ 67.110313] [<c12facad>] __mutex_lock_common+0x4d/0x410 > [ 67.110313] [<c12fb0c8>] mutex_lock_nested+0x18/0x20 > [ 67.110313] [<d032a278>] reiserfs_write_lock+0x28/0x40 [reiserfs] > [ 67.110313] [<d0325c06>] do_journal_begin_r+0x86/0x340 [reiserfs] > [ 67.110313] [<d0325f77>] journal_begin+0x77/0x140 [reiserfs] > [ 67.110313] [<d0315be4>] reiserfs_remount+0x224/0x530 [reiserfs] > [ 67.110313] [<c10b6a20>] do_remount_sb+0x60/0x110 > [ 67.110313] [<c10cee25>] do_mount+0x625/0x790 > [ 67.110313] [<c10cf014>] sys_mount+0x84/0xb0 > [ 67.110313] [<c12fca3d>] syscall_call+0x7/0xb > [ 67.110313] > [ 67.110313] -> #0 (&journal->j_mutex){+.+...}: > [ 67.110313] [<c10560f6>] __lock_acquire+0x1026/0x1180 > [ 67.110313] [<c10562b7>] lock_acquire+0x67/0x80 > [ 67.110313] [<c12facad>] __mutex_lock_common+0x4d/0x410 > [ 67.110313] [<c12fb0c8>] mutex_lock_nested+0x18/0x20 > [ 67.110313] [<d0325bff>] do_journal_begin_r+0x7f/0x340 [reiserfs] > [ 67.110313] [<d0325f77>] journal_begin+0x77/0x140 [reiserfs] > [ 67.110313] [<d0326271>] reiserfs_persistent_transaction+0x41/0x90 [reiserfs] > [ 67.110313] [<d030d06c>] reiserfs_get_block+0x22c/0x1530 [reiserfs] > [ 67.110313] [<c10db9db>] __block_prepare_write+0x1bb/0x3a0 > [ 67.110313] [<c10dbbe6>] block_prepare_write+0x26/0x40 > [ 67.110313] [<d030b738>] reiserfs_prepare_write+0x88/0x170 [reiserfs] > [ 67.110313] [<d03294d6>] reiserfs_unpack+0xe6/0x120 [reiserfs] > [ 67.110313] [<d0329782>] reiserfs_ioctl+0x272/0x320 [reiserfs] > [ 67.110313] [<c10c3188>] vfs_ioctl+0x28/0xa0 > [ 67.110313] [<c10c3bbd>] do_vfs_ioctl+0x32d/0x5c0 > [ 67.110313] [<c10c3eb3>] sys_ioctl+0x63/0x70 > [ 67.110313] [<c12fca3d>] syscall_call+0x7/0xb > [ 67.110313] > [ 67.110313] other info that might help us debug this: > [ 67.110313] > [ 67.110313] 2 locks held by lilo/1620: > [ 67.110313] #0: (&sb->s_type->i_mutex_key#8){+.+.+.}, at: [<d032945a>] reiserfs_unpack+0x6a/0x120 [reiserfs] > [ 67.110313] #1: (&REISERFS_SB(s)->lock){+.+.+.}, at: [<d032a278>] reiserfs_write_lock+0x28/0x40 [reiserfs] > [ 67.110313] > [ 67.110313] stack backtrace: > [ 67.110313] Pid: 1620, comm: lilo Not tainted 2.6.35.4.4a #3 > [ 67.110313] Call Trace: > [ 67.110313] [<c12f9aba>] ? printk+0x18/0x1e > [ 67.110313] [<c1054182>] print_circular_bug+0xd2/0xe0 > [ 67.110313] [<c10560f6>] __lock_acquire+0x1026/0x1180 > [ 67.110313] [<c10562b7>] lock_acquire+0x67/0x80 > [ 67.110313] [<d0325bff>] ? do_journal_begin_r+0x7f/0x340 [reiserfs] > [ 67.110313] [<c12facad>] __mutex_lock_common+0x4d/0x410 > [ 67.110313] [<d0325bff>] ? do_journal_begin_r+0x7f/0x340 [reiserfs] > [ 67.110313] [<c1055275>] ? __lock_acquire+0x1a5/0x1180 > [ 67.110313] [<c10897ae>] ? mempool_alloc_slab+0xe/0x10 > [ 67.110313] [<c12fb0c8>] mutex_lock_nested+0x18/0x20 > [ 67.110313] [<d0325bff>] ? do_journal_begin_r+0x7f/0x340 [reiserfs] > [ 67.110313] [<d0325bff>] do_journal_begin_r+0x7f/0x340 [reiserfs] > [ 67.110313] [<c1054bd2>] ? mark_held_locks+0x62/0x80 > [ 67.110313] [<c10b163d>] ? kmem_cache_alloc+0x7d/0xb0 > [ 67.110313] [<c1054e5c>] ? trace_hardirqs_on_caller+0x11c/0x160 > [ 67.110313] [<d0325f77>] journal_begin+0x77/0x140 [reiserfs] > [ 67.110313] [<d0326262>] ? reiserfs_persistent_transaction+0x32/0x90 [reiserfs] > [ 67.110313] [<d0326271>] reiserfs_persistent_transaction+0x41/0x90 [reiserfs] > [ 67.110313] [<d030d06c>] reiserfs_get_block+0x22c/0x1530 [reiserfs] > [ 67.110313] [<c1055275>] ? __lock_acquire+0x1a5/0x1180 > [ 67.110313] [<c12fc672>] ? _raw_spin_unlock_irq+0x22/0x50 > [ 67.110313] [<c10b163d>] ? kmem_cache_alloc+0x7d/0xb0 > [ 67.110313] [<c1054e5c>] ? trace_hardirqs_on_caller+0x11c/0x160 > [ 67.110313] [<c102789b>] ? sub_preempt_count+0x7b/0xb0 > [ 67.110313] [<c12fc457>] ? _raw_spin_unlock+0x27/0x40 > [ 67.110313] [<c10db9db>] __block_prepare_write+0x1bb/0x3a0 > [ 67.110313] [<c10dbbe6>] block_prepare_write+0x26/0x40 > [ 67.110313] [<d030ce40>] ? reiserfs_get_block+0x0/0x1530 [reiserfs] > [ 67.110313] [<d030b738>] reiserfs_prepare_write+0x88/0x170 [reiserfs] > [ 67.110313] [<d030ce40>] ? reiserfs_get_block+0x0/0x1530 [reiserfs] > [ 67.110313] [<d03294d6>] reiserfs_unpack+0xe6/0x120 [reiserfs] > [ 67.110313] [<d0329782>] reiserfs_ioctl+0x272/0x320 [reiserfs] > [ 67.110313] [<d0329510>] ? reiserfs_ioctl+0x0/0x320 [reiserfs] > [ 67.110313] [<c10c3188>] vfs_ioctl+0x28/0xa0 > [ 67.110313] [<c10c3bbd>] do_vfs_ioctl+0x32d/0x5c0 > [ 67.110313] [<c109a228>] ? might_fault+0x88/0x90 > [ 67.110313] [<c109a1e2>] ? might_fault+0x42/0x90 > [ 67.110313] [<c10b6588>] ? fget_light+0xf8/0x2f0 > [ 67.110313] [<c10c3eb3>] sys_ioctl+0x63/0x70 > [ 67.110313] [<c12fca3d>] syscall_call+0x7/0xb > [ 67.110313] [<c12f007b>] ? cookie_v6_check+0x44b/0x630 > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Re: [Bug] possible circular locking in reiserfs_unpack 2010-09-09 14:55 ` Jarek Poplawski 2010-09-09 15:34 ` Frederic Weisbecker @ 2010-09-22 13:49 ` Frederic Weisbecker 2010-09-22 17:22 ` Jarek Poplawski 1 sibling, 1 reply; 8+ messages in thread From: Frederic Weisbecker @ 2010-09-22 13:49 UTC (permalink / raw) To: Jarek Poplawski Cc: Andrew Morton, linux-kernel, reiserfs-devel, Christoph Hellwig, Al Viro On Thu, Sep 09, 2010 at 04:55:34PM +0200, Jarek Poplawski wrote: > Frederic Weisbecker wrote, On 12/23/-28158 08:59 PM: > > > On Wed, Sep 08, 2010 at 03:37:30PM -0700, Andrew Morton wrote: > >> On Sun, 5 Sep 2010 13:31:21 +0200 > >> Jarek Poplawski <jarkao2@gmail.com> wrote: > >> > >>> Hi, > >>> I get this warning on every lilo write with 2.6.35.4 and a bit/git > >>> later too. > >>> > >> Can you tell us the latest kernel version which did *not* have this > >> bug? That way we can narrow the problem down a bit. > >> > >> Thanks. > > > > > > > > Ah, when you see &REISERFS_SB(s)->lock in a bug report, don't hesitate to blame me :-) > > > > This is a problem resulting from the bkl conversion to a mutex that introduced > > a lot of new locking dependencies. Most of them have been fixed, but for less > > tested paths like ioctl, we hear about it later. > > > > Does the following patch fixes the issue? > > If so, I'll make a proper changelog and put the appropriate 2.6.33-35 stable > > tags for the backport. > > > > Thnaks! > > > > > > diff --git a/fs/reiserfs/ioctl.c b/fs/reiserfs/ioctl.c > > index f53505d..679d502 100644 > > --- a/fs/reiserfs/ioctl.c > > +++ b/fs/reiserfs/ioctl.c > > @@ -188,7 +188,7 @@ int reiserfs_unpack(struct inode *inode, struct file *filp) > > /* we need to make sure nobody is changing the file size beneath > > ** us > > */ > > - mutex_lock(&inode->i_mutex); > > + reiserfs_mutex_lock_safe(&inode->i_mutex, inode->i_sb); > > reiserfs_write_lock(inode->i_sb); > > > > write_from = inode->i_size & (blocksize - 1); > > > > > So, there is still a warning but a bit different now. > > Jarek P. I can reproduce your first case, but not this one. So, I hope you can give a try to the following fix, Thanks! diff --git a/fs/reiserfs/ioctl.c b/fs/reiserfs/ioctl.c index f53505d..90a757b 100644 --- a/fs/reiserfs/ioctl.c +++ b/fs/reiserfs/ioctl.c @@ -170,7 +170,7 @@ int reiserfs_prepare_write(struct file *f, struct page *page, int reiserfs_unpack(struct inode *inode, struct file *filp) { int retval = 0; - int index; + int index, depth; struct page *page; struct address_space *mapping; unsigned long write_from; @@ -185,11 +185,12 @@ int reiserfs_unpack(struct inode *inode, struct file *filp) return 0; } + depth = reiserfs_write_lock_once(inode->i_sb); + /* we need to make sure nobody is changing the file size beneath ** us */ - mutex_lock(&inode->i_mutex); - reiserfs_write_lock(inode->i_sb); + reiserfs_mutex_lock_safe(&inode->i_mutex, inode->i_sb); write_from = inode->i_size & (blocksize - 1); /* if we are on a block boundary, we are already unpacked. */ @@ -224,6 +225,6 @@ int reiserfs_unpack(struct inode *inode, struct file *filp) out: mutex_unlock(&inode->i_mutex); - reiserfs_write_unlock(inode->i_sb); + reiserfs_write_unlock_once(inode->i_sb, depth); return retval; } ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: Re: [Bug] possible circular locking in reiserfs_unpack 2010-09-22 13:49 ` Frederic Weisbecker @ 2010-09-22 17:22 ` Jarek Poplawski 0 siblings, 0 replies; 8+ messages in thread From: Jarek Poplawski @ 2010-09-22 17:22 UTC (permalink / raw) To: Frederic Weisbecker Cc: Andrew Morton, linux-kernel, reiserfs-devel, Christoph Hellwig, Al Viro On Wed, Sep 22, 2010 at 03:49:48PM +0200, Frederic Weisbecker wrote: > > I can reproduce your first case, but not this one. > > So, I hope you can give a try to the following fix, > > Thanks! > With this patch I don't have any lockdep warnings during lilo writes. Well done, thanks! Tested-by: Jarek Poplawski <jarkao2@gmail.com> > > diff --git a/fs/reiserfs/ioctl.c b/fs/reiserfs/ioctl.c > index f53505d..90a757b 100644 > --- a/fs/reiserfs/ioctl.c > +++ b/fs/reiserfs/ioctl.c > @@ -170,7 +170,7 @@ int reiserfs_prepare_write(struct file *f, struct page *page, > int reiserfs_unpack(struct inode *inode, struct file *filp) > { > int retval = 0; > - int index; > + int index, depth; > struct page *page; > struct address_space *mapping; > unsigned long write_from; > @@ -185,11 +185,12 @@ int reiserfs_unpack(struct inode *inode, struct file *filp) > return 0; > } > > + depth = reiserfs_write_lock_once(inode->i_sb); > + > /* we need to make sure nobody is changing the file size beneath > ** us > */ > - mutex_lock(&inode->i_mutex); > - reiserfs_write_lock(inode->i_sb); > + reiserfs_mutex_lock_safe(&inode->i_mutex, inode->i_sb); > > write_from = inode->i_size & (blocksize - 1); > /* if we are on a block boundary, we are already unpacked. */ > @@ -224,6 +225,6 @@ int reiserfs_unpack(struct inode *inode, struct file *filp) > > out: > mutex_unlock(&inode->i_mutex); > - reiserfs_write_unlock(inode->i_sb); > + reiserfs_write_unlock_once(inode->i_sb, depth); > return retval; > } > ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2010-09-22 17:22 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-09-05 11:31 [Bug] possible circular locking in reiserfs_unpack Jarek Poplawski 2010-09-08 22:37 ` Andrew Morton 2010-09-09 1:53 ` Frederic Weisbecker 2010-09-09 6:07 ` Jarek Poplawski 2010-09-09 14:55 ` Jarek Poplawski 2010-09-09 15:34 ` Frederic Weisbecker 2010-09-22 13:49 ` Frederic Weisbecker 2010-09-22 17:22 ` Jarek Poplawski
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).