btrfs: still lockdep splat for 4.9-rc5+ (btrfs_log_inode)

btrfs: still lockdep splat for 4.9-rc5+ (btrfs_log_inode)

[Christian Borntraeger]

FWIW, I still see the lockdep splat in btrfs in 4.9-rc5+

[  159.698343] =============================================
[  159.698345] [ INFO: possible recursive locking detected ]
[  159.698347] 4.9.0-rc5+ #136 Tainted: G        W      
[  159.698348] ---------------------------------------------
[  159.698349] vim/6913 is trying to acquire lock:
[  159.698350]  (
[  159.698351] &ei->log_mutex
[  159.698352] ){+.+...}
[  159.698353] , at: 
[  159.698445] [<000003ff8202c3f4>] btrfs_log_inode+0x124/0x1048 [btrfs]
[  159.698447] 
               but task is already holding lock:
[  159.698448]  (
[  159.698449] &ei->log_mutex
[  159.698449] ){+.+...}
[  159.698450] , at: 
[  159.698469] [<000003ff8202c3f4>] btrfs_log_inode+0x124/0x1048 [btrfs]
[  159.698470] 
               other info that might help us debug this:
[  159.698471]  Possible unsafe locking scenario:

[  159.698472]        CPU0
[  159.698473]        ----
[  159.698474]   lock(
[  159.698475] &ei->log_mutex
[  159.698476] );
[  159.698476]   lock(
[  159.698477] &ei->log_mutex
[  159.698478] );
[  159.698479] 
                *** DEADLOCK ***

[  159.698480]  May be due to missing lock nesting notation

[  159.698482] 3 locks held by vim/6913:
[  159.698483]  #0: 
[  159.698483]  (
[  159.698484] &sb->s_type->i_mutex_key
[  159.698508] #15
[  159.698509] ){+.+.+.}
[  159.698509] , at: 
[  159.698528] [<000003ff81ff66f8>] btrfs_sync_file+0x1b8/0x560 [btrfs]
[  159.698529]  #1: 
[  159.698530]  (
[  159.698531] sb_internal
[  159.698531] #2
[  159.698532] ){.+.+..}
[  159.698532] , at: 
[  159.698551] [<000003ff81fdad2a>] start_transaction+0x312/0x600 [btrfs]
[  159.698552]  #2: 
[  159.698552]  (
[  159.698553] &ei->log_mutex
[  159.698554] ){+.+...}
[  159.698555] , at: 
[  159.698573] [<000003ff8202c3f4>] btrfs_log_inode+0x124/0x1048 [btrfs]
[  159.698574] 
               stack backtrace:
[  159.698577] CPU: 22 PID: 6913 Comm: vim Tainted: G        W       4.9.0-rc5+ #136
[  159.698578] Hardware name: IBM              2964 NC9              704              (LPAR)
[  159.698580] Stack:
[  159.698581]        000000fae55635f0 000000fae5563680 0000000000000003 0000000000000000
[  159.698584]        000000fae5563720 000000fae5563698 000000fae5563698 0000000000000020
[  159.698587]        0000000000000000 000000fa00000020 000000fa0000000a 000000000000000a
[  159.698589]        000000000000000c 000000fae55636e8 0000000000000000 0000000000000000
[  159.698592]        04000000037e5c40 00000000001127a4 000000fae5563680 000000fae55636d8
[  159.698594] Call Trace:
[  159.698599] ([<000000000011266a>] show_trace+0xea/0xf0)
[  159.698601]  [<0000000000112748>] show_stack+0x68/0xe0 
[  159.698605]  [<00000000004ef502>] dump_stack+0x9a/0xd8 
[  159.698609]  [<00000000001a6078>] validate_chain.isra.22+0xbd8/0xd48 
[  159.698611]  [<00000000001a741c>] __lock_acquire+0x304/0x7f0 
[  159.698613]  [<00000000001a7fe6>] lock_acquire+0xfe/0x2d8 
[  159.698617]  [<00000000008c5216>] mutex_lock_nested+0x86/0x3e8 
[  159.698636]  [<000003ff8202c3f4>] btrfs_log_inode+0x124/0x1048 [btrfs] 
[  159.698655]  [<000003ff8202cfcc>] btrfs_log_inode+0xcfc/0x1048 [btrfs] 
[  159.698674]  [<000003ff8202d5bc>] btrfs_log_inode_parent+0x1fc/0x918 [btrfs] 
[  159.698693]  [<000003ff8202ef22>] btrfs_log_dentry_safe+0x7a/0xa0 [btrfs] 
[  159.698712]  [<000003ff81ff68fc>] btrfs_sync_file+0x3bc/0x560 [btrfs] 
[  159.698715]  [<0000000000349ade>] do_fsync+0x5e/0x90 
[  159.698716]  [<0000000000349e6a>] SyS_fsync+0x32/0x40 
[  159.698718]  [<00000000008cae8e>] system_call+0xd6/0x270 
[  159.698719] INFO: lockdep is turned off.

