Skip to content

Commit b40130b

Browse files
josefbacikkdave
authored andcommitted
btrfs: fix lockdep splat with reloc root extent buffers
We have been hitting the following lockdep splat with btrfs/187 recently WARNING: possible circular locking dependency detected 5.19.0-rc8+ #775 Not tainted ------------------------------------------------------ btrfs/752500 is trying to acquire lock: ffff97e1875a97b8 (btrfs-treloc-02#2){+.+.}-{3:3}, at: __btrfs_tree_lock+0x24/0x110 but task is already holding lock: ffff97e1875a9278 (btrfs-tree-01/1){+.+.}-{3:3}, at: __btrfs_tree_lock+0x24/0x110 which lock already depends on the new lock. the existing dependency chain (in reverse order) is: -> #2 (btrfs-tree-01/1){+.+.}-{3:3}: down_write_nested+0x41/0x80 __btrfs_tree_lock+0x24/0x110 btrfs_init_new_buffer+0x7d/0x2c0 btrfs_alloc_tree_block+0x120/0x3b0 __btrfs_cow_block+0x136/0x600 btrfs_cow_block+0x10b/0x230 btrfs_search_slot+0x53b/0xb70 btrfs_lookup_inode+0x2a/0xa0 __btrfs_update_delayed_inode+0x5f/0x280 btrfs_async_run_delayed_root+0x24c/0x290 btrfs_work_helper+0xf2/0x3e0 process_one_work+0x271/0x590 worker_thread+0x52/0x3b0 kthread+0xf0/0x120 ret_from_fork+0x1f/0x30 -> #1 (btrfs-tree-01){++++}-{3:3}: down_write_nested+0x41/0x80 __btrfs_tree_lock+0x24/0x110 btrfs_search_slot+0x3c3/0xb70 do_relocation+0x10c/0x6b0 relocate_tree_blocks+0x317/0x6d0 relocate_block_group+0x1f1/0x560 btrfs_relocate_block_group+0x23e/0x400 btrfs_relocate_chunk+0x4c/0x140 btrfs_balance+0x755/0xe40 btrfs_ioctl+0x1ea2/0x2c90 __x64_sys_ioctl+0x88/0xc0 do_syscall_64+0x38/0x90 entry_SYSCALL_64_after_hwframe+0x63/0xcd -> #0 (btrfs-treloc-02#2){+.+.}-{3:3}: __lock_acquire+0x1122/0x1e10 lock_acquire+0xc2/0x2d0 down_write_nested+0x41/0x80 __btrfs_tree_lock+0x24/0x110 btrfs_lock_root_node+0x31/0x50 btrfs_search_slot+0x1cb/0xb70 replace_path+0x541/0x9f0 merge_reloc_root+0x1d6/0x610 merge_reloc_roots+0xe2/0x260 relocate_block_group+0x2c8/0x560 btrfs_relocate_block_group+0x23e/0x400 btrfs_relocate_chunk+0x4c/0x140 btrfs_balance+0x755/0xe40 btrfs_ioctl+0x1ea2/0x2c90 __x64_sys_ioctl+0x88/0xc0 do_syscall_64+0x38/0x90 entry_SYSCALL_64_after_hwframe+0x63/0xcd other info that might help us debug this: Chain exists of: btrfs-treloc-02#2 --> btrfs-tree-01 --> btrfs-tree-01/1 Possible unsafe locking scenario: CPU0 CPU1 ---- ---- lock(btrfs-tree-01/1); lock(btrfs-tree-01); lock(btrfs-tree-01/1); lock(btrfs-treloc-02#2); *** DEADLOCK *** 7 locks held by btrfs/752500: #0: ffff97e292fdf460 (sb_writers#12){.+.+}-{0:0}, at: btrfs_ioctl+0x208/0x2c90 #1: ffff97e284c02050 (&fs_info->reclaim_bgs_lock){+.+.}-{3:3}, at: btrfs_balance+0x55f/0xe40 #2: ffff97e284c00878 (&fs_info->cleaner_mutex){+.+.}-{3:3}, at: btrfs_relocate_block_group+0x236/0x400 #3: ffff97e292fdf650 (sb_internal#2){.+.+}-{0:0}, at: merge_reloc_root+0xef/0x610 #4: ffff97e284c02378 (btrfs_trans_num_writers){++++}-{0:0}, at: join_transaction+0x1a8/0x5a0 #5: ffff97e284c023a0 (btrfs_trans_num_extwriters){++++}-{0:0}, at: join_transaction+0x1a8/0x5a0 #6: ffff97e1875a9278 (btrfs-tree-01/1){+.+.}-{3:3}, at: __btrfs_tree_lock+0x24/0x110 stack backtrace: CPU: 1 PID: 752500 Comm: btrfs Not tainted 5.19.0-rc8+ #775 Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.13.0-2.fc32 04/01/2014 Call Trace: dump_stack_lvl+0x56/0x73 check_noncircular+0xd6/0x100 ? lock_is_held_type+0xe2/0x140 __lock_acquire+0x1122/0x1e10 lock_acquire+0xc2/0x2d0 ? __btrfs_tree_lock+0x24/0x110 down_write_nested+0x41/0x80 ? __btrfs_tree_lock+0x24/0x110 __btrfs_tree_lock+0x24/0x110 btrfs_lock_root_node+0x31/0x50 btrfs_search_slot+0x1cb/0xb70 ? lock_release+0x137/0x2d0 ? _raw_spin_unlock+0x29/0x50 ? release_extent_buffer+0x128/0x180 replace_path+0x541/0x9f0 merge_reloc_root+0x1d6/0x610 merge_reloc_roots+0xe2/0x260 relocate_block_group+0x2c8/0x560 btrfs_relocate_block_group+0x23e/0x400 btrfs_relocate_chunk+0x4c/0x140 btrfs_balance+0x755/0xe40 btrfs_ioctl+0x1ea2/0x2c90 ? lock_is_held_type+0xe2/0x140 ? lock_is_held_type+0xe2/0x140 ? __x64_sys_ioctl+0x88/0xc0 __x64_sys_ioctl+0x88/0xc0 do_syscall_64+0x38/0x90 entry_SYSCALL_64_after_hwframe+0x63/0xcd This isn't necessarily new, it's just tricky to hit in practice. There are two competing things going on here. With relocation we create a snapshot of every fs tree with a reloc tree. Any extent buffers that get initialized here are initialized with the reloc root lockdep key. However since it is a snapshot, any blocks that are currently in cache that originally belonged to the fs tree will have the normal tree lockdep key set. This creates the lock dependency of reloc tree -> normal tree for the extent buffer locking during the first phase of the relocation as we walk down the reloc root to relocate blocks. However this is problematic because the final phase of the relocation is merging the reloc root into the original fs root. This involves searching down to any keys that exist in the original fs root and then swapping the relocated block and the original fs root block. We have to search down to the fs root first, and then go search the reloc root for the block we need to replace. This creates the dependency of normal tree -> reloc tree which is why lockdep complains. Additionally even if we were to fix this particular mismatch with a different nesting for the merge case, we're still slotting in a block that has a owner of the reloc root objectid into a normal tree, so that block will have its lockdep key set to the tree reloc root, and create a lockdep splat later on when we wander into that block from the fs root. Unfortunately the only solution here is to make sure we do not set the lockdep key to the reloc tree lockdep key normally, and then reset any blocks we wander into from the reloc root when we're doing the merged. This solves the problem of having mixed tree reloc keys intermixed with normal tree keys, and then allows us to make sure in the merge case we maintain the lock order of normal tree -> reloc tree We handle this by setting a bit on the reloc root when we do the search for the block we want to relocate, and any block we search into or COW at that point gets set to the reloc tree key. This works correctly because we only ever COW down to the parent node, so we aren't resetting the key for the block we're linking into the fs root. With this patch we no longer have the lockdep splat in btrfs/187. Signed-off-by: Josef Bacik <[email protected]> Reviewed-by: David Sterba <[email protected]> Signed-off-by: David Sterba <[email protected]>
1 parent 0a27a04 commit b40130b

