Re: [PATCH 10/18] btrfs: track owning root in btrfs_ref

[Josef Bacik <josef@toxicpanda.com>]

On Wed, Jul 05, 2023 at 04:20:47PM -0700, Boris Burkov wrote:
> While data extents require us to store additional inline refs to track
> the original owner on free, this information is available implicitly for
> metadata. It is found in the owner field of the header of the tree
> block. Even if other trees refer to this block and the original ref goes
> away, we will not rewrite that header field, so it will reliably give the
> original owner.
> 
> In addition, there is a relocation case where a new data extent needs to
> have an owning root separate from the referring root wired through
> delayed refs.
> 
> To use it for recording simple quota deltas, we need to wire this root
> id through from when we create the delayed ref until we fully process
> it. Store it in the generic btrfs_ref struct of the delayed ref.
> 
> Signed-off-by: Boris Burkov <boris@bur.io>
> ---
>  fs/btrfs/delayed-ref.c |  7 ++++---
>  fs/btrfs/delayed-ref.h | 13 +++++++++++--
>  fs/btrfs/extent-tree.c | 19 ++++++++++++-------
>  fs/btrfs/file.c        | 10 +++++-----
>  fs/btrfs/inode-item.c  |  2 +-
>  fs/btrfs/relocation.c  | 16 +++++++++-------
>  fs/btrfs/tree-log.c    |  3 ++-
>  7 files changed, 44 insertions(+), 26 deletions(-)
> 
> diff --git a/fs/btrfs/delayed-ref.c b/fs/btrfs/delayed-ref.c
> index f0bae1e1c455..49c320f2334b 100644
> --- a/fs/btrfs/delayed-ref.c
> +++ b/fs/btrfs/delayed-ref.c
> @@ -840,7 +840,7 @@ add_delayed_ref_head(struct btrfs_trans_handle *trans,
>  static void init_delayed_ref_common(struct btrfs_fs_info *fs_info,
>  				    struct btrfs_delayed_ref_node *ref,
>  				    u64 bytenr, u64 num_bytes, u64 ref_root,
> -				    int action, u8 ref_type)
> +				    int action, u8 ref_type, u64 owning_root)
>  {
>  	u64 seq = 0;
>  
> @@ -857,6 +857,7 @@ static void init_delayed_ref_common(struct btrfs_fs_info *fs_info,
>  	ref->action = action;
>  	ref->seq = seq;
>  	ref->type = ref_type;
> +	ref->owning_root = owning_root;
>  	RB_CLEAR_NODE(&ref->ref_node);
>  	INIT_LIST_HEAD(&ref->add_list);
>  }
> @@ -915,7 +916,7 @@ int btrfs_add_delayed_tree_ref(struct btrfs_trans_handle *trans,
>  
>  	init_delayed_ref_common(fs_info, &ref->node, bytenr, num_bytes,
>  				generic_ref->tree_ref.ref_root, action,
> -				ref_type);
> +				ref_type, generic_ref->owning_root);
>  	ref->root = generic_ref->tree_ref.ref_root;
>  	ref->parent = parent;
>  	ref->level = level;
> @@ -989,7 +990,7 @@ int btrfs_add_delayed_data_ref(struct btrfs_trans_handle *trans,
>  	else
>  	        ref_type = BTRFS_EXTENT_DATA_REF_KEY;
>  	init_delayed_ref_common(fs_info, &ref->node, bytenr, num_bytes,
> -				ref_root, action, ref_type);
> +				ref_root, action, ref_type, ref_root);
>  	ref->root = ref_root;
>  	ref->parent = parent;
>  	ref->objectid = owner;
> diff --git a/fs/btrfs/delayed-ref.h b/fs/btrfs/delayed-ref.h
> index a71eff78469c..336c33c28191 100644
> --- a/fs/btrfs/delayed-ref.h
> +++ b/fs/btrfs/delayed-ref.h
> @@ -32,6 +32,12 @@ struct btrfs_delayed_ref_node {
>  	/* seq number to keep track of insertion order */
>  	u64 seq;
>  
> +	/*
> +	 * root which originally allocated this extent and owns it for
> +	 * simple quota accounting purposes.
> +	 */
> +	u64 owning_root;
> +
>  	/* ref count on this data structure */
>  	refcount_t refs;
>  
> @@ -239,6 +245,7 @@ struct btrfs_ref {
>  #endif
>  	u64 bytenr;
>  	u64 len;
> +	u64 owning_root;
>  
>  	/* Bytenr of the parent tree block */
>  	u64 parent;
> @@ -278,16 +285,18 @@ static inline u64 btrfs_calc_delayed_ref_bytes(const struct btrfs_fs_info *fs_in
>  }
>  
>  static inline void btrfs_init_generic_ref(struct btrfs_ref *generic_ref,
> -				int action, u64 bytenr, u64 len, u64 parent)
> +				int action, u64 bytenr, u64 len, u64 parent, u64 owning_root)
>  {
>  	generic_ref->action = action;
>  	generic_ref->bytenr = bytenr;
>  	generic_ref->len = len;
>  	generic_ref->parent = parent;
> +	generic_ref->owning_root = owning_root;
>  }
>  
>  static inline void btrfs_init_tree_ref(struct btrfs_ref *generic_ref,
> -				int level, u64 root, u64 mod_root, bool skip_qgroup)
> +				int level, u64 root, u64 mod_root,
> +				bool skip_qgroup)
>  {
>  #ifdef CONFIG_BTRFS_FS_REF_VERIFY
>  	/* If @real_root not set, use @root as fallback */
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index 041f2eb153d7..fa53f7cbd84a 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -2410,7 +2410,7 @@ static int __btrfs_mod_ref(struct btrfs_trans_handle *trans,
>  			num_bytes = btrfs_file_extent_disk_num_bytes(buf, fi);
>  			key.offset -= btrfs_file_extent_offset(buf, fi);
>  			btrfs_init_generic_ref(&generic_ref, action, bytenr,
> -					       num_bytes, parent);
> +					       num_bytes, parent, ref_root);
>  			btrfs_init_data_ref(&generic_ref, ref_root, key.objectid,
>  					    key.offset, root->root_key.objectid,
>  					    for_reloc);
> @@ -2424,7 +2424,7 @@ static int __btrfs_mod_ref(struct btrfs_trans_handle *trans,
>  			bytenr = btrfs_node_blockptr(buf, i);
>  			num_bytes = fs_info->nodesize;
>  			btrfs_init_generic_ref(&generic_ref, action, bytenr,
> -					       num_bytes, parent);
> +					       num_bytes, parent, ref_root);
>  			btrfs_init_tree_ref(&generic_ref, level - 1, ref_root,
>  					    root->root_key.objectid, for_reloc);
>  			if (inc)
> @@ -3242,7 +3242,7 @@ void btrfs_free_tree_block(struct btrfs_trans_handle *trans,
>  	int ret;
>  
>  	btrfs_init_generic_ref(&generic_ref, BTRFS_DROP_DELAYED_REF,
> -			       buf->start, buf->len, parent);
> +			       buf->start, buf->len, parent, btrfs_header_owner(buf));
>  	btrfs_init_tree_ref(&generic_ref, btrfs_header_level(buf),
>  			    root_id, 0, false);
>  
> @@ -4699,12 +4699,16 @@ int btrfs_alloc_reserved_file_extent(struct btrfs_trans_handle *trans,
>  				     struct btrfs_key *ins)
>  {
>  	struct btrfs_ref generic_ref = { 0 };
> +	u64 root_objectid = root->root_key.objectid;
> +	u64 owning_root = root_objectid;
> +
> +	BUG_ON(root_objectid == BTRFS_TREE_LOG_OBJECTID);
>  

This is a duplicate check of what's checked below.  Also we don't want to add
new BUG_ON()'s, we want to add ASSERT()'s.  Thanks,

Josef