Re: Btrfs: separate out tests into their own directory V2
Sorry, please ignore this thread... Thanks, wang On 09/01/2013 02:15 PM, Wang Shilong wrote: Hello, Using checkpatch.pl, i get the following warnings(errors): WARNING: kfree(NULL) is safe this check is probably not required #132: FILE: fs/btrfs/free-space-cache.c:3035: + if (map) + kfree(map); WARNING: line over 80 characters #882: FILE: fs/btrfs/tests/free-space-tests.c:211: + ret = test_add_free_space_entry(cache, 4 * 1024 * 1024, 1 * 1024 * 1024, 1); WARNING: line over 80 characters #927: FILE: fs/btrfs/tests/free-space-tests.c:256: + ret = test_add_free_space_entry(cache, 1 * 1024 * 1024, 4 * 1024 * 1024, 1); WARNING: line over 80 characters #947: FILE: fs/btrfs/tests/free-space-tests.c:276: + ret = test_add_free_space_entry(cache, 4 * 1024 * 1024, 4 * 1024 * 1024, 1); WARNING: line over 80 characters #953: FILE: fs/btrfs/tests/free-space-tests.c:282: + ret = test_add_free_space_entry(cache, 2 * 1024 * 1024, 2 * 1024 * 1024, 0); WARNING: line over 80 characters #1016: FILE: fs/btrfs/tests/free-space-tests.c:345: + ret = test_add_free_space_entry(cache, 1 * 1024 * 1024, 2 * 1024 * 1024, 1); WARNING: line over 80 characters #1022: FILE: fs/btrfs/tests/free-space-tests.c:351: + ret = test_add_free_space_entry(cache, 3 * 1024 * 1024, 1 * 1024 * 1024, 0); WARNING: line over 80 characters #1030: FILE: fs/btrfs/tests/free-space-tests.c:359: + test_msg("Error removing bitmap and extent overlapping %d\n", ret); total: 0 errors, 8 warnings, 997 lines checked patch has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. Thanks, Wang -- 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 -- 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
Re: Btrfs: separate out tests into their own directory V2
Hello, Using checkpatch.pl, i get the following warnings(errors): WARNING: kfree(NULL) is safe this check is probably not required #132: FILE: fs/btrfs/free-space-cache.c:3035: + if (map) + kfree(map); WARNING: line over 80 characters #882: FILE: fs/btrfs/tests/free-space-tests.c:211: + ret = test_add_free_space_entry(cache, 4 * 1024 * 1024, 1 * 1024 * 1024, 1); WARNING: line over 80 characters #927: FILE: fs/btrfs/tests/free-space-tests.c:256: + ret = test_add_free_space_entry(cache, 1 * 1024 * 1024, 4 * 1024 * 1024, 1); WARNING: line over 80 characters #947: FILE: fs/btrfs/tests/free-space-tests.c:276: + ret = test_add_free_space_entry(cache, 4 * 1024 * 1024, 4 * 1024 * 1024, 1); WARNING: line over 80 characters #953: FILE: fs/btrfs/tests/free-space-tests.c:282: + ret = test_add_free_space_entry(cache, 2 * 1024 * 1024, 2 * 1024 * 1024, 0); WARNING: line over 80 characters #1016: FILE: fs/btrfs/tests/free-space-tests.c:345: + ret = test_add_free_space_entry(cache, 1 * 1024 * 1024, 2 * 1024 * 1024, 1); WARNING: line over 80 characters #1022: FILE: fs/btrfs/tests/free-space-tests.c:351: + ret = test_add_free_space_entry(cache, 3 * 1024 * 1024, 1 * 1024 * 1024, 0); WARNING: line over 80 characters #1030: FILE: fs/btrfs/tests/free-space-tests.c:359: + test_msg("Error removing bitmap and extent overlapping %d\n", ret); total: 0 errors, 8 warnings, 997 lines checked patch has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. Thanks, Wang -- 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
Re: [PATCH] Btrfs: separate out tests into their own directory V2
On Fri, Aug 30, 2013 at 09:44:59AM +0800, Wang Shilong wrote: > On 08/30/2013 03:29 AM, Josef Bacik wrote: > >The plan is to have a bunch of unit tests that run when btrfs is loaded when > >you > >build with the appropriate config option. My ultimate goal is to have a test > >for every non-static function we have, but at first I'm going to focus on the > >things that cause us the most problems. To start out with this just adds a > >tests/ directory and moves the existing free space cache tests into that > >directory and sets up all of the infrastructure. Thanks, > > > >Signed-off-by: Josef Bacik > > Nothing serious, use checkpatch.pl and i get the following warnings: > > WARNING: kfree(NULL) is safe this check is probably not required > #132: FILE: fs/btrfs/free-space-cache.c:3035: > +if (map) > +kfree(map); > Good point about this, I will do it. > WARNING: line over 80 characters > #882: FILE: fs/btrfs/tests/free-space-tests.c:211: > +ret = test_add_free_space_entry(cache, 4 * 1024 * 1024, 1 * 1024 * > 1024, 1); > > WARNING: line over 80 characters > #927: FILE: fs/btrfs/tests/free-space-tests.c:256: > +ret = test_add_free_space_entry(cache, 1 * 1024 * 1024, 4 * 1024 * > 1024, 1); > > WARNING: line over 80 characters > #947: FILE: fs/btrfs/tests/free-space-tests.c:276: > +ret = test_add_free_space_entry(cache, 4 * 1024 * 1024, 4 * 1024 * > 1024, 1); > > WARNING: line over 80 characters > #953: FILE: fs/btrfs/tests/free-space-tests.c:282: > +ret = test_add_free_space_entry(cache, 2 * 1024 * 1024, 2 * 1024 * > 1024, 0); > > WARNING: line over 80 characters > #1016: FILE: fs/btrfs/tests/free-space-tests.c:345: > +ret = test_add_free_space_entry(cache, 1 * 1024 * 1024, 2 * 1024 * > 1024, 1); > > WARNING: line over 80 characters > #1022: FILE: fs/btrfs/tests/free-space-tests.c:351: > +ret = test_add_free_space_entry(cache, 3 * 1024 * 1024, 1 * 1024 * > 1024, 0); > > WARNING: line over 80 characters > #1030: FILE: fs/btrfs/tests/free-space-tests.c:359: > +test_msg("Error removing bitmap and extent overlapping %d\n", ret); > Yeah I skipped the 80 char limit in here to make the tests a little easier to read. Thanks, Josef -- 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
Re: [PATCH] Btrfs: separate out tests into their own directory V2
On 08/30/2013 03:29 AM, Josef Bacik wrote: The plan is to have a bunch of unit tests that run when btrfs is loaded when you build with the appropriate config option. My ultimate goal is to have a test for every non-static function we have, but at first I'm going to focus on the things that cause us the most problems. To start out with this just adds a tests/ directory and moves the existing free space cache tests into that directory and sets up all of the infrastructure. Thanks, Signed-off-by: Josef Bacik Nothing serious, use checkpatch.pl and i get the following warnings: WARNING: kfree(NULL) is safe this check is probably not required #132: FILE: fs/btrfs/free-space-cache.c:3035: +if (map) +kfree(map); WARNING: line over 80 characters #882: FILE: fs/btrfs/tests/free-space-tests.c:211: +ret = test_add_free_space_entry(cache, 4 * 1024 * 1024, 1 * 1024 * 1024, 1); WARNING: line over 80 characters #927: FILE: fs/btrfs/tests/free-space-tests.c:256: +ret = test_add_free_space_entry(cache, 1 * 1024 * 1024, 4 * 1024 * 1024, 1); WARNING: line over 80 characters #947: FILE: fs/btrfs/tests/free-space-tests.c:276: +ret = test_add_free_space_entry(cache, 4 * 1024 * 1024, 4 * 1024 * 1024, 1); WARNING: line over 80 characters #953: FILE: fs/btrfs/tests/free-space-tests.c:282: +ret = test_add_free_space_entry(cache, 2 * 1024 * 1024, 2 * 1024 * 1024, 0); WARNING: line over 80 characters #1016: FILE: fs/btrfs/tests/free-space-tests.c:345: +ret = test_add_free_space_entry(cache, 1 * 1024 * 1024, 2 * 1024 * 1024, 1); WARNING: line over 80 characters #1022: FILE: fs/btrfs/tests/free-space-tests.c:351: +ret = test_add_free_space_entry(cache, 3 * 1024 * 1024, 1 * 1024 * 1024, 0); WARNING: line over 80 characters #1030: FILE: fs/btrfs/tests/free-space-tests.c:359: +test_msg("Error removing bitmap and extent overlapping %d\n", ret); Thanks, Wang -- 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
[PATCH] Btrfs: separate out tests into their own directory V2
The plan is to have a bunch of unit tests that run when btrfs is loaded when you build with the appropriate config option. My ultimate goal is to have a test for every non-static function we have, but at first I'm going to focus on the things that cause us the most problems. To start out with this just adds a tests/ directory and moves the existing free space cache tests into that directory and sets up all of the infrastructure. Thanks, Signed-off-by: Josef Bacik --- V1->V2: register the file system _after_ running the selftests fs/btrfs/Makefile |2 + fs/btrfs/free-space-cache.c | 488 + fs/btrfs/free-space-cache.h |8 +- fs/btrfs/super.c | 19 +- fs/btrfs/tests/btrfs-tests.h | 34 +++ fs/btrfs/tests/free-space-tests.c | 395 ++ 6 files changed, 510 insertions(+), 436 deletions(-) create mode 100644 fs/btrfs/tests/btrfs-tests.h create mode 100644 fs/btrfs/tests/free-space-tests.c diff --git a/fs/btrfs/Makefile b/fs/btrfs/Makefile index 3932224..47553d5 100644 --- a/fs/btrfs/Makefile +++ b/fs/btrfs/Makefile @@ -12,3 +12,5 @@ btrfs-y += super.o ctree.o extent-tree.o print-tree.o root-tree.o dir-item.o \ btrfs-$(CONFIG_BTRFS_FS_POSIX_ACL) += acl.o btrfs-$(CONFIG_BTRFS_FS_CHECK_INTEGRITY) += check-integrity.o + +btrfs-$(CONFIG_BTRFS_FS_RUN_SANITY_TESTS) += tests/free-space-tests.o diff --git a/fs/btrfs/free-space-cache.c b/fs/btrfs/free-space-cache.c index 2c262a5..34ecc65 100644 --- a/fs/btrfs/free-space-cache.c +++ b/fs/btrfs/free-space-cache.c @@ -2972,33 +2972,68 @@ int btrfs_write_out_ino_cache(struct btrfs_root *root, } #ifdef CONFIG_BTRFS_FS_RUN_SANITY_TESTS -static struct btrfs_block_group_cache *init_test_block_group(void) +/* + * Use this if you need to make a bitmap or extent entry specifically, it + * doesn't do any of the merging that add_free_space does, this acts a lot like + * how the free space cache loading stuff works, so you can get really weird + * configurations. + */ +int test_add_free_space_entry(struct btrfs_block_group_cache *cache, + u64 offset, u64 bytes, bool bitmap) { - struct btrfs_block_group_cache *cache; + struct btrfs_free_space_ctl *ctl = cache->free_space_ctl; + struct btrfs_free_space *info = NULL, *bitmap_info; + void *map = NULL; + u64 bytes_added; + int ret; - cache = kzalloc(sizeof(*cache), GFP_NOFS); - if (!cache) - return NULL; - cache->free_space_ctl = kzalloc(sizeof(*cache->free_space_ctl), - GFP_NOFS); - if (!cache->free_space_ctl) { - kfree(cache); - return NULL; +again: + if (!info) { + info = kmem_cache_zalloc(btrfs_free_space_cachep, GFP_NOFS); + if (!info) + return -ENOMEM; } - cache->key.objectid = 0; - cache->key.offset = 1024 * 1024 * 1024; - cache->key.type = BTRFS_BLOCK_GROUP_ITEM_KEY; - cache->sectorsize = 4096; + if (!bitmap) { + spin_lock(&ctl->tree_lock); + info->offset = offset; + info->bytes = bytes; + ret = link_free_space(ctl, info); + spin_unlock(&ctl->tree_lock); + if (ret) + kmem_cache_free(btrfs_free_space_cachep, info); + return ret; + } + + if (!map) { + map = kzalloc(PAGE_CACHE_SIZE, GFP_NOFS); + if (!map) { + kmem_cache_free(btrfs_free_space_cachep, info); + return -ENOMEM; + } + } + + spin_lock(&ctl->tree_lock); + bitmap_info = tree_search_offset(ctl, offset_to_bitmap(ctl, offset), +1, 0); + if (!bitmap_info) { + info->bitmap = map; + map = NULL; + add_new_bitmap(ctl, info, offset); + bitmap_info = info; + } - spin_lock_init(&cache->lock); - INIT_LIST_HEAD(&cache->list); - INIT_LIST_HEAD(&cache->cluster_list); - INIT_LIST_HEAD(&cache->new_bg_list); + bytes_added = add_bytes_to_bitmap(ctl, bitmap_info, offset, bytes); + bytes -= bytes_added; + offset += bytes_added; + spin_unlock(&ctl->tree_lock); - btrfs_init_free_space_ctl(cache); + if (bytes) + goto again; - return cache; + if (map) + kfree(map); + return 0; } /* @@ -3006,8 +3041,8 @@ static struct btrfs_block_group_cache *init_test_block_group(void) * just used to check the absence of space, so if there is free space in the * range at all we will return 1. */ -static int check_exists(struct btrfs_block_group_cache *cache, u64 offset, - u64 bytes) +int test_check_exists(struct btrfs_block_group_cach
[PATCH] Btrfs: separate out tests into their own directory V2
The plan is to have a bunch of unit tests that run when btrfs is loaded when you build with the appropriate config option. My ultimate goal is to have a test for every non-static function we have, but at first I'm going to focus on the things that cause us the most problems. To start out with this just adds a tests/ directory and moves the existing free space cache tests into that directory and sets up all of the infrastructure. Thanks, Signed-off-by: Josef Bacik --- V1->V2: -fix some build errors with different config options -add gpl header to the new files fs/btrfs/Makefile |2 + fs/btrfs/free-space-cache.c | 488 + fs/btrfs/free-space-cache.h |8 +- fs/btrfs/super.c | 10 +- fs/btrfs/tests/btrfs-tests.h | 34 +++ fs/btrfs/tests/free-space-tests.c | 395 ++ 6 files changed, 505 insertions(+), 432 deletions(-) create mode 100644 fs/btrfs/tests/btrfs-tests.h create mode 100644 fs/btrfs/tests/free-space-tests.c diff --git a/fs/btrfs/Makefile b/fs/btrfs/Makefile index 3932224..47553d5 100644 --- a/fs/btrfs/Makefile +++ b/fs/btrfs/Makefile @@ -12,3 +12,5 @@ btrfs-y += super.o ctree.o extent-tree.o print-tree.o root-tree.o dir-item.o \ btrfs-$(CONFIG_BTRFS_FS_POSIX_ACL) += acl.o btrfs-$(CONFIG_BTRFS_FS_CHECK_INTEGRITY) += check-integrity.o + +btrfs-$(CONFIG_BTRFS_FS_RUN_SANITY_TESTS) += tests/free-space-tests.o diff --git a/fs/btrfs/free-space-cache.c b/fs/btrfs/free-space-cache.c index 2c262a5..34ecc65 100644 --- a/fs/btrfs/free-space-cache.c +++ b/fs/btrfs/free-space-cache.c @@ -2972,33 +2972,68 @@ int btrfs_write_out_ino_cache(struct btrfs_root *root, } #ifdef CONFIG_BTRFS_FS_RUN_SANITY_TESTS -static struct btrfs_block_group_cache *init_test_block_group(void) +/* + * Use this if you need to make a bitmap or extent entry specifically, it + * doesn't do any of the merging that add_free_space does, this acts a lot like + * how the free space cache loading stuff works, so you can get really weird + * configurations. + */ +int test_add_free_space_entry(struct btrfs_block_group_cache *cache, + u64 offset, u64 bytes, bool bitmap) { - struct btrfs_block_group_cache *cache; + struct btrfs_free_space_ctl *ctl = cache->free_space_ctl; + struct btrfs_free_space *info = NULL, *bitmap_info; + void *map = NULL; + u64 bytes_added; + int ret; - cache = kzalloc(sizeof(*cache), GFP_NOFS); - if (!cache) - return NULL; - cache->free_space_ctl = kzalloc(sizeof(*cache->free_space_ctl), - GFP_NOFS); - if (!cache->free_space_ctl) { - kfree(cache); - return NULL; +again: + if (!info) { + info = kmem_cache_zalloc(btrfs_free_space_cachep, GFP_NOFS); + if (!info) + return -ENOMEM; } - cache->key.objectid = 0; - cache->key.offset = 1024 * 1024 * 1024; - cache->key.type = BTRFS_BLOCK_GROUP_ITEM_KEY; - cache->sectorsize = 4096; + if (!bitmap) { + spin_lock(&ctl->tree_lock); + info->offset = offset; + info->bytes = bytes; + ret = link_free_space(ctl, info); + spin_unlock(&ctl->tree_lock); + if (ret) + kmem_cache_free(btrfs_free_space_cachep, info); + return ret; + } + + if (!map) { + map = kzalloc(PAGE_CACHE_SIZE, GFP_NOFS); + if (!map) { + kmem_cache_free(btrfs_free_space_cachep, info); + return -ENOMEM; + } + } + + spin_lock(&ctl->tree_lock); + bitmap_info = tree_search_offset(ctl, offset_to_bitmap(ctl, offset), +1, 0); + if (!bitmap_info) { + info->bitmap = map; + map = NULL; + add_new_bitmap(ctl, info, offset); + bitmap_info = info; + } - spin_lock_init(&cache->lock); - INIT_LIST_HEAD(&cache->list); - INIT_LIST_HEAD(&cache->cluster_list); - INIT_LIST_HEAD(&cache->new_bg_list); + bytes_added = add_bytes_to_bitmap(ctl, bitmap_info, offset, bytes); + bytes -= bytes_added; + offset += bytes_added; + spin_unlock(&ctl->tree_lock); - btrfs_init_free_space_ctl(cache); + if (bytes) + goto again; - return cache; + if (map) + kfree(map); + return 0; } /* @@ -3006,8 +3041,8 @@ static struct btrfs_block_group_cache *init_test_block_group(void) * just used to check the absence of space, so if there is free space in the * range at all we will return 1. */ -static int check_exists(struct btrfs_block_group_cache *cache, u64 offset, - u64 bytes) +int test_check_exist