Re: [PATCH 2/4] btrfs: add extent_map tests for dropping with odd layouts

[Filipe Manana <fdmanana@kernel.org>]

On Thu, Aug 17, 2023 at 04:57:31PM -0400, Josef Bacik wrote:
> While investigating weird problems with the extent_map I wrote a self
> test testing the various edge cases of btrfs_drop_extent_map_range.
> This can split in different ways and behaves different in each case, so
> test the various edge cases to make sure everything is functioning
> properly.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>

Reviewed-by: Filipe Manana <fdmanana@suse.com>

Looks good, just some minor comment below.

> ---
>  fs/btrfs/tests/extent-map-tests.c | 219 ++++++++++++++++++++++++++++++
>  1 file changed, 219 insertions(+)
> 
> diff --git a/fs/btrfs/tests/extent-map-tests.c b/fs/btrfs/tests/extent-map-tests.c
> index ed0f36ae5346..d5f5e48ab55c 100644
> --- a/fs/btrfs/tests/extent-map-tests.c
> +++ b/fs/btrfs/tests/extent-map-tests.c
> @@ -6,6 +6,7 @@
>  #include <linux/types.h>
>  #include "btrfs-tests.h"
>  #include "../ctree.h"
> +#include "../btrfs_inode.h"
>  #include "../volumes.h"
>  #include "../disk-io.h"
>  #include "../block-group.h"
> @@ -442,6 +443,219 @@ static int test_case_4(struct btrfs_fs_info *fs_info,
>  	return ret;
>  }
>  
> +static int add_compressed_extent(struct extent_map_tree *em_tree,
> +				 const u64 start, const u64 len,
> +				 const u64 block_start)
> +{
> +	struct extent_map *em;
> +	int ret;
> +
> +	em = alloc_extent_map();
> +	if (!em) {
> +		test_std_err(TEST_ALLOC_EXTENT_MAP);
> +		return -ENOMEM;
> +	}
> +
> +	em->start = start;
> +	em->len = len;
> +	em->block_start = block_start;
> +	em->block_len = SZ_4K;
> +	set_bit(EXTENT_FLAG_COMPRESSED, &em->flags);
> +	write_lock(&em_tree->lock);
> +	ret = add_extent_mapping(em_tree, em, 0);
> +	write_unlock(&em_tree->lock);
> +	free_extent_map(em);
> +	if (ret < 0) {
> +		test_err("cannot add extent map [%llu, %llu)", start,
> +			 start + len);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +struct extent_range {
> +	u64 start;
> +	u64 len;
> +};
> +
> +/* The valid states of the tree after every drop, as described below. */
> +struct extent_range valid_ranges[][7] = {
> +	{
> +	  { .start = 0,			.len = SZ_8K },		/* [0, 8K) */
> +	  { .start = SZ_4K * 3,		.len = SZ_4K * 3},	/* [12k, 24k) */
> +	  { .start = SZ_4K * 6,		.len = SZ_4K * 3},	/* [24k, 36k) */
> +	  { .start = SZ_32K + SZ_4K,	.len = SZ_4K},		/* [36k, 40k) */
> +	  { .start = SZ_4K * 10,	.len = SZ_4K * 6},	/* [40k, 64k) */
> +	},
> +	{
> +	  { .start = 0,			.len = SZ_8K },		/* [0, 8K) */
> +	  { .start = SZ_4K * 5,		.len = SZ_4K},		/* [20k, 24k) */
> +	  { .start = SZ_4K * 6,		.len = SZ_4K * 3},	/* [24k, 36k) */
> +	  { .start = SZ_32K + SZ_4K,	.len = SZ_4K},		/* [36k, 40k) */
> +	  { .start = SZ_4K * 10,	.len = SZ_4K * 6},	/* [40k, 64k) */
> +	},
> +	{
> +	  { .start = 0,			.len = SZ_8K },		/* [0, 8K) */
> +	  { .start = SZ_4K * 5,		.len = SZ_4K},		/* [20k, 24k) */
> +	  { .start = SZ_4K * 6,		.len = SZ_4K},		/* [24k, 28k) */
> +	  { .start = SZ_32K,		.len = SZ_4K},		/* [32k, 36k) */
> +	  { .start = SZ_32K + SZ_4K,	.len = SZ_4K},		/* [36k, 40k) */
> +	  { .start = SZ_4K * 10,	.len = SZ_4K * 6},	/* [40k, 64k) */
> +	},
> +	{
> +	  { .start = 0,			.len = SZ_8K},		/* [0, 8K) */
> +	  { .start = SZ_4K * 5,		.len = SZ_4K},		/* [20k, 24k) */
> +	  { .start = SZ_4K * 6,		.len = SZ_4K},		/* [24k, 28k) */
> +	}
> +};
> +
> +static int validate_range(struct extent_map_tree *em_tree, int index)
> +{
> +	struct rb_node *n;
> +	int i;
> +
> +	for (i = 0, n = rb_first_cached(&em_tree->map);
> +	     valid_ranges[index][i].len && n; i++, n = rb_next(n)) {
> +		struct extent_map *entry = rb_entry(n, struct extent_map, rb_node);
> +
> +		if (entry->start != valid_ranges[index][i].start) {
> +			test_err("mapping has start %llu expected %llu",
> +				 entry->start, valid_ranges[index][i].start);
> +			return -EINVAL;
> +		}
> +
> +		if (entry->len != valid_ranges[index][i].len) {
> +			test_err("mapping has len %llu expected %llu",
> +				 entry->len, valid_ranges[index][i].len);
> +			return -EINVAL;
> +		}
> +	}
> +
> +	/*
> +	 * We exited because we don't have any more entries in the extent_map
> +	 * but we still expect more valid entries.
> +	 */
> +	if (valid_ranges[index][i].len) {
> +		test_err("missing an entry");
> +		return -EINVAL;
> +	}
> +
> +	/* We exited the loop but still have entries in the extent map. */
> +	if (n) {
> +		test_err("we have a left over entry in the extent map we didn't expect");
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +/*
> + * Test scenario:
> + *
> + * Test the various edge cases of btrfs_drop_extent_map_range, create the
> + * following ranges
> + *
> + * [0, 12k)[12k, 24k)[24k, 36k)[36k, 40k)[40k,64k)
> + *
> + * And then we'll drop:
> + *
> + * [8k, 12k) - test the single front split
> + * [12k, 20k) - test the single back split
> + * [28k, 32k) - test the double split
> + * [32k, 64k) - test whole em dropping
> + *
> + * They'll have the EXTENT_FLAG_COMPRESSED flag set to keep the em tree from
> + * merging the em's.
> + */
> +static int test_case_5(void)
> +{
> +	struct extent_map_tree *em_tree;
> +	struct inode *inode = NULL;

This initialization is not needed.

> +	u64 start, end;
> +	int ret;
> +
> +	test_msg("Running btrfs_drop_extent_map_range tests");
> +
> +	inode = btrfs_new_test_inode();
> +	if (!inode) {
> +		test_std_err(TEST_ALLOC_INODE);
> +		return -ENOMEM;
> +	}
> +
> +	em_tree = &BTRFS_I(inode)->extent_tree;
> +
> +	/* [0, 12k) */
> +	ret = add_compressed_extent(em_tree, 0, SZ_4K * 3, 0);
> +	if (ret) {
> +		test_err("cannot add extent range [0, 12K)");
> +		goto out;
> +	}
> +
> +	/* [12k, 24k) */
> +	ret = add_compressed_extent(em_tree, SZ_4K * 3, SZ_4K * 3, SZ_4K);
> +	if (ret) {
> +		test_err("cannot add extent range [12k, 24k)");
> +		goto out;
> +	}
> +
> +	/* [24k, 36k) */
> +	ret = add_compressed_extent(em_tree, SZ_4K * 6, SZ_4K * 3, SZ_8K);
> +	if (ret) {
> +		test_err("cannot add extent range [12k, 24k)");
> +		goto out;
> +	}
> +
> +	/* [36k, 40k) */
> +	ret = add_compressed_extent(em_tree, SZ_32K + SZ_4K, SZ_4K, SZ_4K * 3);
> +	if (ret) {
> +		test_err("cannot add extent range [12k, 24k)");
> +		goto out;
> +	}
> +
> +	/* [40k, 64k) */
> +	ret = add_compressed_extent(em_tree, SZ_4K * 10, SZ_4K * 6, SZ_16K);
> +	if (ret) {
> +		test_err("cannot add extent range [12k, 24k)");
> +		goto out;
> +	}
> +
> +	/* Drop [8k, 12k) */
> +	start = SZ_8K;
> +	end = (3 * SZ_4K) - 1;
> +	btrfs_drop_extent_map_range(BTRFS_I(inode), start, end, false);
> +	ret = validate_range(&BTRFS_I(inode)->extent_tree, 0);
> +	if (ret)
> +		goto out;
> +
> +	/* Drop [12k, 20k) */
> +	start = SZ_4K * 3;
> +	end = SZ_16K + SZ_4K - 1;
> +	btrfs_drop_extent_map_range(BTRFS_I(inode), start, end, false);
> +	ret = validate_range(&BTRFS_I(inode)->extent_tree, 1);
> +	if (ret)
> +		goto out;
> +
> +	/* Drop [28k, 32k) */
> +	start = SZ_32K - SZ_4K;
> +	end = SZ_32K - 1;
> +	btrfs_drop_extent_map_range(BTRFS_I(inode), start, end, false);
> +	ret = validate_range(&BTRFS_I(inode)->extent_tree, 2);
> +	if (ret)
> +		goto out;
> +
> +	/* Drop [32k, 64k) */
> +	start = SZ_32K;
> +	end = SZ_64K - 1;
> +	btrfs_drop_extent_map_range(BTRFS_I(inode), start, end, false);
> +	ret = validate_range(&BTRFS_I(inode)->extent_tree, 3);
> +	if (ret)
> +		goto out;
> +out:
> +	iput(inode);
> +	return ret;
> +}
> +
>  struct rmap_test_vector {
>  	u64 raid_type;
>  	u64 physical_start;
> @@ -619,6 +833,11 @@ int btrfs_test_extent_map(void)
>  	if (ret)
>  		goto out;
>  	ret = test_case_4(fs_info, em_tree);
> +	if (ret)
> +		goto out;
> +	ret = test_case_5();
> +	if (ret)
> +		goto out;
>  
>  	test_msg("running rmap tests");
>  	for (i = 0; i < ARRAY_SIZE(rmap_tests); i++) {
> -- 
> 2.26.3
>