On Fri, May 27, 2016 at 03:00:36PM +0800, Feifei Xu wrote:
> self-tests code assumes 4k as the sectorsize and nodesize. This commit
> enables the self-tests code to be executed on non-4k page sized
> systems (e.g. ppc64).
>
> To test all possible sectorsizes, this commit adds a sectorsize
> array. This commit executes the tests for all possible sectorsizes and
> nodesizes.
>
> Signed-off-by: Feifei Xu <xufei...@linux.vnet.ibm.com>
> Signed-off-by: Chandan Rajendra <chan...@linux.vnet.ibm.com>
> ---
> fs/btrfs/ctree.c | 6 +-
> fs/btrfs/disk-io.c | 9 +-
> fs/btrfs/disk-io.h | 3 +-
> fs/btrfs/extent_io.c | 10 +-
> fs/btrfs/extent_io.h | 4 +-
> fs/btrfs/free-space-cache.c | 2 +-
> fs/btrfs/super.c | 62 ++++--
> fs/btrfs/tests/btrfs-tests.c | 6 +-
> fs/btrfs/tests/btrfs-tests.h | 27 +--
> fs/btrfs/tests/extent-buffer-tests.c | 13 +-
> fs/btrfs/tests/extent-io-tests.c | 85 ++++----
> fs/btrfs/tests/free-space-tests.c | 67 +++---
> fs/btrfs/tests/free-space-tree-tests.c | 30 +--
> fs/btrfs/tests/inode-tests.c | 379
> ++++++++++++++++++---------------
> fs/btrfs/tests/qgroup-tests.c | 111 ++++++----
> 15 files changed, 462 insertions(+), 352 deletions(-)
The patch is quite big, please separate all changes that are not related
to the 4096/sectorsize/PAGE_SIZE change, eg. adding missing "\n" to
strings, replacement of constants with defines
> @@ -1314,14 +1315,16 @@ static struct btrfs_root *btrfs_alloc_root(struct
> btrfs_fs_info *fs_info,
>
> #ifdef CONFIG_BTRFS_FS_RUN_SANITY_TESTS
> /* Should only be used by the testing infrastructure */
> -struct btrfs_root *btrfs_alloc_dummy_root(void)
> +struct btrfs_root *
> +btrfs_alloc_dummy_root(u32 sectorsize, u32 nodesize)
Please keep the style as-is, ie. return type and name on one line
> {
> struct btrfs_root *root;
>
> root = btrfs_alloc_root(NULL, GFP_KERNEL);
> if (!root)
> return ERR_PTR(-ENOMEM);
> - __setup_root(4096, 4096, 4096, root, NULL, 1);
> + __setup_root(nodesize, sectorsize, sectorsize, root, NULL,
The 3rd parameter is stripesize, but we don't use it anywhere so
sectorsize should be fine, just looks a bit strange.
> + BTRFS_ROOT_TREE_OBJECTID);
> set_bit(BTRFS_ROOT_DUMMY_ROOT, &root->state);
> root->alloc_bytenr = 0;
>
> --- a/fs/btrfs/extent_io.c
> +++ b/fs/btrfs/extent_io.c
> @@ -4714,16 +4714,16 @@ err:
> }
>
> struct extent_buffer *alloc_dummy_extent_buffer(struct btrfs_fs_info
> *fs_info,
> - u64 start)
> + u64 start, u32 alt_nodesize)
This could be named 'nodesize'.
> {
> unsigned long len;
>
> if (!fs_info) {
> /*
> * Called only from tests that don't always have a fs_info
> - * available, but we know that nodesize is 4096
> + * available
> */
> - len = 4096;
> + len = alt_nodesize;
> } else {
> len = fs_info->tree_root->nodesize;
> }
> @@ -4819,7 +4819,7 @@ struct extent_buffer *find_extent_buffer(struct
> btrfs_fs_info *fs_info,
>
> #ifdef CONFIG_BTRFS_FS_RUN_SANITY_TESTS
> struct extent_buffer *alloc_test_extent_buffer(struct btrfs_fs_info *fs_info,
> - u64 start)
> + u64 start, u32 alt_nodesize)
dtto
> {
> struct extent_buffer *eb, *exists = NULL;
> int ret;
> @@ -4827,7 +4827,7 @@ struct extent_buffer *alloc_test_extent_buffer(struct
> btrfs_fs_info *fs_info,
> eb = find_extent_buffer(fs_info, start);
> if (eb)
> return eb;
> - eb = alloc_dummy_extent_buffer(fs_info, start);
> + eb = alloc_dummy_extent_buffer(fs_info, start, alt_nodesize);
> if (!eb)
> return NULL;
> eb->fs_info = fs_info;
> ctl->extents_thresh = 0;
> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
> index bf71071..c97c250 100644
> --- a/fs/btrfs/super.c
> +++ b/fs/btrfs/super.c
> @@ -2318,28 +2318,54 @@ static void btrfs_print_mod_info(void)
>
> static int btrfs_run_sanity_tests(void)
> {
> - int ret;
> -
> + int ret, i;
> + u32 sectorsize, nodesize;
> + u32 test_sectorsize[] = {
> + PAGE_SIZE,
> + };
> ret = btrfs_init_test_fs();
> if (ret)
> return ret;
> + for (i = 0; i < ARRAY_SIZE(test_sectorsize); i++) {
> + /* Validate sectorsize. */
> + sectorsize = test_sectorsize[i];
> + if (!is_power_of_2(sectorsize) || sectorsize < SZ_4K ||
> + sectorsize > PAGE_SIZE) {
> + continue;
> + }
>
> - ret = btrfs_test_free_space_cache();
> - if (ret)
> - goto out;
> - ret = btrfs_test_extent_buffer_operations();
> - if (ret)
> - goto out;
> - ret = btrfs_test_extent_io();
> - if (ret)
> - goto out;
> - ret = btrfs_test_inodes();
> - if (ret)
> - goto out;
> - ret = btrfs_test_qgroups();
> - if (ret)
> - goto out;
> - ret = btrfs_test_free_space_tree();
> + for (nodesize = sectorsize;
> + nodesize <= BTRFS_MAX_METADATA_BLOCKSIZE;
> + nodesize <<= 1) {
> + /* Validate nodesize.*/
> + if (!is_power_of_2(nodesize) || nodesize < sectorsize ||
> + nodesize > SZ_64K){
The whole if does not make sense, the values will always satisfy the
conditions.
> + pr_info("Invalid nodesize %u\n", nodesize);
> + continue;
> + }
> + pr_info("Btrfs: Sanity tests sectorsize %u nodesize
> %u\n",
> + sectorsize, nodesize);
Please use the same wording as the 'test_msg' does (that we cannot use
here).
> + ret = btrfs_test_free_space_cache(sectorsize, nodesize);
> + if (ret)
> + goto out;
> + ret = btrfs_test_extent_buffer_operations(sectorsize,
> + nodesize);
> + if (ret)
> + goto out;
> + ret = btrfs_test_extent_io(sectorsize, nodesize);
> + if (ret)
> + goto out;
> + ret = btrfs_test_inodes(sectorsize, nodesize);
> + if (ret)
> + goto out;
> + ret = btrfs_test_qgroups(sectorsize, nodesize);
> + if (ret)
> + goto out;
> + ret = btrfs_test_free_space_tree(sectorsize, nodesize);
> + if (ret)
> + goto out;
> + }
> + }
> out:
> btrfs_destroy_test_fs();
> return ret;
> + test_msg("Cache free space is not %u\n", sectorsize);
> return -EINVAL;
> }
>
> offset = btrfs_find_space_for_alloc(cache,
> - 0, 4096, 0,
> + 0, sectorsize, 0,
> &max_extent_size);
> if (offset != (SZ_128M + SZ_16M)) {
> - test_msg("Failed to allocate 4Kb from space cache, returned
> offset is: %llu\n",
> - offset);
> + test_msg("Failed to allocate %u, returned offset:%llu\n",
test_msg("Failed to allocate %u, returned offset: %llu\n",
> + sectorsize, offset);
> return -EINVAL;
> }
>
> return ret;
> ret = check_num_extents_and_bitmaps(cache, 2, 1);
> if (ret)
> @@ -782,8 +788,8 @@ test_steal_space_from_bitmap_to_extent(struct
> btrfs_block_group_cache *cache)
> return -ENOENT;
> }
>
> - if (cache->free_space_ctl->free_space != (SZ_1M + 8192)) {
> - test_msg("Cache free space is not 1Mb + 8Kb\n");
> + if (cache->free_space_ctl->free_space != (SZ_1M + 2 * sectorsize)) {
> + test_msg("Cache free space is not 1Mb + %u\n", 2*sectorsize);
test_msg("Cache free space is not 1Mb + %u\n", 2 * sectorsize);
There are more instances of the pointed style issues, please fix all of
them. As the changes do not affect functionality I'll add the paches to
for-next, but I'm expecting a v2.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
