Commit 67c5e7d4 authored by Filipe Manana's avatar Filipe Manana Committed by Chris Mason

Btrfs: fix race between balance and unused block group deletion

We have a race between deleting an unused block group and balancing the
same block group that leads to an assertion failure/BUG(), producing the
following trace:

[181631.208236] BTRFS: assertion failed: 0, file: fs/btrfs/volumes.c, line: 2622
[181631.220591] ------------[ cut here ]------------
[181631.222959] kernel BUG at fs/btrfs/ctree.h:4062!
[181631.223932] invalid opcode: 0000 [#1] PREEMPT SMP DEBUG_PAGEALLOC
[181631.224566] Modules linked in: btrfs dm_flakey dm_mod crc32c_generic xor raid6_pq nfsd auth_rpcgss oid_registry nfs_acl nfs lockd grace fscache sunrpc loop fuse acpi_cpufreq parpor$
[181631.224566] CPU: 8 PID: 17451 Comm: btrfs Tainted: G        W       4.1.0-rc5-btrfs-next-10+ #1
[181631.224566] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.8.1-0-g4adadbd-20150316_085822-nilsson.home.kraxel.org 04/01/2014
[181631.224566] task: ffff880127e09590 ti: ffff8800b5824000 task.ti: ffff8800b5824000
[181631.224566] RIP: 0010:[<ffffffffa03f19f6>]  [<ffffffffa03f19f6>] assfail.constprop.50+0x1e/0x20 [btrfs]
[181631.224566] RSP: 0018:ffff8800b5827ae8  EFLAGS: 00010246
[181631.224566] RAX: 0000000000000040 RBX: ffff8800109fc218 RCX: ffffffff81095dce
[181631.224566] RDX: 0000000000005124 RSI: ffffffff81464819 RDI: 00000000ffffffff
[181631.224566] RBP: ffff8800b5827ae8 R08: 0000000000000001 R09: 0000000000000000
[181631.224566] R10: 0000000000000000 R11: 0000000000000000 R12: ffff8800109fc200
[181631.224566] R13: ffff880020095000 R14: ffff8800b1a13f38 R15: ffff880020095000
[181631.224566] FS:  00007f70ca0b0c80(0000) GS:ffff88013ec00000(0000) knlGS:0000000000000000
[181631.224566] CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
[181631.224566] CR2: 00007f2872ab6e68 CR3: 00000000a717c000 CR4: 00000000000006e0
[181631.224566] Stack:
[181631.224566]  ffff8800b5827ba8 ffffffffa03f3916 ffff8800b5827b38 ffffffffa03d080e
[181631.224566]  ffffffffa03d1423 ffff880020095000 ffff88001233c000 0000000000000001
[181631.224566]  ffff880020095000 ffff8800b1a13f38 0000000a69c00000 0000000000000000
[181631.224566] Call Trace:
[181631.224566]  [<ffffffffa03f3916>] btrfs_remove_chunk+0xa4/0x6bb [btrfs]
[181631.224566]  [<ffffffffa03d080e>] ? join_transaction.isra.8+0xb9/0x3ba [btrfs]
[181631.224566]  [<ffffffffa03d1423>] ? wait_current_trans.isra.13+0x22/0xfc [btrfs]
[181631.224566]  [<ffffffffa03f3fbc>] btrfs_relocate_chunk.isra.29+0x8f/0xa7 [btrfs]
[181631.224566]  [<ffffffffa03f54df>] btrfs_balance+0xaa4/0xc52 [btrfs]
[181631.224566]  [<ffffffffa03fd388>] btrfs_ioctl_balance+0x23f/0x2b0 [btrfs]
[181631.224566]  [<ffffffff810872f9>] ? trace_hardirqs_on+0xd/0xf
[181631.224566]  [<ffffffffa04019a3>] btrfs_ioctl+0xfe2/0x2220 [btrfs]
[181631.224566]  [<ffffffff812603ed>] ? __this_cpu_preempt_check+0x13/0x15
[181631.224566]  [<ffffffff81084669>] ? arch_local_irq_save+0x9/0xc
[181631.224566]  [<ffffffff81138def>] ? handle_mm_fault+0x834/0xcd2
[181631.224566]  [<ffffffff81138def>] ? handle_mm_fault+0x834/0xcd2
[181631.224566]  [<ffffffff8103e48c>] ? __do_page_fault+0x211/0x424
[181631.224566]  [<ffffffff811755e6>] do_vfs_ioctl+0x3c6/0x479
(...)

The sequence of steps leading to this are:

           CPU 0                                         CPU 1

  btrfs_balance()
    btrfs_relocate_chunk()

      btrfs_relocate_block_group(bg X)
        btrfs_lookup_block_group(bg X)

                                               cleaner_kthread
                                                  locks fs_info->cleaner_mutex

                                                  btrfs_delete_unused_bgs()
                                                    finds bg X, which became
                                                    unused in the previous
                                                    transaction

                                                    checks bg X ->ro == 0,
                                                    so it proceeds
        sets bg X ->ro to 1
        (btrfs_set_block_group_ro(bg X))

        blocks on fs_info->cleaner_mutex
                                                    btrfs_remove_chunk(bg X)
                                                  unlocks fs_info->cleaner_mutex

        acquires fs_info->cleaner_mutex
        relocate_block_group()
          --> does nothing, no extents found in
              the extent tree from bg X
        unlocks fs_info->cleaner_mutex

      btrfs_relocate_block_group(bg X) returns

    btrfs_remove_chunk(bg X)
       extent map not found
          --> ASSERT(0)

Fix this by using a new mutex to make sure these 2 operations, block
group relocation and removal, are serialized.

This issue is reproducible by running fstests generic/038 (which stresses
chunk allocation and automatic removal of unused block groups) together
with the following balance loop:

    while true; do btrfs balance start -dusage=0 <mountpoint> ; done
Signed-off-by: default avatarFilipe Manana <fdmanana@suse.com>
Signed-off-by: default avatarChris Mason <clm@fb.com>
parent e82afc52
...@@ -1778,6 +1778,7 @@ struct btrfs_fs_info { ...@@ -1778,6 +1778,7 @@ struct btrfs_fs_info {
spinlock_t unused_bgs_lock; spinlock_t unused_bgs_lock;
struct list_head unused_bgs; struct list_head unused_bgs;
struct mutex unused_bg_unpin_mutex; struct mutex unused_bg_unpin_mutex;
struct mutex delete_unused_bgs_mutex;
/* For btrfs to record security options */ /* For btrfs to record security options */
struct security_mnt_opts security_opts; struct security_mnt_opts security_opts;
......
...@@ -1772,7 +1772,6 @@ static int cleaner_kthread(void *arg) ...@@ -1772,7 +1772,6 @@ static int cleaner_kthread(void *arg)
} }
btrfs_run_delayed_iputs(root); btrfs_run_delayed_iputs(root);
btrfs_delete_unused_bgs(root->fs_info);
again = btrfs_clean_one_deleted_snapshot(root); again = btrfs_clean_one_deleted_snapshot(root);
mutex_unlock(&root->fs_info->cleaner_mutex); mutex_unlock(&root->fs_info->cleaner_mutex);
...@@ -1781,6 +1780,16 @@ static int cleaner_kthread(void *arg) ...@@ -1781,6 +1780,16 @@ static int cleaner_kthread(void *arg)
* needn't do anything special here. * needn't do anything special here.
*/ */
btrfs_run_defrag_inodes(root->fs_info); btrfs_run_defrag_inodes(root->fs_info);
/*
* Acquires fs_info->delete_unused_bgs_mutex to avoid racing
* with relocation (btrfs_relocate_chunk) and relocation
* acquires fs_info->cleaner_mutex (btrfs_relocate_block_group)
* after acquiring fs_info->delete_unused_bgs_mutex. So we
* can't hold, nor need to, fs_info->cleaner_mutex when deleting
* unused block groups.
*/
btrfs_delete_unused_bgs(root->fs_info);
sleep: sleep:
if (!try_to_freeze() && !again) { if (!try_to_freeze() && !again) {
set_current_state(TASK_INTERRUPTIBLE); set_current_state(TASK_INTERRUPTIBLE);
...@@ -2492,6 +2501,7 @@ int open_ctree(struct super_block *sb, ...@@ -2492,6 +2501,7 @@ int open_ctree(struct super_block *sb,
spin_lock_init(&fs_info->unused_bgs_lock); spin_lock_init(&fs_info->unused_bgs_lock);
rwlock_init(&fs_info->tree_mod_log_lock); rwlock_init(&fs_info->tree_mod_log_lock);
mutex_init(&fs_info->unused_bg_unpin_mutex); mutex_init(&fs_info->unused_bg_unpin_mutex);
mutex_init(&fs_info->delete_unused_bgs_mutex);
mutex_init(&fs_info->reloc_mutex); mutex_init(&fs_info->reloc_mutex);
mutex_init(&fs_info->delalloc_root_mutex); mutex_init(&fs_info->delalloc_root_mutex);
seqlock_init(&fs_info->profiles_lock); seqlock_init(&fs_info->profiles_lock);
......
...@@ -9889,6 +9889,8 @@ void btrfs_delete_unused_bgs(struct btrfs_fs_info *fs_info) ...@@ -9889,6 +9889,8 @@ void btrfs_delete_unused_bgs(struct btrfs_fs_info *fs_info)
} }
spin_unlock(&fs_info->unused_bgs_lock); spin_unlock(&fs_info->unused_bgs_lock);
mutex_lock(&root->fs_info->delete_unused_bgs_mutex);
/* Don't want to race with allocators so take the groups_sem */ /* Don't want to race with allocators so take the groups_sem */
down_write(&space_info->groups_sem); down_write(&space_info->groups_sem);
spin_lock(&block_group->lock); spin_lock(&block_group->lock);
...@@ -9983,6 +9985,7 @@ void btrfs_delete_unused_bgs(struct btrfs_fs_info *fs_info) ...@@ -9983,6 +9985,7 @@ void btrfs_delete_unused_bgs(struct btrfs_fs_info *fs_info)
end_trans: end_trans:
btrfs_end_transaction(trans, root); btrfs_end_transaction(trans, root);
next: next:
mutex_unlock(&root->fs_info->delete_unused_bgs_mutex);
btrfs_put_block_group(block_group); btrfs_put_block_group(block_group);
spin_lock(&fs_info->unused_bgs_lock); spin_lock(&fs_info->unused_bgs_lock);
} }
......
...@@ -2766,6 +2766,20 @@ static int btrfs_relocate_chunk(struct btrfs_root *root, ...@@ -2766,6 +2766,20 @@ static int btrfs_relocate_chunk(struct btrfs_root *root,
root = root->fs_info->chunk_root; root = root->fs_info->chunk_root;
extent_root = root->fs_info->extent_root; extent_root = root->fs_info->extent_root;
/*
* Prevent races with automatic removal of unused block groups.
* After we relocate and before we remove the chunk with offset
* chunk_offset, automatic removal of the block group can kick in,
* resulting in a failure when calling btrfs_remove_chunk() below.
*
* Make sure to acquire this mutex before doing a tree search (dev
* or chunk trees) to find chunks. Otherwise the cleaner kthread might
* call btrfs_remove_chunk() (through btrfs_delete_unused_bgs()) after
* we release the path used to search the chunk/dev tree and before
* the current task acquires this mutex and calls us.
*/
ASSERT(mutex_is_locked(&root->fs_info->delete_unused_bgs_mutex));
ret = btrfs_can_relocate(extent_root, chunk_offset); ret = btrfs_can_relocate(extent_root, chunk_offset);
if (ret) if (ret)
return -ENOSPC; return -ENOSPC;
...@@ -2814,13 +2828,18 @@ static int btrfs_relocate_sys_chunks(struct btrfs_root *root) ...@@ -2814,13 +2828,18 @@ static int btrfs_relocate_sys_chunks(struct btrfs_root *root)
key.type = BTRFS_CHUNK_ITEM_KEY; key.type = BTRFS_CHUNK_ITEM_KEY;
while (1) { while (1) {
mutex_lock(&root->fs_info->delete_unused_bgs_mutex);
ret = btrfs_search_slot(NULL, chunk_root, &key, path, 0, 0); ret = btrfs_search_slot(NULL, chunk_root, &key, path, 0, 0);
if (ret < 0) if (ret < 0) {
mutex_unlock(&root->fs_info->delete_unused_bgs_mutex);
goto error; goto error;
}
BUG_ON(ret == 0); /* Corruption */ BUG_ON(ret == 0); /* Corruption */
ret = btrfs_previous_item(chunk_root, path, key.objectid, ret = btrfs_previous_item(chunk_root, path, key.objectid,
key.type); key.type);
if (ret)
mutex_unlock(&root->fs_info->delete_unused_bgs_mutex);
if (ret < 0) if (ret < 0)
goto error; goto error;
if (ret > 0) if (ret > 0)
...@@ -2843,6 +2862,7 @@ static int btrfs_relocate_sys_chunks(struct btrfs_root *root) ...@@ -2843,6 +2862,7 @@ static int btrfs_relocate_sys_chunks(struct btrfs_root *root)
else else
BUG_ON(ret); BUG_ON(ret);
} }
mutex_unlock(&root->fs_info->delete_unused_bgs_mutex);
if (found_key.offset == 0) if (found_key.offset == 0)
break; break;
...@@ -3299,9 +3319,12 @@ static int __btrfs_balance(struct btrfs_fs_info *fs_info) ...@@ -3299,9 +3319,12 @@ static int __btrfs_balance(struct btrfs_fs_info *fs_info)
goto error; goto error;
} }
mutex_lock(&fs_info->delete_unused_bgs_mutex);
ret = btrfs_search_slot(NULL, chunk_root, &key, path, 0, 0); ret = btrfs_search_slot(NULL, chunk_root, &key, path, 0, 0);
if (ret < 0) if (ret < 0) {
mutex_unlock(&fs_info->delete_unused_bgs_mutex);
goto error; goto error;
}
/* /*
* this shouldn't happen, it means the last relocate * this shouldn't happen, it means the last relocate
...@@ -3313,6 +3336,7 @@ static int __btrfs_balance(struct btrfs_fs_info *fs_info) ...@@ -3313,6 +3336,7 @@ static int __btrfs_balance(struct btrfs_fs_info *fs_info)
ret = btrfs_previous_item(chunk_root, path, 0, ret = btrfs_previous_item(chunk_root, path, 0,
BTRFS_CHUNK_ITEM_KEY); BTRFS_CHUNK_ITEM_KEY);
if (ret) { if (ret) {
mutex_unlock(&fs_info->delete_unused_bgs_mutex);
ret = 0; ret = 0;
break; break;
} }
...@@ -3321,8 +3345,10 @@ static int __btrfs_balance(struct btrfs_fs_info *fs_info) ...@@ -3321,8 +3345,10 @@ static int __btrfs_balance(struct btrfs_fs_info *fs_info)
slot = path->slots[0]; slot = path->slots[0];
btrfs_item_key_to_cpu(leaf, &found_key, slot); btrfs_item_key_to_cpu(leaf, &found_key, slot);
if (found_key.objectid != key.objectid) if (found_key.objectid != key.objectid) {
mutex_unlock(&fs_info->delete_unused_bgs_mutex);
break; break;
}
chunk = btrfs_item_ptr(leaf, slot, struct btrfs_chunk); chunk = btrfs_item_ptr(leaf, slot, struct btrfs_chunk);
...@@ -3335,10 +3361,13 @@ static int __btrfs_balance(struct btrfs_fs_info *fs_info) ...@@ -3335,10 +3361,13 @@ static int __btrfs_balance(struct btrfs_fs_info *fs_info)
ret = should_balance_chunk(chunk_root, leaf, chunk, ret = should_balance_chunk(chunk_root, leaf, chunk,
found_key.offset); found_key.offset);
btrfs_release_path(path); btrfs_release_path(path);
if (!ret) if (!ret) {
mutex_unlock(&fs_info->delete_unused_bgs_mutex);
goto loop; goto loop;
}
if (counting) { if (counting) {
mutex_unlock(&fs_info->delete_unused_bgs_mutex);
spin_lock(&fs_info->balance_lock); spin_lock(&fs_info->balance_lock);
bctl->stat.expected++; bctl->stat.expected++;
spin_unlock(&fs_info->balance_lock); spin_unlock(&fs_info->balance_lock);
...@@ -3348,6 +3377,7 @@ static int __btrfs_balance(struct btrfs_fs_info *fs_info) ...@@ -3348,6 +3377,7 @@ static int __btrfs_balance(struct btrfs_fs_info *fs_info)
ret = btrfs_relocate_chunk(chunk_root, ret = btrfs_relocate_chunk(chunk_root,
found_key.objectid, found_key.objectid,
found_key.offset); found_key.offset);
mutex_unlock(&fs_info->delete_unused_bgs_mutex);
if (ret && ret != -ENOSPC) if (ret && ret != -ENOSPC)
goto error; goto error;
if (ret == -ENOSPC) { if (ret == -ENOSPC) {
...@@ -4087,11 +4117,16 @@ int btrfs_shrink_device(struct btrfs_device *device, u64 new_size) ...@@ -4087,11 +4117,16 @@ int btrfs_shrink_device(struct btrfs_device *device, u64 new_size)
key.type = BTRFS_DEV_EXTENT_KEY; key.type = BTRFS_DEV_EXTENT_KEY;
do { do {
mutex_lock(&root->fs_info->delete_unused_bgs_mutex);
ret = btrfs_search_slot(NULL, root, &key, path, 0, 0); ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
if (ret < 0) if (ret < 0) {
mutex_unlock(&root->fs_info->delete_unused_bgs_mutex);
goto done; goto done;
}
ret = btrfs_previous_item(root, path, 0, key.type); ret = btrfs_previous_item(root, path, 0, key.type);
if (ret)
mutex_unlock(&root->fs_info->delete_unused_bgs_mutex);
if (ret < 0) if (ret < 0)
goto done; goto done;
if (ret) { if (ret) {
...@@ -4105,6 +4140,7 @@ int btrfs_shrink_device(struct btrfs_device *device, u64 new_size) ...@@ -4105,6 +4140,7 @@ int btrfs_shrink_device(struct btrfs_device *device, u64 new_size)
btrfs_item_key_to_cpu(l, &key, path->slots[0]); btrfs_item_key_to_cpu(l, &key, path->slots[0]);
if (key.objectid != device->devid) { if (key.objectid != device->devid) {
mutex_unlock(&root->fs_info->delete_unused_bgs_mutex);
btrfs_release_path(path); btrfs_release_path(path);
break; break;
} }
...@@ -4113,6 +4149,7 @@ int btrfs_shrink_device(struct btrfs_device *device, u64 new_size) ...@@ -4113,6 +4149,7 @@ int btrfs_shrink_device(struct btrfs_device *device, u64 new_size)
length = btrfs_dev_extent_length(l, dev_extent); length = btrfs_dev_extent_length(l, dev_extent);
if (key.offset + length <= new_size) { if (key.offset + length <= new_size) {
mutex_unlock(&root->fs_info->delete_unused_bgs_mutex);
btrfs_release_path(path); btrfs_release_path(path);
break; break;
} }
...@@ -4122,6 +4159,7 @@ int btrfs_shrink_device(struct btrfs_device *device, u64 new_size) ...@@ -4122,6 +4159,7 @@ int btrfs_shrink_device(struct btrfs_device *device, u64 new_size)
btrfs_release_path(path); btrfs_release_path(path);
ret = btrfs_relocate_chunk(root, chunk_objectid, chunk_offset); ret = btrfs_relocate_chunk(root, chunk_objectid, chunk_offset);
mutex_unlock(&root->fs_info->delete_unused_bgs_mutex);
if (ret && ret != -ENOSPC) if (ret && ret != -ENOSPC)
goto done; goto done;
if (ret == -ENOSPC) if (ret == -ENOSPC)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment