On 14.11.18 г. 21:53 ч., David Sterba wrote:
> On Wed, Nov 14, 2018 at 02:54:37PM +0100, Johannes Thumshirn wrote:
>> On 14/11/2018 14:52, Nikolay Borisov wrote:
>>> I agree with this patch, however you go into the gray area of
>>> "everything which is exported should have btrfs_ prefix". It's up to
>>> David to see if he is content with this change.
>>
>> Yep you're right. I think I should change it, but I'll wait for David's
>> response first.
>
> There's one prior example of the conditionally exported functions:
> btrfs_find_lock_delalloc_range . This function declaration and
> definition are under the ifdef and it's a simple wrapper around a static
> function find_lock_delalloc_range.
I have a patch which actually simplifies this somewhat because, frankly I find it stupid and redundant to do this frickin' dance for functions which are used only in debugging. Here is the diff (still not submitted since it's waiting for some other cleanups to be merged):
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -1556,10 +1556,13 @@ static noinline int lock_delalloc_pages(struct inode *inode,
*
* 1 is returned if we find something, 0 if nothing was in the tree
*/
-static noinline_for_stack u64 find_lock_delalloc_range(struct inode *inode,
- struct extent_io_tree *tree,
- struct page *locked_page, u64 *start,
- u64 *end)
+#ifndef CONFIG_BTRFS_FS_RUN_SANITY_TESTS
+static
+#endif
+noinline_for_stack u64 find_lock_delalloc_range(struct inode *inode,
+ struct extent_io_tree *tree,
+ struct page *locked_page,
+ u64 *start, u64 *end)
{
u64 max_bytes = BTRFS_MAX_EXTENT_SIZE;
u64 delalloc_start;
@@ -1637,16 +1640,6 @@ static noinline_for_stack u64 find_lock_delalloc_range(struct inode *inode,
return found;
}
-#ifdef CONFIG_BTRFS_FS_RUN_SANITY_TESTS
-u64 btrfs_find_lock_delalloc_range(struct inode *inode,
- struct extent_io_tree *tree,
- struct page *locked_page, u64 *start,
- u64 *end)
-{
- return find_lock_delalloc_range(inode, tree, locked_page, start, end);
-}
-#endif
-
static int __process_pages_contig(struct address_space *mapping,
struct page *locked_page,
pgoff_t start_index, pgoff_t end_index,
diff --git a/fs/btrfs/extent_io.h b/fs/btrfs/extent_io.h
index 30bfeefa2d89..12d300fae7a0 100644
--- a/fs/btrfs/extent_io.h
+++ b/fs/btrfs/extent_io.h
@@ -522,10 +522,8 @@ int free_io_failure(struct extent_io_tree *failure_tree,
struct extent_io_tree *io_tree,
struct io_failure_record *rec);
#ifdef CONFIG_BTRFS_FS_RUN_SANITY_TESTS
-u64 btrfs_find_lock_delalloc_range(struct inode *inode,
- struct extent_io_tree *tree,
- struct page *locked_page, u64 *start,
- u64 *end);
+u64 find_lock_delalloc_range(struct inode *inode, struct extent_io_tree *tree,
+ struct page *locked_page, u64 *start, u64 *end);
#endif
struct extent_buffer *alloc_test_extent_buffer(struct btrfs_fs_info *fs_info,
>
> I'd do the same for the functions in your list, where applies. A static
> function can be better optimized and I don't want to make it harder for
> the compiler just to have it exported for the tests. They're not enabled
> by default and never in production builds.
>
> So:
>
> free-space-tree.h:
>
> #ifdef CONFIG_BTRFS_FS_RUN_SANITY_TESTS
>
> struct btrfs_free_space_info *btrfs_search_free_space_info(
> struct btrfs_trans_handle *trans, struct btrfs_fs_info *fs_info,
> struct btrfs_block_group_cache *block_group, struct btrfs_path *path,
> int cow);
> ...
>
> #endif
>
> free-space-tree.c:
>
> #ifdef CONFIG_BTRFS_FS_RUN_SANITY_TESTS
>
> struct btrfs_free_space_info *btrfs_search_free_space_info(
> struct btrfs_trans_handle *trans, struct btrfs_fs_info *fs_info,
> struct btrfs_block_group_cache *block_group, struct btrfs_path *path,
> int cow)
> {
> return search_free_space_info(trans, fs_info, block_group, path, cow);
> }
>
> ...
>
> #endif
>
> I hope it's a reasonable compromise among the options.
>
