Re: [RFC ONLY 5/8] btrfs: add code to delete raid extent

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

On 2022/5/16 22:31, Johannes Thumshirn wrote:
> Add boilerplate code to delete entries from the raid-stripe-tree if the
> corresponding file extent got deleted.
>
> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
> ---
>   fs/btrfs/ctree.c            |   1 +
>   fs/btrfs/extent-tree.c      |   9 +++
>   fs/btrfs/file.c             |   1 -
>   fs/btrfs/raid-stripe-tree.c | 111 ++++++++++++++++++++++++++++++++++++
>   fs/btrfs/raid-stripe-tree.h |   8 +++
>   5 files changed, 129 insertions(+), 1 deletion(-)
>
> diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
> index 1e24695ede0a..b7b4e421e9b8 100644
> --- a/fs/btrfs/ctree.c
> +++ b/fs/btrfs/ctree.c
> @@ -3623,6 +3623,7 @@ static noinline int setup_leaf_for_split(struct btrfs_trans_handle *trans,
>   	btrfs_item_key_to_cpu(leaf, &key, path->slots[0]);
>
>   	BUG_ON(key.type != BTRFS_EXTENT_DATA_KEY &&
> +	       key.type != BTRFS_RAID_STRIPE_KEY &&
>   	       key.type != BTRFS_EXTENT_CSUM_KEY);
>
>   	if (btrfs_leaf_free_space(leaf) >= ins_len)
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index f477035a2ac2..00af3e469881 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -36,6 +36,7 @@
>   #include "rcu-string.h"
>   #include "zoned.h"
>   #include "dev-replace.h"
> +#include "raid-stripe-tree.h"
>
>   #undef SCRAMBLE_DELAYED_REFS
>
> @@ -3199,6 +3200,14 @@ static int __btrfs_free_extent(struct btrfs_trans_handle *trans,
>   			}
>   		}

