Message ID | 20191210180045.2047-1-nborisov@suse.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [v2] btrfs: Add self-tests for btrfs_rmap_block | expand |
On Tue, Dec 10, 2019 at 08:00:45PM +0200, Nikolay Borisov wrote: > This is enough to exercise out of boundary address exclusion as well as > address matching. > > Signed-off-by: Nikolay Borisov <nborisov@suse.com> > --- > V2: > * Adjusted comments about some members of struct rmap_test_vector > * Fixed inline comments > * Correctly handle error when initialising dummy device > * Other minor cosmetic changes around comments/braces for single statement 'if' > and structure initialization I still found issues unfixed from v1 and some that I did not notice before > fs/btrfs/tests/extent-map-tests.c | 146 +++++++++++++++++++++++++++++- > 1 file changed, 145 insertions(+), 1 deletion(-) > > diff --git a/fs/btrfs/tests/extent-map-tests.c b/fs/btrfs/tests/extent-map-tests.c > index 4a7f796c9900..4878904434af 100644 > --- a/fs/btrfs/tests/extent-map-tests.c > +++ b/fs/btrfs/tests/extent-map-tests.c > @@ -6,6 +6,10 @@ > #include <linux/types.h> > #include "btrfs-tests.h" > #include "../ctree.h" > +#include "../volumes.h" > +#include "../disk-io.h" > +#include "../block-group.h" > + Extra newline > > static void free_extent_map_tree(struct extent_map_tree *em_tree) > { > @@ -437,11 +441,144 @@ static int test_case_4(struct btrfs_fs_info *fs_info, > return ret; > } > > +struct rmap_test_vector { > + u64 raid_type; > + u64 physical_start; > + u64 data_stripe_size; > + u64 num_data_stripes; > + u64 num_stripes; > + /* Assume we won't have more than 5 physical stripes */ > + u64 data_stripe_phys_start[5]; > + int expected_mapped_addr; This should be bool > + /* Physical to logical addresses */ > + u64 mapped_logical[5]; > +}; > + > +static int test_rmap_block(struct btrfs_fs_info *fs_info, > + struct rmap_test_vector *test) > +{ > + struct extent_map *em; > + struct map_lookup *map = NULL; > + u64 *logical; > + int i, out_ndaddrs, out_stripe_len; > + int ret = -EINVAL; > + > + em = alloc_extent_map(); > + if (!em) { > + test_std_err(TEST_ALLOC_EXTENT_MAP); > + return -ENOMEM; > + } > + > + map = kmalloc(map_lookup_size(test->num_stripes), GFP_KERNEL); > + if (!map) { > + kfree(em); > + test_std_err(TEST_ALLOC_EXTENT_MAP); > + return -ENOMEM; > + } > + > + set_bit(EXTENT_FLAG_FS_MAPPING, &em->flags); > + /* Start at 4gb logical address */ > + em->start = SZ_4G; > + em->len = test->data_stripe_size * test->num_data_stripes; > + em->block_len = em->len; > + em->orig_block_len = test->data_stripe_size; > + em->map_lookup = map; > + > + map->num_stripes = test->num_stripes; > + map->stripe_len = BTRFS_STRIPE_LEN; > + map->type = test->raid_type; > + > + for (i = 0; i < map->num_stripes; i++) > + { > + struct btrfs_device *dev = btrfs_alloc_dummy_device(fs_info); > + if (!dev) { > + test_err("ENOMEM while allocating dummy device"); ret = -ENOMEM; And the error message should follow the scheme of the other standard error messages (defined in test_error) > + goto out; > + } > + map->stripes[i].dev = dev; > + map->stripes[i].physical = test->data_stripe_phys_start[i]; > + } > + > + write_lock(&fs_info->mapping_tree.lock); > + ret = add_extent_mapping(&fs_info->mapping_tree, em, 0); > + write_unlock(&fs_info->mapping_tree.lock); > + if (ret) > + test_err("Error adding block group mapping to mapping tree"); Error found but no exit, other selftests do that. And no capital letter at the beginning of the string. I've added a label before the 2nd free_extent_map. > + > + ret = btrfs_rmap_block(fs_info, em->start, btrfs_sb_offset(1), > + &logical, &out_ndaddrs, &out_stripe_len); > + if (ret || (out_ndaddrs == 0 && test->expected_mapped_addr)) { > + test_err("Didn't rmap anything but expected %d", ... in all strings passed to test_err > + test->expected_mapped_addr); > + goto out; > + } > + > + if (out_stripe_len != BTRFS_STRIPE_LEN) { > + test_err("Calculated stripe len doesn't match"); Here > + goto out; > + } > + > + if (out_ndaddrs != test->expected_mapped_addr) { > + for (i = 0; i < out_ndaddrs; i++) > + test_msg("Mapped %llu", logical[i]); Here > + test_err("Unexpected number of mapped addresses: %d", out_ndaddrs); Here > + goto out; > + } > + > + for (i = 0; i < out_ndaddrs; i++) { > + if (logical[i] != test->mapped_logical[i]) { > + test_err("Unexpected logical address mapped"); Here > + goto out; > + } > + } > + > + ret = 0; > +out: > + write_lock(&fs_info->mapping_tree.lock); > + remove_extent_mapping(&fs_info->mapping_tree, em); > + write_unlock(&fs_info->mapping_tree.lock); > + /* For us */ > + free_extent_map(em); > + /* For the tree */ > + free_extent_map(em); > + return ret; > +} > + > int btrfs_test_extent_map(void) > { > struct btrfs_fs_info *fs_info = NULL; > struct extent_map_tree *em_tree; > - int ret = 0; > + int ret = 0, i; > + struct rmap_test_vector rmap_tests[] = { > + { > + /* > + * Tests a chunk with 2 data stripes one of which > + * interesects the physical address of the super block > + * is correctly recognised. > + */ > + .raid_type = BTRFS_BLOCK_GROUP_RAID1, > + .physical_start = SZ_64M - SZ_4M, > + .data_stripe_size = SZ_256M, > + .num_data_stripes = 2, > + .num_stripes = 2, > + .data_stripe_phys_start = {SZ_64M - SZ_4M, SZ_64M - SZ_4M + SZ_256M}, Formatting > + .expected_mapped_addr = 1, > + .mapped_logical= {SZ_4G + SZ_4M} > + }, > + { > + /* test that out of range physical addresses are ignored */ > + > + /* SINGLE chunk type */ > + .raid_type = 0, > + .physical_start = SZ_4G, > + .data_stripe_size = SZ_256M, > + .num_data_stripes = 1, > + .num_stripes = 1, > + .data_stripe_phys_start = {SZ_256M}, > + .expected_mapped_addr = 0, > + .mapped_logical = {0} > + } > + }; > > test_msg("running extent_map tests"); > > @@ -474,6 +611,13 @@ int btrfs_test_extent_map(void) > goto out; > ret = test_case_4(fs_info, em_tree); > > + test_msg("Running rmap tests."); test_msg("running rmap tests"); > + for (i = 0; i < ARRAY_SIZE(rmap_tests); i++) { > + ret = test_rmap_block(fs_info, &rmap_tests[i]); > + if (ret) > + goto out; > + } > + > out: > kfree(em_tree); > btrfs_free_dummy_fs_info(fs_info); > -- > 2.17.1
On 2.01.20 г. 17:40 ч., David Sterba wrote: > On Tue, Dec 10, 2019 at 08:00:45PM +0200, Nikolay Borisov wrote: >> This is enough to exercise out of boundary address exclusion as well as >> address matching. >> >> Signed-off-by: Nikolay Borisov <nborisov@suse.com> >> --- >> V2: >> * Adjusted comments about some members of struct rmap_test_vector >> * Fixed inline comments >> * Correctly handle error when initialising dummy device >> * Other minor cosmetic changes around comments/braces for single statement 'if' >> and structure initialization > > I still found issues unfixed from v1 and some that I did not notice > before > >> fs/btrfs/tests/extent-map-tests.c | 146 +++++++++++++++++++++++++++++- >> 1 file changed, 145 insertions(+), 1 deletion(-) >> >> diff --git a/fs/btrfs/tests/extent-map-tests.c b/fs/btrfs/tests/extent-map-tests.c >> index 4a7f796c9900..4878904434af 100644 >> --- a/fs/btrfs/tests/extent-map-tests.c >> +++ b/fs/btrfs/tests/extent-map-tests.c >> @@ -6,6 +6,10 @@ >> #include <linux/types.h> >> #include "btrfs-tests.h" >> #include "../ctree.h" >> +#include "../volumes.h" >> +#include "../disk-io.h" >> +#include "../block-group.h" >> + > > Extra newline > >> >> static void free_extent_map_tree(struct extent_map_tree *em_tree) >> { >> @@ -437,11 +441,144 @@ static int test_case_4(struct btrfs_fs_info *fs_info, >> return ret; >> } >> >> +struct rmap_test_vector { >> + u64 raid_type; >> + u64 physical_start; >> + u64 data_stripe_size; >> + u64 num_data_stripes; >> + u64 num_stripes; >> + /* Assume we won't have more than 5 physical stripes */ >> + u64 data_stripe_phys_start[5]; >> + int expected_mapped_addr; > > This should be bool Actually the idea here is for expected_mapped_addr to contains the number of addresses we are expected to map. Currently tests only expect 0 or 1 but if tests are expanded in the future this might be 2 or 3. THe body of the test does: if (out_ndaddrs != test->expected_mapped_addr) { for (i = 0; i < out_ndaddrs; i++) test_msg("Mapped %llu", logical[i]); <snip> >> int btrfs_test_extent_map(void) >> { >> struct btrfs_fs_info *fs_info = NULL; >> struct extent_map_tree *em_tree; >> - int ret = 0; >> + int ret = 0, i; >> + struct rmap_test_vector rmap_tests[] = { >> + { >> + /* >> + * Tests a chunk with 2 data stripes one of which >> + * interesects the physical address of the super block >> + * is correctly recognised. >> + */ >> + .raid_type = BTRFS_BLOCK_GROUP_RAID1, >> + .physical_start = SZ_64M - SZ_4M, >> + .data_stripe_size = SZ_256M, >> + .num_data_stripes = 2, >> + .num_stripes = 2, >> + .data_stripe_phys_start = {SZ_64M - SZ_4M, SZ_64M - SZ_4M + SZ_256M}, > > Formatting What do you mean? <snip>
On Fri, Jan 10, 2020 at 04:46:20PM +0200, Nikolay Borisov wrote: > >> + int expected_mapped_addr; > > > > This should be bool > > Actually the idea here is for expected_mapped_addr to contains the > number of addresses we are expected to map. Currently tests only expect > 0 or 1 but if tests are expanded in the future this might be 2 or 3. > > THe body of the test does: > > if (out_ndaddrs != test->expected_mapped_addr) { > for (i = 0; i < out_ndaddrs; i++) > > test_msg("Mapped %llu", logical[i]); Ok, int is fine then. > >> + struct rmap_test_vector rmap_tests[] = { > >> + { > >> + /* > >> + * Tests a chunk with 2 data stripes one of which > >> + * interesects the physical address of the super block > >> + * is correctly recognised. > >> + */ > >> + .raid_type = BTRFS_BLOCK_GROUP_RAID1, > >> + .physical_start = SZ_64M - SZ_4M, > >> + .data_stripe_size = SZ_256M, > >> + .num_data_stripes = 2, > >> + .num_stripes = 2, > >> + .data_stripe_phys_start = {SZ_64M - SZ_4M, SZ_64M - SZ_4M + SZ_256M}, > > > > Formatting > > What do you mean? Line over 80 cols
diff --git a/fs/btrfs/tests/extent-map-tests.c b/fs/btrfs/tests/extent-map-tests.c index 4a7f796c9900..4878904434af 100644 --- a/fs/btrfs/tests/extent-map-tests.c +++ b/fs/btrfs/tests/extent-map-tests.c @@ -6,6 +6,10 @@ #include <linux/types.h> #include "btrfs-tests.h" #include "../ctree.h" +#include "../volumes.h" +#include "../disk-io.h" +#include "../block-group.h" + static void free_extent_map_tree(struct extent_map_tree *em_tree) { @@ -437,11 +441,144 @@ static int test_case_4(struct btrfs_fs_info *fs_info, return ret; } +struct rmap_test_vector { + u64 raid_type; + u64 physical_start; + u64 data_stripe_size; + u64 num_data_stripes; + u64 num_stripes; + /* Assume we won't have more than 5 physical stripes */ + u64 data_stripe_phys_start[5]; + int expected_mapped_addr; + /* Physical to logical addresses */ + u64 mapped_logical[5]; +}; + +static int test_rmap_block(struct btrfs_fs_info *fs_info, + struct rmap_test_vector *test) +{ + struct extent_map *em; + struct map_lookup *map = NULL; + u64 *logical; + int i, out_ndaddrs, out_stripe_len; + int ret = -EINVAL; + + em = alloc_extent_map(); + if (!em) { + test_std_err(TEST_ALLOC_EXTENT_MAP); + return -ENOMEM; + } + + map = kmalloc(map_lookup_size(test->num_stripes), GFP_KERNEL); + if (!map) { + kfree(em); + test_std_err(TEST_ALLOC_EXTENT_MAP); + return -ENOMEM; + } + + set_bit(EXTENT_FLAG_FS_MAPPING, &em->flags); + /* Start at 4gb logical address */ + em->start = SZ_4G; + em->len = test->data_stripe_size * test->num_data_stripes; + em->block_len = em->len; + em->orig_block_len = test->data_stripe_size; + em->map_lookup = map; + + map->num_stripes = test->num_stripes; + map->stripe_len = BTRFS_STRIPE_LEN; + map->type = test->raid_type; + + for (i = 0; i < map->num_stripes; i++) + { + struct btrfs_device *dev = btrfs_alloc_dummy_device(fs_info); + if (!dev) { + test_err("ENOMEM while allocating dummy device"); + goto out; + } + map->stripes[i].dev = dev; + map->stripes[i].physical = test->data_stripe_phys_start[i]; + } + + write_lock(&fs_info->mapping_tree.lock); + ret = add_extent_mapping(&fs_info->mapping_tree, em, 0); + write_unlock(&fs_info->mapping_tree.lock); + if (ret) + test_err("Error adding block group mapping to mapping tree"); + + ret = btrfs_rmap_block(fs_info, em->start, btrfs_sb_offset(1), + &logical, &out_ndaddrs, &out_stripe_len); + if (ret || (out_ndaddrs == 0 && test->expected_mapped_addr)) { + test_err("Didn't rmap anything but expected %d", + test->expected_mapped_addr); + goto out; + } + + if (out_stripe_len != BTRFS_STRIPE_LEN) { + test_err("Calculated stripe len doesn't match"); + goto out; + } + + if (out_ndaddrs != test->expected_mapped_addr) { + for (i = 0; i < out_ndaddrs; i++) + test_msg("Mapped %llu", logical[i]); + test_err("Unexpected number of mapped addresses: %d", out_ndaddrs); + goto out; + } + + for (i = 0; i < out_ndaddrs; i++) { + if (logical[i] != test->mapped_logical[i]) { + test_err("Unexpected logical address mapped"); + goto out; + } + } + + ret = 0; +out: + write_lock(&fs_info->mapping_tree.lock); + remove_extent_mapping(&fs_info->mapping_tree, em); + write_unlock(&fs_info->mapping_tree.lock); + /* For us */ + free_extent_map(em); + /* For the tree */ + free_extent_map(em); + return ret; +} + int btrfs_test_extent_map(void) { struct btrfs_fs_info *fs_info = NULL; struct extent_map_tree *em_tree; - int ret = 0; + int ret = 0, i; + struct rmap_test_vector rmap_tests[] = { + { + /* + * Tests a chunk with 2 data stripes one of which + * interesects the physical address of the super block + * is correctly recognised. + */ + .raid_type = BTRFS_BLOCK_GROUP_RAID1, + .physical_start = SZ_64M - SZ_4M, + .data_stripe_size = SZ_256M, + .num_data_stripes = 2, + .num_stripes = 2, + .data_stripe_phys_start = {SZ_64M - SZ_4M, SZ_64M - SZ_4M + SZ_256M}, + .expected_mapped_addr = 1, + .mapped_logical= {SZ_4G + SZ_4M} + }, + { + /* test that out of range physical addresses are ignored */ + + /* SINGLE chunk type */ + .raid_type = 0, + .physical_start = SZ_4G, + .data_stripe_size = SZ_256M, + .num_data_stripes = 1, + .num_stripes = 1, + .data_stripe_phys_start = {SZ_256M}, + .expected_mapped_addr = 0, + .mapped_logical = {0} + } + }; test_msg("running extent_map tests"); @@ -474,6 +611,13 @@ int btrfs_test_extent_map(void) goto out; ret = test_case_4(fs_info, em_tree); + test_msg("Running rmap tests."); + for (i = 0; i < ARRAY_SIZE(rmap_tests); i++) { + ret = test_rmap_block(fs_info, &rmap_tests[i]); + if (ret) + goto out; + } + out: kfree(em_tree); btrfs_free_dummy_fs_info(fs_info);
This is enough to exercise out of boundary address exclusion as well as address matching. Signed-off-by: Nikolay Borisov <nborisov@suse.com> --- V2: * Adjusted comments about some members of struct rmap_test_vector * Fixed inline comments * Correctly handle error when initialising dummy device * Other minor cosmetic changes around comments/braces for single statement 'if' and structure initialization fs/btrfs/tests/extent-map-tests.c | 146 +++++++++++++++++++++++++++++- 1 file changed, 145 insertions(+), 1 deletion(-) -- 2.17.1