On 2016/5/28 3:29, David Sterba wrote:
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
OK.
@@ -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.
Thanks, I will send out v2 soon according to all above comments.

Thanks
Feifei
--
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

Reply via email to