On 23.05.2018 10:29, Qu Wenruo wrote:
> When doing qgroup rescan using the following script (modified from
> btrfs/017 test case), we can sometimes hit qgroup corruption.
>
> ------
> umount $dev &> /dev/null
> umount $mnt &> /dev/null
>
> mkfs.btrfs -f -n 64k $dev
> mount $dev $mnt
>
> extent_size=8192
>
> xfs_io -f -d -c "pwrite 0 $extent_size" $mnt/foo > /dev/null
> btrfs subvolume snapshot $mnt $mnt/snap
>
> xfs_io -f -c "reflink $mnt/foo" $mnt/foo-reflink > /dev/null
> xfs_io -f -c "reflink $mnt/foo" $mnt/snap/foo-reflink > /dev/null
> xfs_io -f -c "reflink $mnt/foo" $mnt/snap/foo-reflink2 > /dev/unll
> btrfs quota enable $mnt
>
> # -W is the new option to only wait rescan while not starting new one
> btrfs quota rescan -W $mnt
> btrfs qgroup show -prce $mnt
> umount $mnt
>
> # Need to patch btrfs-progs to report qgroup mismatch as error
> btrfs check $dev || _fail
> ------
>
> For fast machine, we can hit some corruption which missed accounting
> tree blocks:
> ------
> qgroupid rfer excl max_rfer max_excl parent child
> -------- ---- ---- -------- -------- ------ -----
> 0/5 8.00KiB 0.00B none none --- ---
> 0/257 8.00KiB 0.00B none none --- ---
> ------
>
> This is due to the fact that we're always searching commit root for
> btrfs_find_all_roots() at qgroup_rescan_leaf(), but the leaf we get is
> from current transaction, not commit root.
>
> And if our tree blocks get modified in current transaction, we won't
> find any owner in commit root, thus causing the corruption.
>
> Fix it by searching commit root for extent tree for
> qgroup_rescan_leaf().
>
> Reported-by: Nikolay Borisov <nbori...@suse.com>
> Signed-off-by: Qu Wenruo <w...@suse.com>
Reviewed-by: Nikolay Borisov <nbori...@suse.com>
> ---
> changelog:
> v2:
> Remove unused tree_mod_seq_elem for the 1st patch.
> v3:
> Also skip locking to avoid lock assert() with Liu Bo's
> btrfs_search_slot() patchset.
> ---
> fs/btrfs/qgroup.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
> index 9fb758d5077a..a88330088d03 100644
> --- a/fs/btrfs/qgroup.c
> +++ b/fs/btrfs/qgroup.c
> @@ -2590,7 +2590,6 @@ qgroup_rescan_leaf(struct btrfs_fs_info *fs_info,
> struct btrfs_path *path,
> struct btrfs_key found;
> struct extent_buffer *scratch_leaf = NULL;
> struct ulist *roots = NULL;
> - struct seq_list tree_mod_seq_elem = SEQ_LIST_INIT(tree_mod_seq_elem);
> u64 num_bytes;
> int slot;
> int ret;
> @@ -2625,7 +2624,6 @@ qgroup_rescan_leaf(struct btrfs_fs_info *fs_info,
> struct btrfs_path *path,
> btrfs_header_nritems(path->nodes[0]) - 1);
> fs_info->qgroup_rescan_progress.objectid = found.objectid + 1;
>
> - btrfs_get_tree_mod_seq(fs_info, &tree_mod_seq_elem);
> scratch_leaf = btrfs_clone_extent_buffer(path->nodes[0]);
> if (!scratch_leaf) {
> ret = -ENOMEM;
> @@ -2664,7 +2662,6 @@ qgroup_rescan_leaf(struct btrfs_fs_info *fs_info,
> struct btrfs_path *path,
> btrfs_tree_read_unlock_blocking(scratch_leaf);
> free_extent_buffer(scratch_leaf);
> }
> - btrfs_put_tree_mod_seq(fs_info, &tree_mod_seq_elem);
>
> return ret;
> }
> @@ -2681,6 +2678,12 @@ static void btrfs_qgroup_rescan_worker(struct
> btrfs_work *work)
> path = btrfs_alloc_path();
> if (!path)
> goto out;
> + /*
> + * Rescan should only search for commit root, and any later difference
> + * should be recorded by qgroup
> + */
> + path->search_commit_root = 1;
> + path->skip_locking = 1;
>
> err = 0;
> while (!err && !btrfs_fs_closing(fs_info)) {
>
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