Re: btrfs: still lockdep splat for 4.9-rc5+ (btrfs_log_inode)

[Chris Mason]

On Fri, Nov 25, 2016 at 10:03:25AM +0100, Christian Borntraeger wrote:
>FWIW, I still see the lockdep splat in btrfs in 4.9-rc5+

Filipe reworked the code to avoid taking the same lock twice.  As far as 
I can tell, this just needs some annotation.

-chris

Re: btrfs: still lockdep splat for 4.9-rc5+ (btrfs_log_inode)

[Liu Bo]

On Sat, Nov 26, 2016 at 08:46:38AM -0500, Chris Mason wrote:
> On Fri, Nov 25, 2016 at 10:03:25AM +0100, Christian Borntraeger wrote:
> > FWIW, I still see the lockdep splat in btrfs in 4.9-rc5+
> 
> Filipe reworked the code to avoid taking the same lock twice.  As far as I
> can tell, this just needs some annotation.

Besides this log_mutex, we have another false lockdep warning around
block_group_cache->data_rwsem, we can use down_read_nested() to get rid
of it.

Thanks,

-liubo

[PATCH 0/1] btrfs lockdep annotation

[Christian Borntraeger]

Chris,

since my bug report about this did not result in any fix and since
this disables lockdep before the the code that I want to debug runs
here is my attempt to fix it.
Please double check if the subclass looks right. It seems to work
for me but I do not know enough about btrfs to decide if this is
right or not.

Christian Borntraeger ():
  btrfs: add lockdep annotation for btrfs_log_inode

 fs/btrfs/tree-log.c    | 2 +-

-- 
2.7.4

[PATCH 2/2] btrfs: add lockdep annotation for btrfs_log_inode

[Christian Borntraeger]

Add a proper subclass to get rid of the following lockdep
error.

 [ INFO: possible recursive locking detected ]
 4.9.0+ #279 Not tainted
 ---------------------------------------------
 vim/4801 is trying to acquire lock:
  (&ei->log_mutex){+.+...}, at: [<000003ff82057592>]
btrfs_log_inode+0x182/0xfa8 [btrfs]

  (&ei->log_mutex){+.+...}, at: [<000003ff82057592>]
btrfs_log_inode+0x182/0xfa8 [btrfs]

  Possible unsafe locking scenario:
        CPU0
        ----
   lock(&ei->log_mutex);
   lock(&ei->log_mutex);

                                 *** DEADLOCK ***
  May be due to missing lock nesting notation
 3 locks held by vim/4801:
  #0:  (&sb->s_type->i_mutex_key#15){+.+.+.}, at: [<000003ff81fc274c>]
btrfs_sync_file+0x204/0x728 [btrfs]
  #1:  (sb_internal#2){.+.+..}, at: [<000003ff81fa38e0>]
