diff mbox series

[8/8] btrfs: move btrfs_free_excluded_extents() into block-group.c

Message ID cb507b1dbff5ee7f776d98a9ea1da0d40ddeacfc.1688137156.git.fdmanana@suse.com (mailing list archive)
State New, archived
Headers show
Series btrfs: remove a couple BUG_ON()s and cleanups | expand

Commit Message

Filipe Manana June 30, 2023, 3:03 p.m. UTC 
From: Filipe Manana <fdmanana@suse.com>

The function btrfs_free_excluded_extents() is only used by block-group.c,
so move it into block-group.c and make it static. Also removed unnecessary
variables that are used only once.

Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 fs/btrfs/block-group.c |  6 ++++++
 fs/btrfs/extent-tree.c | 12 ------------
 fs/btrfs/extent-tree.h |  1 -
 3 files changed, 6 insertions(+), 13 deletions(-)

Comments

Filipe Manana July 10, 2023, 9:56 a.m. UTC | #1 
On 07/07/23 23:17, Boris Burkov wrote:
> On Fri, Jun 30, 2023 at 04:03:51PM +0100, Filipe Manana wrote:
>> The function btrfs_free_excluded_extents() is only used by block-group.c,
>> so move it into block-group.c and make it static. Also removed unnecessary
>> variables that are used only once.
> 
> Since you added the btrfs_ for the function that's exported in the
> header earlier I think it would be nice to also drop it from this
> one as it goes static in block_group.c.

I don't think we have a policy to make static function without the 
"btrfs_" prefix mandatory. We have plenty of cases with and without the 
prefix.

Personally I prefer to have the prefix, because when reading a function 
call it makes it obvious that it's btrfs code and not generic code being 
called.

However I'm not against dropping the prefix, if that's what everyone 
prefers.

Also btw, you forgot to CC the list... for all patches in the same 
series. I'm CC'ing back the list here, but not for the other patches.

Thanks.