Considering we're already in __btrfs_free_extents(), and the branch
we're in is already for refs == 1 case, which means we're already the
last one owning the file extent (and its stripe tree entry).
>
> +		if (is_data) {
> +			ret = btrfs_delete_raid_extent(trans, bytenr, num_bytes);
> +			if (ret) {
> +				btrfs_abort_transaction(trans, ret);
> +				return ret;
> +			}
> +		}
> +
>   		ret = btrfs_del_items(trans, extent_root, path, path->slots[0],
>   				      num_to_del);
>   		if (ret) {
> diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
> index bd329316945f..6021188dcb9a 100644
> --- a/fs/btrfs/file.c
> +++ b/fs/btrfs/file.c
> @@ -1009,7 +1009,6 @@ int btrfs_drop_extents(struct btrfs_trans_handle *trans,
>   		btrfs_release_path(path);
>   out:
>   	args->drop_end = found ? min(args->end, last_end) : args->end;
> -
>   	return ret;
>   }
>
> diff --git a/fs/btrfs/raid-stripe-tree.c b/fs/btrfs/raid-stripe-tree.c
> index 426066bd7c0d..370ea68fe343 100644
> --- a/fs/btrfs/raid-stripe-tree.c
> +++ b/fs/btrfs/raid-stripe-tree.c
> @@ -6,6 +6,117 @@
>   #include "raid-stripe-tree.h"
>   #include "volumes.h"
>
> +int btrfs_delete_raid_extent(struct btrfs_trans_handle *trans, u64 start,
> +			     u64 length)
> +{
> +	struct btrfs_fs_info *fs_info = trans->fs_info;
> +	struct btrfs_root *stripe_root = fs_info->stripe_root;
> +	struct btrfs_path *path;
> +	struct btrfs_key stripe_key;
> +	struct btrfs_key found_key;
> +	struct extent_buffer *leaf;
> +	u64 end = start + length;
> +	u64 found_start;
> +	u64 found_end;
> +	int slot;
> +	int ret;
> +
> +	if (!stripe_root)
> +		return 0;
> +
> +	stripe_key.objectid = start;
> +	stripe_key.type = BTRFS_RAID_STRIPE_KEY;
> +	stripe_key.offset = end;
> +
> +	path = btrfs_alloc_path();
> +	if (!path)
> +		return -ENOMEM;
> +
> +	ret = btrfs_search_slot(trans, stripe_root, &stripe_key, path, -1, 1);
> +	if (ret < 0)
> +		goto out;
> +	if (ret == 0)
> +		goto delete;
> +
> +	leaf = path->nodes[0];
> +	slot = path->slots[0];
> +	btrfs_item_key_to_cpu(leaf, &found_key, slot);
> +	found_start = found_key.objectid;
> +	found_end = found_start + found_key.offset;
> +
> +	/*
> +	 * | -- range to drop --|
> +	 * | ---------- extent ---------- |
> +	 */

Thus I believe we don't need those complex checking.

The call site has make sure we're the last one owning the file extent,
and since raid stripe is 1:1 mapped to the full extent (not just part of
a data extent, like btrfs_file_extent_item can do), we should be safe to
just do an ASSERT() without the complex split.


Thus, I guess to be extra accurate, the 1:1 mapping is between an (data)
EXTENT_ITEM and a raid stripe?

Thanks,
Qu
> +front_split:
> +	if (start > found_start) {
> +		struct btrfs_key front_key;
> +		struct btrfs_dp_stripe *raid_stripe;
> +		struct extent_buffer *front_leaf;
> +		struct btrfs_stripe_extent *stripe_extent;
> +		int num_stripes;
> +		int i;
> +
> +		front_key.objectid = found_start + length;
> +		front_key.type = BTRFS_RAID_STRIPE_KEY;
> +		front_key.offset = found_end - length;
> +
> +		num_stripes = btrfs_num_raid_stripes(btrfs_item_size(leaf, slot));
> +
> +		ret = btrfs_duplicate_item(trans, stripe_root, path, &front_key);
> +		if (ret == -EAGAIN) {
> +			btrfs_release_path(path);
> +			goto front_split;
> +		}
> +		if (ret < 0)
> +			goto out;
> +		front_leaf = path->nodes[0];
> +
> +		raid_stripe = btrfs_item_ptr(leaf, slot, struct btrfs_dp_stripe);
> +		stripe_extent = &raid_stripe->extents;
> +		for (i = 0; i < num_stripes; i++) {
> +			u64 physical;
> +
> +			physical = btrfs_stripe_extent_offset(leaf, stripe_extent);
> +			btrfs_set_stripe_extent_offset(front_leaf, stripe_extent,
> +							  physical + length);
> +			stripe_extent++;
> +		}
> +
> +		btrfs_mark_buffer_dirty(front_leaf);
> +	}
> +
> +	/*
> +	 *           | -- range to drop --|
> +	 * | ---------- extent ---------- |
> +	 */
> +tail_split:
> +	if (end < found_end) {
> +		struct btrfs_key tail_key;
> +
> +
> +		tail_key.objectid = start;
> +		tail_key.type = BTRFS_RAID_STRIPE_KEY;
> +		tail_key.offset = found_end - end;
> +
> +		ret = btrfs_duplicate_item(trans, stripe_root, path, &tail_key);
> +		if (ret == -EAGAIN) {
> +			btrfs_release_path(path);
> +			goto tail_split;
> +		}
> +		if (ret < 0)
> +			goto out;
> +		btrfs_mark_buffer_dirty(path->nodes[0]);
> +	}
> +
> +delete:
> +	ret = btrfs_del_item(trans, stripe_root, path);
> +out:
> +	btrfs_free_path(path);
> +	return ret;
> +
> +}
> +
>   static void btrfs_insert_raid_extent(struct btrfs_trans_handle *trans,
>   				     struct btrfs_io_context *bioc)
>   {
> diff --git a/fs/btrfs/raid-stripe-tree.h b/fs/btrfs/raid-stripe-tree.h
> index 320a110ecc66..766634df8601 100644
> --- a/fs/btrfs/raid-stripe-tree.h
> +++ b/fs/btrfs/raid-stripe-tree.h
> @@ -5,8 +5,16 @@
>
>   #include "volumes.h"
>
> +int btrfs_delete_raid_extent(struct btrfs_trans_handle *trans, u64 start,
> +			     u64 length);
>   void btrfs_raid_stripe_tree_fn(struct work_struct *work);
>
> +static inline int btrfs_num_raid_stripes(u32 item_size)
> +{
> +	return item_size - offsetof(struct btrfs_dp_stripe, extents) /
> +		sizeof(struct btrfs_stripe_extent);
> +}
> +
>   static inline bool btrfs_need_stripe_tree_update(struct btrfs_io_context *bioc)
>   {
>   	u64 type = bioc->map_type & BTRFS_BLOCK_GROUP_TYPE_MASK;