On Fri, Nov 30, 2018 at 4:53 PM Josef Bacik <jo...@toxicpanda.com> wrote:
>
> From: Josef Bacik <jba...@fb.com>
>
> When debugging some weird extent reference bug I suspected that we were
> changing a snapshot while we were deleting it, which could explain my
> bug. This was indeed what was happening, and this patch helped me
> verify my theory. It is never correct to modify the snapshot once it's
> being deleted, so mark the root when we are deleting it and make sure we
> complain about it when it happens.
>
> Signed-off-by: Josef Bacik <jo...@toxicpanda.com>
> ---
> fs/btrfs/ctree.c | 3 +++
> fs/btrfs/ctree.h | 1 +
> fs/btrfs/extent-tree.c | 9 +++++++++
> 3 files changed, 13 insertions(+)
>
> diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
> index 5912a97b07a6..5f82f86085e8 100644
> --- a/fs/btrfs/ctree.c
> +++ b/fs/btrfs/ctree.c
> @@ -1440,6 +1440,9 @@ noinline int btrfs_cow_block(struct btrfs_trans_handle
> *trans,
> u64 search_start;
> int ret;
>
> + if (test_bit(BTRFS_ROOT_DELETING, &root->state))
> + WARN(1, KERN_CRIT "cow'ing blocks on a fs root thats being
> dropped\n");
Please use btrfs_warn(), it makes sure we use a consistent message
style, identifies the fs, etc.
Also, "thats" should be "that is" or "that's".
With that fixed,
Reviewed-by: Filipe Manana <fdman...@suse.com>
> +
> if (trans->transaction != fs_info->running_transaction)
> WARN(1, KERN_CRIT "trans %llu running %llu\n",
> trans->transid,
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index facde70c15ed..5a3a94ccb65c 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -1199,6 +1199,7 @@ enum {
> BTRFS_ROOT_FORCE_COW,
> BTRFS_ROOT_MULTI_LOG_TASKS,
> BTRFS_ROOT_DIRTY,
> + BTRFS_ROOT_DELETING,
> };
>
> /*
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index 581c2a0b2945..dcb699dd57f3 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -9333,6 +9333,15 @@ int btrfs_drop_snapshot(struct btrfs_root *root,
> if (block_rsv)
> trans->block_rsv = block_rsv;
>
> + /*
> + * This will help us catch people modifying the fs tree while we're
> + * dropping it. It is unsafe to mess with the fs tree while it's
> being
> + * dropped as we unlock the root node and parent nodes as we walk down
> + * the tree, assuming nothing will change. If something does change
> + * then we'll have stale information and drop references to blocks
> we've
> + * already dropped.
> + */
> + set_bit(BTRFS_ROOT_DELETING, &root->state);
> if (btrfs_disk_key_objectid(&root_item->drop_progress) == 0) {
> level = btrfs_header_level(root->node);
> path->nodes[level] = btrfs_lock_root_node(root);
> --
> 2.14.3
>
--
Filipe David Manana,
“Whether you think you can, or you think you can't — you're right.”
