diff mbox series

[4/4] btrfs: test invalid splitting when skipping pinned drop extent_map

Message ID cb4e2f77d7ab9670223ca0d76594abb93bb1c32d.1692305624.git.josef@toxicpanda.com (mailing list archive)
State New, archived
Headers show
Series Fix incorrect splitting logic in btrfs_drop_extent_map_range | expand

Commit Message

Josef Bacik Aug. 17, 2023, 8:57 p.m. UTC 
This reproduces the bug fixed by "btrfs: fix incorrect splitting in
btrfs_drop_extent_map_range", we were improperly calculating the range
for the split extent.  Add a test that exercises this scenario and
validates that we get the correct resulting extent_maps in our tree.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 fs/btrfs/tests/extent-map-tests.c | 138 ++++++++++++++++++++++++++++++
 1 file changed, 138 insertions(+)

Comments

Filipe Manana Aug. 18, 2023, 10:49 a.m. UTC | #1 
On Thu, Aug 17, 2023 at 04:57:33PM -0400, Josef Bacik wrote:
> This reproduces the bug fixed by "btrfs: fix incorrect splitting in
> btrfs_drop_extent_map_range", we were improperly calculating the range
> for the split extent.  Add a test that exercises this scenario and
> validates that we get the correct resulting extent_maps in our tree.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>

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

Looks good, just a minor comment below.