> 
>>
>> Signed-off-by: Filipe Manana <fdmanana@suse.com>
>> ---
>>   fs/btrfs/block-group.c |  6 ++++++
>>   fs/btrfs/extent-tree.c | 12 ------------
>>   fs/btrfs/extent-tree.h |  1 -
>>   3 files changed, 6 insertions(+), 13 deletions(-)
>>
>> diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c
>> index 7485660a1529..796e4be167a0 100644
>> --- a/fs/btrfs/block-group.c
>> +++ b/fs/btrfs/block-group.c
>> @@ -823,6 +823,12 @@ static int load_extent_tree_free(struct btrfs_caching_control *caching_ctl)
>>   	return ret;
>>   }
>>   
>> +static inline void btrfs_free_excluded_extents(const struct btrfs_block_group *bg)
>> +{
>> +	clear_extent_bits(&bg->fs_info->excluded_extents, bg->start,
>> +			  bg->start + bg->length - 1, EXTENT_UPTODATE);
>> +}
>> +
>>   static noinline void caching_thread(struct btrfs_work *work)
>>   {
>>   	struct btrfs_block_group *block_group;
>> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
>> index 701fa08cffb6..b0fcc2a042ad 100644
>> --- a/fs/btrfs/extent-tree.c
>> +++ b/fs/btrfs/extent-tree.c
>> @@ -69,18 +69,6 @@ static int block_group_bits(struct btrfs_block_group *cache, u64 bits)
>>   	return (cache->flags & bits) == bits;
>>   }
>>   
>> -void btrfs_free_excluded_extents(struct btrfs_block_group *cache)
>> -{
>> -	struct btrfs_fs_info *fs_info = cache->fs_info;
>> -	u64 start, end;
>> -
>> -	start = cache->start;
>> -	end = start + cache->length - 1;
>> -
>> -	clear_extent_bits(&fs_info->excluded_extents, start, end,
>> -			  EXTENT_UPTODATE);
>> -}
>> -
>>   /* simple helper to search for an existing data extent at a given offset */
>>   int btrfs_lookup_data_extent(struct btrfs_fs_info *fs_info, u64 start, u64 len)
>>   {
>> diff --git a/fs/btrfs/extent-tree.h b/fs/btrfs/extent-tree.h
>> index 3b2f265f4653..b9e148adcd28 100644
>> --- a/fs/btrfs/extent-tree.h
>> +++ b/fs/btrfs/extent-tree.h
>> @@ -96,7 +96,6 @@ int btrfs_get_extent_inline_ref_type(const struct extent_buffer *eb,
>>   				     enum btrfs_inline_ref_type is_data);
>>   u64 hash_extent_data_ref(u64 root_objectid, u64 owner, u64 offset);
>>   
>> -void btrfs_free_excluded_extents(struct btrfs_block_group *cache);
>>   int btrfs_run_delayed_refs(struct btrfs_trans_handle *trans, unsigned long count);
>>   void btrfs_cleanup_ref_head_accounting(struct btrfs_fs_info *fs_info,
>>   				  struct btrfs_delayed_ref_root *delayed_refs,
>> -- 
>> 2.34.1
David Sterba July 12, 2023, 4:10 p.m. UTC | #2 
On Mon, Jul 10, 2023 at 10:56:19AM +0100, Filipe Manana wrote:
> On 07/07/23 23:17, Boris Burkov wrote:
> > On Fri, Jun 30, 2023 at 04:03:51PM +0100, Filipe Manana wrote:
> >> The function btrfs_free_excluded_extents() is only used by block-group.c,
> >> so move it into block-group.c and make it static. Also removed unnecessary
> >> variables that are used only once.
> > 
> > Since you added the btrfs_ for the function that's exported in the
> > header earlier I think it would be nice to also drop it from this
> > one as it goes static in block_group.c.
> 
> I don't think we have a policy to make static function without the 
> "btrfs_" prefix mandatory. We have plenty of cases with and without the 
> prefix.
> 
> Personally I prefer to have the prefix, because when reading a function 
> call it makes it obvious that it's btrfs code and not generic code being 
> called.
> 
> However I'm not against dropping the prefix, if that's what everyone 
> prefers.

I think we don't have a strict policy here, exported functions should
have the btrfs_ prefix unless there's some other common prefix or
historical reasons. For static helper it's nice to have to make the
functions obvious in stack traces but some simple helpers don't need it.
More prefixes or naming unifications are probably better in the long run.
diff mbox series

Patch

diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c
index 7485660a1529..796e4be167a0 100644
--- a/fs/btrfs/block-group.c
+++ b/fs/btrfs/block-group.c
@@ -823,6 +823,12 @@  static int load_extent_tree_free(struct btrfs_caching_control *caching_ctl)
 	return ret;
 }
 
+static inline void btrfs_free_excluded_extents(const struct btrfs_block_group *bg)
+{
+	clear_extent_bits(&bg->fs_info->excluded_extents, bg->start,
+			  bg->start + bg->length - 1, EXTENT_UPTODATE);
+}
+
 static noinline void caching_thread(struct btrfs_work *work)
 {
 	struct btrfs_block_group *block_group;
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 701fa08cffb6..b0fcc2a042ad 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -69,18 +69,6 @@  static int block_group_bits(struct btrfs_block_group *cache, u64 bits)
 	return (cache->flags & bits) == bits;
 }
 
-void btrfs_free_excluded_extents(struct btrfs_block_group *cache)
-{
-	struct btrfs_fs_info *fs_info = cache->fs_info;
-	u64 start, end;
-
-	start = cache->start;
-	end = start + cache->length - 1;
-
-	clear_extent_bits(&fs_info->excluded_extents, start, end,
-			  EXTENT_UPTODATE);
-}
-
 /* simple helper to search for an existing data extent at a given offset */
 int btrfs_lookup_data_extent(struct btrfs_fs_info *fs_info, u64 start, u64 len)
 {
diff --git a/fs/btrfs/extent-tree.h b/fs/btrfs/extent-tree.h
index 3b2f265f4653..b9e148adcd28 100644
--- a/fs/btrfs/extent-tree.h
+++ b/fs/btrfs/extent-tree.h
@@ -96,7 +96,6 @@  int btrfs_get_extent_inline_ref_type(const struct extent_buffer *eb,
 				     enum btrfs_inline_ref_type is_data);
 u64 hash_extent_data_ref(u64 root_objectid, u64 owner, u64 offset);
 
-void btrfs_free_excluded_extents(struct btrfs_block_group *cache);
 int btrfs_run_delayed_refs(struct btrfs_trans_handle *trans, unsigned long count);
 void btrfs_cleanup_ref_head_accounting(struct btrfs_fs_info *fs_info,
 				  struct btrfs_delayed_ref_root *delayed_refs,