All of lore.kernel.org
 help / color / mirror / Atom feed
* [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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.