start_transaction+0x318/0x770 [btrfs]
  #2:  (&ei->log_mutex){+.+...}, at: [<000003ff82057592>]

[...]
 Call Trace:
 ([<0000000000115ffc>] show_trace+0xe4/0x108)
  [<00000000001160f8>] show_stack+0x68/0xe0
  [<0000000000652d52>] dump_stack+0x9a/0xd8
  [<0000000000209bb0>] __lock_acquire+0xac8/0x1bd0
  [<000000000020b3c6>] lock_acquire+0x106/0x4a0
  [<0000000000a1fb36>] mutex_lock_nested+0xa6/0x428
  [<000003ff82057592>] btrfs_log_inode+0x182/0xfa8 [btrfs]
  [<000003ff82057c76>] btrfs_log_inode+0x866/0xfa8 [btrfs]
  [<000003ff81ffe278>] btrfs_log_inode_parent+0x218/0x988 [btrfs]
  [<000003ff81ffffaa>] btrfs_log_dentry_safe+0x7a/0xa0 [btrfs]
  [<000003ff81fc29b6>] btrfs_sync_file+0x46e/0x728 [btrfs]
  [<000000000044aeee>] do_fsync+0x5e/0x90
  [<000000000044b2ba>] SyS_fsync+0x32/0x40
  [<0000000000a26786>] system_call+0xd6/0x288

Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 fs/btrfs/tree-log.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
index 3d33c4e..a3ec717 100644
--- a/fs/btrfs/tree-log.c
+++ b/fs/btrfs/tree-log.c
@@ -4648,7 +4648,7 @@ static int btrfs_log_inode(struct btrfs_trans_handle *trans,
 		return ret;
 	}
 
-	mutex_lock(&BTRFS_I(inode)->log_mutex);
+	mutex_lock_nested(&BTRFS_I(inode)->log_mutex, inode_only);
 
 	/*
 	 * a brute force approach to making sure we get the most uptodate
-- 
2.7.4

Re: [PATCH 0/1] btrfs lockdep annotation

[Filipe Manana]

On Tue, Jan 24, 2017 at 9:01 AM, Christian Borntraeger
<borntraeger@de.ibm.com> wrote:
> Chris,
>
> since my bug report about this did not result in any fix and since

It was fixed and the fix landed in 4.10-rc4:

https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=781feef7e6befafd4d9787d1f7ada1f9ccd504e4

> this disables lockdep before the the code that I want to debug runs
> here is my attempt to fix it.
> Please double check if the subclass looks right. It seems to work
> for me but I do not know enough about btrfs to decide if this is
> right or not.
>
> Christian Borntraeger ():
>   btrfs: add lockdep annotation for btrfs_log_inode
>
>  fs/btrfs/tree-log.c    | 2 +-
>
> --
> 2.7.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Filipe David Manana,

"People will forget what you said,
 people will forget what you did,
 but people will never forget how you made them feel."

Re: [PATCH 0/1] btrfs lockdep annotation

[Christian Borntraeger]

On 01/24/2017 11:22 AM, Filipe Manana wrote:
> On Tue, Jan 24, 2017 at 9:01 AM, Christian Borntraeger
> <borntraeger@de.ibm.com> wrote:
>> Chris,
>>
>> since my bug report about this did not result in any fix and since
> 
> It was fixed and the fix landed in 4.10-rc4:

Thanks, I missed that last pull.

> 
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=781feef7e6befafd4d9787d1f7ada1f9ccd504e4
> 
>> this disables lockdep before the the code that I want to debug runs
>> here is my attempt to fix it.
>> Please double check if the subclass looks right. It seems to work
>> for me but I do not know enough about btrfs to decide if this is
>> right or not.
>>
>> Christian Borntraeger ():
>>   btrfs: add lockdep annotation for btrfs_log_inode
>>
>>  fs/btrfs/tree-log.c    | 2 +-
>>
>> --
>> 2.7.4
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 
>