File tree

7 files changed

+50
-2
lines changed

7 files changed

+50
-2
lines changed

fs/btrfs/ctree.c

+3
Original file line numberDiff line numberDiff line change
@@ -2075,6 +2075,9 @@ int btrfs_search_slot(struct btrfs_trans_handle *trans, struct btrfs_root *root,
20752075

20762076
if (!p->skip_locking) {
20772077
level = btrfs_header_level(b);
2078+
2079+
btrfs_maybe_reset_lockdep_class(root, b);
2080+
20782081
if (level <= write_lock_level) {
20792082
btrfs_tree_lock(b);
20802083
p->locks[level] = BTRFS_WRITE_LOCK;

fs/btrfs/ctree.h

+2
Original file line numberDiff line numberDiff line change
@@ -1173,6 +1173,8 @@ enum {
11731173
BTRFS_ROOT_ORPHAN_CLEANUP,
11741174
/* This root has a drop operation that was started previously. */
11751175
BTRFS_ROOT_UNFINISHED_DROP,
1176+
/* This reloc root needs to have its buffers lockdep class reset. */
1177+
BTRFS_ROOT_RESET_LOCKDEP_CLASS,
11761178
};
11771179

11781180
static inline void btrfs_wake_unfinished_drop(struct btrfs_fs_info *fs_info)

fs/btrfs/extent-tree.c

+17-1
Original file line numberDiff line numberDiff line change
@@ -4867,6 +4867,7 @@ btrfs_init_new_buffer(struct btrfs_trans_handle *trans, struct btrfs_root *root,
48674867
{
48684868
struct btrfs_fs_info *fs_info = root->fs_info;
48694869
struct extent_buffer *buf;
4870+
u64 lockdep_owner = owner;
48704871

48714872
buf = btrfs_find_create_tree_block(fs_info, bytenr, owner, level);
48724873
if (IS_ERR(buf))
@@ -4885,12 +4886,27 @@ btrfs_init_new_buffer(struct btrfs_trans_handle *trans, struct btrfs_root *root,
48854886
return ERR_PTR(-EUCLEAN);
48864887
}
48874888

4889+
/*
4890+
* The reloc trees are just snapshots, so we need them to appear to be
4891+
* just like any other fs tree WRT lockdep.
4892+
*
4893+
* The exception however is in replace_path() in relocation, where we
4894+
* hold the lock on the original fs root and then search for the reloc
4895+
* root. At that point we need to make sure any reloc root buffers are
4896+
* set to the BTRFS_TREE_RELOC_OBJECTID lockdep class in order to make
4897+
* lockdep happy.
4898+
*/
4899+
if (lockdep_owner == BTRFS_TREE_RELOC_OBJECTID &&
4900+
!test_bit(BTRFS_ROOT_RESET_LOCKDEP_CLASS, &root->state))
4901+
lockdep_owner = BTRFS_FS_TREE_OBJECTID;
4902+
48884903
/*
48894904
* This needs to stay, because we could allocate a freed block from an
48904905
* old tree into a new tree, so we need to make sure this new block is
48914906
* set to the appropriate level and owner.
48924907
*/
4893-
btrfs_set_buffer_lockdep_class(owner, buf, level);
4908+
btrfs_set_buffer_lockdep_class(lockdep_owner, buf, level);
4909+
48944910
__btrfs_tree_lock(buf, nest);
48954911
btrfs_clean_tree_block(buf);
48964912
clear_bit(EXTENT_BUFFER_STALE, &buf->bflags);

fs/btrfs/extent_io.c

+10-1
Original file line numberDiff line numberDiff line change
@@ -6140,6 +6140,7 @@ struct extent_buffer *alloc_extent_buffer(struct btrfs_fs_info *fs_info,
61406140
struct extent_buffer *exists = NULL;
61416141
struct page *p;
61426142
struct address_space *mapping = fs_info->btree_inode->i_mapping;
6143+
u64 lockdep_owner = owner_root;
61436144
int uptodate = 1;
61446145
int ret;
61456146

@@ -6164,7 +6165,15 @@ struct extent_buffer *alloc_extent_buffer(struct btrfs_fs_info *fs_info,
61646165
eb = __alloc_extent_buffer(fs_info, start, len);
61656166
if (!eb)
61666167
return ERR_PTR(-ENOMEM);
6167-
btrfs_set_buffer_lockdep_class(owner_root, eb, level);
6168+
6169+
/*
6170+
* The reloc trees are just snapshots, so we need them to appear to be
6171+
* just like any other fs tree WRT lockdep.
6172+
*/
6173+
if (lockdep_owner == BTRFS_TREE_RELOC_OBJECTID)
6174+
lockdep_owner = BTRFS_FS_TREE_OBJECTID;
6175+
6176+
btrfs_set_buffer_lockdep_class(lockdep_owner, eb, level);
61686177

61696178
num_pages = num_extent_pages(eb);
61706179
for (i = 0; i < num_pages; i++, index++) {

fs/btrfs/locking.c

+11
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,13 @@ void btrfs_set_buffer_lockdep_class(u64 objectid, struct extent_buffer *eb, int
9191
lockdep_set_class_and_name(&eb->lock, &ks->keys[level], ks->names[level]);
9292
}
9393

94+
void btrfs_maybe_reset_lockdep_class(struct btrfs_root *root, struct extent_buffer *eb)
95+
{
96+
if (test_bit(BTRFS_ROOT_RESET_LOCKDEP_CLASS, &root->state))
97+
btrfs_set_buffer_lockdep_class(root->root_key.objectid,
98+
eb, btrfs_header_level(eb));
99+
}
100+
94101
#endif
95102

96103
/*
@@ -244,6 +251,8 @@ struct extent_buffer *btrfs_lock_root_node(struct btrfs_root *root)
244251

245252
while (1) {
246253
eb = btrfs_root_node(root);
254+
255+
btrfs_maybe_reset_lockdep_class(root, eb);
247256
btrfs_tree_lock(eb);
248257
if (eb == root->node)
249258
break;
@@ -265,6 +274,8 @@ struct extent_buffer *btrfs_read_lock_root_node(struct btrfs_root *root)
265274

266275
while (1) {
267276
eb = btrfs_root_node(root);
277+
278+
btrfs_maybe_reset_lockdep_class(root, eb);
268279
btrfs_tree_read_lock(eb);
269280
if (eb == root->node)
270281
break;

fs/btrfs/locking.h

+5
Original file line numberDiff line numberDiff line change
@@ -133,11 +133,16 @@ void btrfs_drew_read_unlock(struct btrfs_drew_lock *lock);
133133

134134
#ifdef CONFIG_DEBUG_LOCK_ALLOC
135135
void btrfs_set_buffer_lockdep_class(u64 objectid, struct extent_buffer *eb, int level);
136+
void btrfs_maybe_reset_lockdep_class(struct btrfs_root *root, struct extent_buffer *eb);
136137
#else
137138
static inline void btrfs_set_buffer_lockdep_class(u64 objectid,
138139
struct extent_buffer *eb, int level)
139140
{
140141
}
142+
static inline void btrfs_maybe_reset_lockdep_class(struct btrfs_root *root,
143+
struct extent_buffer *eb)
144+
{
145+
}
141146
#endif
142147

143148
#endif

fs/btrfs/relocation.c

+2
Original file line numberDiff line numberDiff line change
@@ -1326,7 +1326,9 @@ int replace_path(struct btrfs_trans_handle *trans, struct reloc_control *rc,
13261326
btrfs_release_path(path);
13271327

13281328
path->lowest_level = level;
1329+
set_bit(BTRFS_ROOT_RESET_LOCKDEP_CLASS, &src->state);
13291330
ret = btrfs_search_slot(trans, src, &key, path, 0, 1);
1331+
clear_bit(BTRFS_ROOT_RESET_LOCKDEP_CLASS, &src->state);
13301332
path->lowest_level = 0;
13311333
if (ret) {
13321334
if (ret > 0)

0 commit comments

Comments
 (0)