Re: [PATCH v2] Btrfs: fix assertion failure when freeing block groups at close_ctree()
On Thu, Feb 02, 2017 at 05:56:33PM +, fdman...@kernel.org wrote:
> From: Filipe Manana
>
> At close_ctree() we free the block groups and then only after we wait for
> any running worker kthreads to finish and shutdown the workqueues. This
> behaviour is racy and it triggers an assertion failure when freeing block
> groups because while we are doing it we can have for example a block group
> caching kthread running, and in that case the block group's reference
> count can still be greater than 1 by the time we assert its reference count
> is 1, leading to an assertion failure:
>
> [19041.198004] assertion failed: atomic_read(&block_group->count) == 1, file:
> fs/btrfs/extent-tree.c, line: 9799
> [19041.200584] [ cut here ]
> [19041.201692] kernel BUG at fs/btrfs/ctree.h:3418!
> [19041.202830] invalid opcode: [#1] PREEMPT SMP
> [19041.203929] Modules linked in: btrfs xor raid6_pq dm_flakey dm_mod
> crc32c_generic ppdev sg psmouse acpi_cpufreq pcspkr parport_pc evdev tpm_tis
> parport tpm_tis_core i2c_piix4 i2c_core tpm serio_raw processor button loop
> autofs4 ext4 crc16 jbd2 mbcache sr_mod cdrom sd_mod ata_generic virtio_scsi
> ata_piix virtio_pci libata virtio_ring virtio e1000 scsi_mod floppy [last
> unloaded: btrfs]
> [19041.208082] CPU: 6 PID: 29051 Comm: umount Not tainted
> 4.9.0-rc7-btrfs-next-36+ #1
> [19041.208082] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
> rel-1.9.1-0-gb3ef39f-prebuilt.qemu-project.org 04/01/2014
> [19041.208082] task: 88015f028980 task.stack: c9000ad34000
> [19041.208082] RIP: 0010:[] []
> assfail.constprop.41+0x1c/0x1e [btrfs]
> [19041.208082] RSP: 0018:c9000ad37d60 EFLAGS: 00010286
> [19041.208082] RAX: 0061 RBX: 88015ecb4000 RCX:
> 0001
> [19041.208082] RDX: 88023f392fb8 RSI: 817ef7ba RDI:
>
> [19041.208082] RBP: c9000ad37d60 R08: 0001 R09:
>
> [19041.208082] R10: c9000ad37cb0 R11: 82f2b66d R12:
> 88023431d170
> [19041.208082] R13: 88015ecb40c0 R14: 88023431d000 R15:
> 88015ecb4100
> [19041.208082] FS: 7f44f3d42840() GS:88023f38()
> knlGS:
> [19041.208082] CS: 0010 DS: ES: CR0: 80050033
> [19041.208082] CR2: 7f65d623b000 CR3: 0002166f2000 CR4:
> 06e0
> [19041.208082] Stack:
> [19041.208082] c9000ad37d98 a035989f 88015ecb4000
> 88015ecb5630
> [19041.208082] 88014f6be000 7ffcf0ba6a10
> c9000ad37df8
> [19041.208082] a0368cd4 88014e9658e0 c9000ad37e08
> 811a634d
> [19041.208082] Call Trace:
> [19041.208082] [] btrfs_free_block_groups+0x17f/0x392
> [btrfs]
> [19041.208082] [] close_ctree+0x1c5/0x2e1 [btrfs]
> [19041.208082] [] ? evict_inodes+0x132/0x141
> [19041.208082] [] btrfs_put_super+0x15/0x17 [btrfs]
> [19041.208082] [] generic_shutdown_super+0x6a/0xeb
> [19041.208082] [] kill_anon_super+0x12/0x1c
> [19041.208082] [] btrfs_kill_super+0x16/0x21 [btrfs]
> [19041.208082] [] deactivate_locked_super+0x3b/0x68
> [19041.208082] [] deactivate_super+0x36/0x39
> [19041.208082] [] cleanup_mnt+0x58/0x76
> [19041.208082] [] __cleanup_mnt+0x12/0x14
> [19041.208082] [] task_work_run+0x6f/0x95
> [19041.208082] [] prepare_exit_to_usermode+0xa3/0xc1
> [19041.208082] [] syscall_return_slowpath+0x16e/0x1d2
> [19041.208082] [] entry_SYSCALL_64_fastpath+0xab/0xad
> [19041.208082] Code: c7 ae a0 3e a0 48 89 e5 e8 4e 74 d4 e0 0f 0b 55 89 f1 48
> c7 c2 0b a4 3e a0 48 89 fe 48 c7 c7 a4 a6 3e a0 48 89 e5 e8 30 74 d4 e0 <0f>
> 0b 55 31 d2 48 89 e5 e8 d5 b9 f7 ff 5d c3 48 63 f6 55 31 c9
> [19041.208082] RIP [] assfail.constprop.41+0x1c/0x1e
> [btrfs]
> [19041.208082] RSP
> [19041.279264] ---[ end trace 23330586f16f064d ]---
>
> This started happening as of kernel 4.8, since commit f3bca8028bd9
> ("Btrfs: add ASSERT for block group's memory leak") introduced these
> assertions.
>
> So fix this by freeing the block groups only after waiting for all
> worker kthreads to complete and shutdown the workqueues.
Reviewed-by: Liu Bo
Thanks,
-liubo
>
> Signed-off-by: Filipe Manana
> ---
>
> v2: Make error path of open_ctree() also stop all workers before freeing
> the block groups. Assert that when freeing block groups, no block
> group can be in the caching started state.
>
> fs/btrfs/disk-io.c | 6 +++---
> fs/btrfs/extent-tree.c | 9 ++---
> 2 files changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index 066d9b9..bf54d7d 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -3263,7 +3263,6 @@ int open_ctree(struct super_block *sb,
>
> fail_block_groups:
> btrfs_put_block_group_cache(fs_info);
> - btrfs_free_block_groups(fs_info);
>
> fail_tree_roots:
> free_root_pointers(fs_info, 1);
> @@ -3271,6 +3270,7 @@ int open_ctree(struct
[PATCH v2] Btrfs: fix assertion failure when freeing block groups at close_ctree()
From: Filipe Manana
At close_ctree() we free the block groups and then only after we wait for
any running worker kthreads to finish and shutdown the workqueues. This
behaviour is racy and it triggers an assertion failure when freeing block
groups because while we are doing it we can have for example a block group
caching kthread running, and in that case the block group's reference
count can still be greater than 1 by the time we assert its reference count
is 1, leading to an assertion failure:
[19041.198004] assertion failed: atomic_read(&block_group->count) == 1, file:
fs/btrfs/extent-tree.c, line: 9799
[19041.200584] [ cut here ]
[19041.201692] kernel BUG at fs/btrfs/ctree.h:3418!
[19041.202830] invalid opcode: [#1] PREEMPT SMP
[19041.203929] Modules linked in: btrfs xor raid6_pq dm_flakey dm_mod
crc32c_generic ppdev sg psmouse acpi_cpufreq pcspkr parport_pc evdev tpm_tis
parport tpm_tis_core i2c_piix4 i2c_core tpm serio_raw processor button loop
autofs4 ext4 crc16 jbd2 mbcache sr_mod cdrom sd_mod ata_generic virtio_scsi
ata_piix virtio_pci libata virtio_ring virtio e1000 scsi_mod floppy [last
unloaded: btrfs]
[19041.208082] CPU: 6 PID: 29051 Comm: umount Not tainted
4.9.0-rc7-btrfs-next-36+ #1
[19041.208082] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
rel-1.9.1-0-gb3ef39f-prebuilt.qemu-project.org 04/01/2014
[19041.208082] task: 88015f028980 task.stack: c9000ad34000
[19041.208082] RIP: 0010:[] []
assfail.constprop.41+0x1c/0x1e [btrfs]
[19041.208082] RSP: 0018:c9000ad37d60 EFLAGS: 00010286
[19041.208082] RAX: 0061 RBX: 88015ecb4000 RCX: 0001
[19041.208082] RDX: 88023f392fb8 RSI: 817ef7ba RDI:
[19041.208082] RBP: c9000ad37d60 R08: 0001 R09:
[19041.208082] R10: c9000ad37cb0 R11: 82f2b66d R12: 88023431d170
[19041.208082] R13: 88015ecb40c0 R14: 88023431d000 R15: 88015ecb4100
[19041.208082] FS: 7f44f3d42840() GS:88023f38()
knlGS:
[19041.208082] CS: 0010 DS: ES: CR0: 80050033
[19041.208082] CR2: 7f65d623b000 CR3: 0002166f2000 CR4: 06e0
[19041.208082] Stack:
[19041.208082] c9000ad37d98 a035989f 88015ecb4000
88015ecb5630
[19041.208082] 88014f6be000 7ffcf0ba6a10
c9000ad37df8
[19041.208082] a0368cd4 88014e9658e0 c9000ad37e08
811a634d
[19041.208082] Call Trace:
[19041.208082] [] btrfs_free_block_groups+0x17f/0x392 [btrfs]
[19041.208082] [] close_ctree+0x1c5/0x2e1 [btrfs]
[19041.208082] [] ? evict_inodes+0x132/0x141
[19041.208082] [] btrfs_put_super+0x15/0x17 [btrfs]
[19041.208082] [] generic_shutdown_super+0x6a/0xeb
[19041.208082] [] kill_anon_super+0x12/0x1c
[19041.208082] [] btrfs_kill_super+0x16/0x21 [btrfs]
[19041.208082] [] deactivate_locked_super+0x3b/0x68
[19041.208082] [] deactivate_super+0x36/0x39
[19041.208082] [] cleanup_mnt+0x58/0x76
[19041.208082] [] __cleanup_mnt+0x12/0x14
[19041.208082] [] task_work_run+0x6f/0x95
[19041.208082] [] prepare_exit_to_usermode+0xa3/0xc1
[19041.208082] [] syscall_return_slowpath+0x16e/0x1d2
[19041.208082] [] entry_SYSCALL_64_fastpath+0xab/0xad
[19041.208082] Code: c7 ae a0 3e a0 48 89 e5 e8 4e 74 d4 e0 0f 0b 55 89 f1 48
c7 c2 0b a4 3e a0 48 89 fe 48 c7 c7 a4 a6 3e a0 48 89 e5 e8 30 74 d4 e0 <0f> 0b
55 31 d2 48 89 e5 e8 d5 b9 f7 ff 5d c3 48 63 f6 55 31 c9
[19041.208082] RIP [] assfail.constprop.41+0x1c/0x1e [btrfs]
[19041.208082] RSP
[19041.279264] ---[ end trace 23330586f16f064d ]---
This started happening as of kernel 4.8, since commit f3bca8028bd9
("Btrfs: add ASSERT for block group's memory leak") introduced these
assertions.
So fix this by freeing the block groups only after waiting for all
worker kthreads to complete and shutdown the workqueues.
Signed-off-by: Filipe Manana
---
v2: Make error path of open_ctree() also stop all workers before freeing
the block groups. Assert that when freeing block groups, no block
group can be in the caching started state.
fs/btrfs/disk-io.c | 6 +++---
fs/btrfs/extent-tree.c | 9 ++---
2 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 066d9b9..bf54d7d 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -3263,7 +3263,6 @@ int open_ctree(struct super_block *sb,
fail_block_groups:
btrfs_put_block_group_cache(fs_info);
- btrfs_free_block_groups(fs_info);
fail_tree_roots:
free_root_pointers(fs_info, 1);
@@ -3271,6 +3270,7 @@ int open_ctree(struct super_block *sb,
fail_sb_buffer:
btrfs_stop_all_workers(fs_info);
+ btrfs_free_block_groups(fs_info);
fail_alloc:
fail_iput:
btrfs_mapping_tree_free(&fs_info->mapping_tree);
@@ -3985,8 +3985,6 @@ void close_ctree(struct btrfs_fs_info *fs_info)
btrfs_put_block_group_cache(fs_info);
-
