On Fri, 2020-07-03 at 15:05 +0800, Qu Wenruo wrote:
> [BUG]
> The following small test script can trigger ASSERT() at unmount time:
>
> mkfs.btrfs -f $dev
> mount $dev $mnt
> mount -o remount,discard=async $mnt
> umount $mnt
>
> The call trace:
> assertion failed: atomic_read(&block_group->count) == 1, in
> fs/btrfs/block-group.c:3431
> ------------[ cut here ]------------
> kernel BUG at fs/btrfs/ctree.h:3204!
> invalid opcode: 0000 [#1] PREEMPT SMP NOPTI
> CPU: 4 PID: 10389 Comm: umount Tainted: G O 5.8.0-
> rc3-custom+ #68
> Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 0.0.0
> 02/06/2015
> Call Trace:
> btrfs_free_block_groups.cold+0x22/0x55 [btrfs]
> close_ctree+0x2cb/0x323 [btrfs]
> btrfs_put_super+0x15/0x17 [btrfs]
> generic_shutdown_super+0x72/0x110
> kill_anon_super+0x18/0x30
> btrfs_kill_super+0x17/0x30 [btrfs]
> deactivate_locked_super+0x3b/0xa0
> deactivate_super+0x40/0x50
> cleanup_mnt+0x135/0x190
> __cleanup_mnt+0x12/0x20
> task_work_run+0x64/0xb0
> __prepare_exit_to_usermode+0x1bc/0x1c0
> __syscall_return_slowpath+0x47/0x230
> do_syscall_64+0x64/0xb0
> entry_SYSCALL_64_after_hwframe+0x44/0xa9
>
> The code:
> ASSERT(atomic_read(&block_group->count) == 1);
> btrfs_put_block_group(block_group);
>
> [CAUSE]
> Obviously it's some btrfs_get_block_group() call doesn't get its put
> call.
>
> The offending btrfs_get_block_group() happens here:
>
> void btrfs_mark_bg_unused(struct btrfs_block_group *bg)
> {
> if (list_empty(&bg->bg_list)) {
> btrfs_get_block_group(bg);
> list_add_tail(&bg->bg_list, &fs_info->unused_bgs);
> }
> }
>
> So every call sites removing the block group from unused_bgs list
> should
> reduce the ref count of that block group.
>
> However for async discard, it didn't follow the call convention:
>
> void btrfs_discard_punt_unused_bgs_list(struct btrfs_fs_info
> *fs_info)
> {
> list_for_each_entry_safe(block_group, next, &fs_info-
> >unused_bgs,
> bg_list) {
> list_del_init(&block_group->bg_list);
> btrfs_discard_queue_work(&fs_info->discard_ctl,
> block_group);
> }
> }
>
> And in btrfs_discard_queue_work(), it doesn't call
> btrfs_put_block_group() either.
>
> [FIX]
> Fix the problem by reducing the reference count when we grab the
> block
> group from unused_bgs list.
xfstests is happy about the change.
Tested-by: Marcos Paulo de Souza <mpdesouza@xxxxxxxx>
>
> Reported-by: Marcos Paulo de Souza <marcos@xxxxxxxxxxxxx>
> Fixes: 6e80d4f8c422 ("btrfs: handle empty block_group removal for
> async discard")
> Signed-off-by: Qu Wenruo <wqu@xxxxxxxx>
> ---
> fs/btrfs/discard.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/fs/btrfs/discard.c b/fs/btrfs/discard.c
> index 5615320fa659..741c7e19c32f 100644
> --- a/fs/btrfs/discard.c
> +++ b/fs/btrfs/discard.c
> @@ -619,6 +619,7 @@ void btrfs_discard_punt_unused_bgs_list(struct
> btrfs_fs_info *fs_info)
> list_for_each_entry_safe(block_group, next, &fs_info-
> >unused_bgs,
> bg_list) {
> list_del_init(&block_group->bg_list);
> + btrfs_put_block_group(block_group);
> btrfs_discard_queue_work(&fs_info->discard_ctl,
> block_group);
> }
> spin_unlock(&fs_info->unused_bgs_lock);
