Re: [PATCH 1/2] btrfs: catch cow on deleting snapshots

[Qu Wenruo <quwenruo.btrfs@gmx.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2651 bytes --]



On 2018/12/1 上午12:52, Josef Bacik wrote:
> From: Josef Bacik <jbacik@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.

May I ask under which case we're going to modify an unlinked snapshot?

Maybe metadata relocation?

Thanks,
Qu

> 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 <josef@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");
> +
>  	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);
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]