> ---
>  fs/btrfs/tests/extent-map-tests.c | 138 ++++++++++++++++++++++++++++++
>  1 file changed, 138 insertions(+)
> 
> diff --git a/fs/btrfs/tests/extent-map-tests.c b/fs/btrfs/tests/extent-map-tests.c
> index 18ab03f0d029..06820a8b4d1f 100644
> --- a/fs/btrfs/tests/extent-map-tests.c
> +++ b/fs/btrfs/tests/extent-map-tests.c
> @@ -710,6 +710,141 @@ static int test_case_6(struct btrfs_fs_info *fs_info,
>  	return ret;
>  }
>  
> +/*
> + * Regression test for btrfs_drop_extent_map_range.  Calling with skip_pinned ==
> + * true would mess up the start/end calculations and subsequent splits would be
> + * incorrect.
> + */
> +static int test_case_7(void)
> +{
> +	struct extent_map_tree *em_tree;
> +	struct extent_map *em = NULL;
> +	struct inode *inode = NULL;

These two initializations to NULL are not needed.

> +	int ret;
> +
> +	test_msg("Running btrfs_drop_extent_cache with pinned");
> +
> +	inode = btrfs_new_test_inode();
> +	if (!inode) {
> +		test_std_err(TEST_ALLOC_INODE);
> +		return -ENOMEM;
> +	}
> +
> +	em_tree = &BTRFS_I(inode)->extent_tree;
> +
> +	em = alloc_extent_map();
> +	if (!em) {
> +		test_std_err(TEST_ALLOC_EXTENT_MAP);
> +		ret = -ENOMEM;
> +		goto out;
> +	}
> +
> +	/* [0, 16K), pinned */
> +	em->start = 0;
> +	em->len = SZ_16K;
> +	em->block_start = 0;
> +	em->block_len = SZ_4K;
> +	set_bit(EXTENT_FLAG_PINNED, &em->flags);
> +	write_lock(&em_tree->lock);
> +	ret = add_extent_mapping(em_tree, em, 0);
> +	write_unlock(&em_tree->lock);
> +	if (ret < 0) {
> +		test_err("couldn't add extent map");
> +		goto out;
> +	}
> +	free_extent_map(em);
> +
> +	em = alloc_extent_map();
> +	if (!em) {
> +		test_std_err(TEST_ALLOC_EXTENT_MAP);
> +		ret = -ENOMEM;
> +		goto out;
> +	}
> +
> +	/* [32K, 48K), not pinned */
> +	em->start = SZ_32K;
> +	em->len = SZ_16K;
> +	em->block_start = SZ_32K;
> +	em->block_len = SZ_16K;
> +	write_lock(&em_tree->lock);
> +	ret = add_extent_mapping(em_tree, em, 0);
> +	write_unlock(&em_tree->lock);
> +	if (ret < 0) {
> +		test_err("couldn't add extent map");
> +		goto out;
> +	}
> +	free_extent_map(em);
> +
> +	/*
> +	 * Drop [0, 36K) This should skip the [0, 4K) extent and then split the
> +	 * [32K, 48K) extent.
> +	 */
> +	btrfs_drop_extent_map_range(BTRFS_I(inode), 0, (36 * SZ_1K) - 1, true);
> +
> +	/* Make sure our extent maps look sane. */
> +	ret = -EINVAL;
> +
> +	em = lookup_extent_mapping(em_tree, 0, SZ_16K);
> +	if (!em) {
> +		test_err("didn't find an em at 0 as expected");
> +		goto out;
> +	}
> +
> +	if (em->start != 0) {
> +		test_err("em->start is %llu, expected 0", em->start);
> +		goto out;
> +	}
> +
> +	if (em->len != SZ_16K) {
> +		test_err("em->len is %llu, expected 16K", em->len);
> +		goto out;
> +	}
> +
> +	free_extent_map(em);
> +
> +	read_lock(&em_tree->lock);
> +	em = lookup_extent_mapping(em_tree, SZ_16K, SZ_16K);
> +	read_unlock(&em_tree->lock);
> +	if (em) {
> +		test_err("found an em when we weren't expecting one");
> +		goto out;
> +	}
> +
> +	read_lock(&em_tree->lock);
> +	em = lookup_extent_mapping(em_tree, SZ_32K, SZ_16K);
> +	read_unlock(&em_tree->lock);
> +	if (!em) {
> +		test_err("didn't find an em at 32K as expected");
> +		goto out;
> +	}
> +
> +	if (em->start != (36 * SZ_1K)) {
> +		test_err("em->start is %llu, expected 36K", em->start);
> +		goto out;
> +	}
> +
> +	if (em->len != (12 * SZ_1K)) {
> +		test_err("em->len is %llu, expected 12K", em->len);
> +		goto out;
> +	}
> +
> +	free_extent_map(em);
> +
> +	read_lock(&em_tree->lock);
> +	em = lookup_extent_mapping(em_tree, 48 * SZ_1K, (u64)-1);
> +	read_unlock(&em_tree->lock);
> +	if (em) {
> +		test_err("found an unexpected em above 48K");
> +		goto out;
> +	}
> +
> +	ret = 0;
> +out:
> +	free_extent_map(em);
> +	iput(inode);
> +	return ret;
> +}
> +
>  struct rmap_test_vector {
>  	u64 raid_type;
>  	u64 physical_start;
> @@ -893,6 +1028,9 @@ int btrfs_test_extent_map(void)
>  	if (ret)
>  		goto out;
>  	ret = test_case_6(fs_info, em_tree);
> +	if (ret)
> +		goto out;
> +	ret = test_case_7();
>  	if (ret)
>  		goto out;
>  
> -- 
> 2.26.3
>
diff mbox series

Patch

diff --git a/fs/btrfs/tests/extent-map-tests.c b/fs/btrfs/tests/extent-map-tests.c
index 18ab03f0d029..06820a8b4d1f 100644
--- a/fs/btrfs/tests/extent-map-tests.c
+++ b/fs/btrfs/tests/extent-map-tests.c
@@ -710,6 +710,141 @@  static int test_case_6(struct btrfs_fs_info *fs_info,
 	return ret;
 }
 
+/*
+ * Regression test for btrfs_drop_extent_map_range.  Calling with skip_pinned ==
+ * true would mess up the start/end calculations and subsequent splits would be
+ * incorrect.
+ */
+static int test_case_7(void)
+{
+	struct extent_map_tree *em_tree;
+	struct extent_map *em = NULL;
+	struct inode *inode = NULL;
+	int ret;
+
+	test_msg("Running btrfs_drop_extent_cache with pinned");
+
+	inode = btrfs_new_test_inode();
+	if (!inode) {
+		test_std_err(TEST_ALLOC_INODE);
+		return -ENOMEM;
+	}
+
+	em_tree = &BTRFS_I(inode)->extent_tree;
+
+	em = alloc_extent_map();
+	if (!em) {
+		test_std_err(TEST_ALLOC_EXTENT_MAP);
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	/* [0, 16K), pinned */
+	em->start = 0;
+	em->len = SZ_16K;
+	em->block_start = 0;
+	em->block_len = SZ_4K;
+	set_bit(EXTENT_FLAG_PINNED, &em->flags);
+	write_lock(&em_tree->lock);
+	ret = add_extent_mapping(em_tree, em, 0);
+	write_unlock(&em_tree->lock);
+	if (ret < 0) {
+		test_err("couldn't add extent map");
+		goto out;
+	}
+	free_extent_map(em);
+
+	em = alloc_extent_map();
+	if (!em) {
+		test_std_err(TEST_ALLOC_EXTENT_MAP);
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	/* [32K, 48K), not pinned */
+	em->start = SZ_32K;
+	em->len = SZ_16K;
+	em->block_start = SZ_32K;
+	em->block_len = SZ_16K;
+	write_lock(&em_tree->lock);
+	ret = add_extent_mapping(em_tree, em, 0);
+	write_unlock(&em_tree->lock);
+	if (ret < 0) {
+		test_err("couldn't add extent map");
+		goto out;
+	}
+	free_extent_map(em);
+
+	/*
+	 * Drop [0, 36K) This should skip the [0, 4K) extent and then split the
+	 * [32K, 48K) extent.
+	 */
+	btrfs_drop_extent_map_range(BTRFS_I(inode), 0, (36 * SZ_1K) - 1, true);
+
+	/* Make sure our extent maps look sane. */
+	ret = -EINVAL;
+
+	em = lookup_extent_mapping(em_tree, 0, SZ_16K);
+	if (!em) {
+		test_err("didn't find an em at 0 as expected");
+		goto out;
+	}
+
+	if (em->start != 0) {
+		test_err("em->start is %llu, expected 0", em->start);
+		goto out;
+	}
+
+	if (em->len != SZ_16K) {
+		test_err("em->len is %llu, expected 16K", em->len);
+		goto out;
+	}
+
+	free_extent_map(em);
+
+	read_lock(&em_tree->lock);
+	em = lookup_extent_mapping(em_tree, SZ_16K, SZ_16K);
+	read_unlock(&em_tree->lock);
+	if (em) {
+		test_err("found an em when we weren't expecting one");
+		goto out;
+	}
+
+	read_lock(&em_tree->lock);
+	em = lookup_extent_mapping(em_tree, SZ_32K, SZ_16K);
+	read_unlock(&em_tree->lock);
+	if (!em) {
+		test_err("didn't find an em at 32K as expected");
+		goto out;
+	}
+
+	if (em->start != (36 * SZ_1K)) {
+		test_err("em->start is %llu, expected 36K", em->start);
+		goto out;
+	}
+
+	if (em->len != (12 * SZ_1K)) {
+		test_err("em->len is %llu, expected 12K", em->len);
+		goto out;
+	}
+
+	free_extent_map(em);
+
+	read_lock(&em_tree->lock);
+	em = lookup_extent_mapping(em_tree, 48 * SZ_1K, (u64)-1);
+	read_unlock(&em_tree->lock);
+	if (em) {
+		test_err("found an unexpected em above 48K");
+		goto out;
+	}
+
+	ret = 0;
+out:
+	free_extent_map(em);
+	iput(inode);
+	return ret;
+}
+
 struct rmap_test_vector {
 	u64 raid_type;
 	u64 physical_start;
@@ -893,6 +1028,9 @@  int btrfs_test_extent_map(void)
 	if (ret)
 		goto out;
 	ret = test_case_6(fs_info, em_tree);
+	if (ret)
+		goto out;
+	ret = test_case_7();
 	if (ret)
 		goto out;