[1/2] btrfs-progs: add comments of block group lookup functions
diff mbox series

Message ID 20191111084226.475957-1-Damenly_Su@gmx.com
State New
Headers show
Series
  • [1/2] btrfs-progs: add comments of block group lookup functions
Related show

Commit Message

Damenly Su Nov. 11, 2019, 8:42 a.m. UTC
From: Su Yue <Damenly_Su@gmx.com>

The progs side function btrfs_lookup_first_block_group() calls
find_first_extent_bit() to find block group which contains bytenr
or after the bytenr. This behavior differs from kernel code, so
add the comments.

Add the coments of btrfs_lookup_block_group() too, this one works
like kernel side.

Signed-off-by: Su Yue <Damenly_Su@gmx.com>
---
 extent-tree.c | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Qu Wenruo Nov. 11, 2019, 9:28 a.m. UTC | #1
On 2019/11/11 下午4:42, damenly.su@gmail.com wrote:
> From: Su Yue <Damenly_Su@gmx.com>
> 
> The progs side function btrfs_lookup_first_block_group() calls
> find_first_extent_bit() to find block group which contains bytenr
> or after the bytenr. This behavior differs from kernel code, so
> add the comments.
> 
> Add the coments of btrfs_lookup_block_group() too, this one works
> like kernel side.
> 
> Signed-off-by: Su Yue <Damenly_Su@gmx.com>
> ---
>  extent-tree.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/extent-tree.c b/extent-tree.c
> index d67e4098351f..f690ae999f37 100644
> --- a/extent-tree.c
> +++ b/extent-tree.c
> @@ -164,6 +164,9 @@ err:
>  	return 0;
>  }
>  
> +/*
> + * Return the block group that contains or after bytenr
> + */

What about "Return the block group thart starts at or after @bytenr" ?

Thanks,
Qu

>  struct btrfs_block_group_cache *btrfs_lookup_first_block_group(struct
>  						       btrfs_fs_info *info,
>  						       u64 bytenr)
> @@ -193,6 +196,9 @@ struct btrfs_block_group_cache *btrfs_lookup_first_block_group(struct
>  	return block_group;
>  }
>  
> +/*
> + * Return the block group that contains the given bytenr
> + */
>  struct btrfs_block_group_cache *btrfs_lookup_block_group(struct
>  							 btrfs_fs_info *info,
>  							 u64 bytenr)
>
Su Yue Nov. 11, 2019, 9:49 a.m. UTC | #2
On 2019/11/11 5:28 PM, Qu Wenruo wrote:
>
>
> On 2019/11/11 下午4:42, damenly.su@gmail.com wrote:
>> From: Su Yue <Damenly_Su@gmx.com>
>>
>> The progs side function btrfs_lookup_first_block_group() calls
>> find_first_extent_bit() to find block group which contains bytenr
>> or after the bytenr. This behavior differs from kernel code, so
>> add the comments.
>>
>> Add the coments of btrfs_lookup_block_group() too, this one works
>> like kernel side.
>>
>> Signed-off-by: Su Yue <Damenly_Su@gmx.com>
>> ---
>>   extent-tree.c | 6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/extent-tree.c b/extent-tree.c
>> index d67e4098351f..f690ae999f37 100644
>> --- a/extent-tree.c
>> +++ b/extent-tree.c
>> @@ -164,6 +164,9 @@ err:
>>   	return 0;
>>   }
>>
>> +/*
>> + * Return the block group that contains or after bytenr
>> + */
>
> What about "Return the block group thart starts at or after @bytenr" ?
>

That's what documented in kernel already.
The thing I try to express is "contains".
For such a block group marked as B[n, m).
btrfs_lookup_first_block_group(x) (x > n && x < m). Kernel code will
return the block group next to B. However, progs side will return the
block group B.

Thanks.
> Thanks,
> Qu
>
>>   struct btrfs_block_group_cache *btrfs_lookup_first_block_group(struct
>>   						       btrfs_fs_info *info,
>>   						       u64 bytenr)
>> @@ -193,6 +196,9 @@ struct btrfs_block_group_cache *btrfs_lookup_first_block_group(struct
>>   	return block_group;
>>   }
>>
>> +/*
>> + * Return the block group that contains the given bytenr
>> + */
>>   struct btrfs_block_group_cache *btrfs_lookup_block_group(struct
>>   							 btrfs_fs_info *info,
>>   							 u64 bytenr)
>>
>
Qu Wenruo Nov. 11, 2019, 10:03 a.m. UTC | #3
On 2019/11/11 下午5:49, Su Yue wrote:
> 
> 
> On 2019/11/11 5:28 PM, Qu Wenruo wrote:
>>
>>
>> On 2019/11/11 下午4:42, damenly.su@gmail.com wrote:
>>> From: Su Yue <Damenly_Su@gmx.com>
>>>
>>> The progs side function btrfs_lookup_first_block_group() calls
>>> find_first_extent_bit() to find block group which contains bytenr
>>> or after the bytenr. This behavior differs from kernel code, so
>>> add the comments.
>>>
>>> Add the coments of btrfs_lookup_block_group() too, this one works
>>> like kernel side.
>>>
>>> Signed-off-by: Su Yue <Damenly_Su@gmx.com>
>>> ---
>>>   extent-tree.c | 6 ++++++
>>>   1 file changed, 6 insertions(+)
>>>
>>> diff --git a/extent-tree.c b/extent-tree.c
>>> index d67e4098351f..f690ae999f37 100644
>>> --- a/extent-tree.c
>>> +++ b/extent-tree.c
>>> @@ -164,6 +164,9 @@ err:
>>>       return 0;
>>>   }
>>>
>>> +/*
>>> + * Return the block group that contains or after bytenr
>>> + */
>>
>> What about "Return the block group thart starts at or after @bytenr" ?
>>
> 
> That's what documented in kernel already.
> The thing I try to express is "contains".
> For such a block group marked as B[n, m).
> btrfs_lookup_first_block_group(x) (x > n && x < m). Kernel code will
> return the block group next to B. However, progs side will return the
> block group B.

"Contains" indeed covers your example.
But the "after @bytenr" part looks strange to me.

Did you mean if @bytenr is not covered by any block group, then the next
block group starts after @bytenr is returned?

Then the comment is good.

Thanks,
Qu

> 
> Thanks.
>> Thanks,
>> Qu
>>
>>>   struct btrfs_block_group_cache *btrfs_lookup_first_block_group(struct
>>>                                  btrfs_fs_info *info,
>>>                                  u64 bytenr)
>>> @@ -193,6 +196,9 @@ struct btrfs_block_group_cache
>>> *btrfs_lookup_first_block_group(struct
>>>       return block_group;
>>>   }
>>>
>>> +/*
>>> + * Return the block group that contains the given bytenr
>>> + */
>>>   struct btrfs_block_group_cache *btrfs_lookup_block_group(struct
>>>                                btrfs_fs_info *info,
>>>                                u64 bytenr)
>>>
>>
Su Yue Nov. 11, 2019, 10:17 a.m. UTC | #4
On 2019/11/11 6:03 PM, Qu Wenruo wrote:
>
>
> On 2019/11/11 下午5:49, Su Yue wrote:
>>
>>
>> On 2019/11/11 5:28 PM, Qu Wenruo wrote:
>>>
>>>
>>> On 2019/11/11 下午4:42, damenly.su@gmail.com wrote:
>>>> From: Su Yue <Damenly_Su@gmx.com>
>>>>
>>>> The progs side function btrfs_lookup_first_block_group() calls
>>>> find_first_extent_bit() to find block group which contains bytenr
>>>> or after the bytenr. This behavior differs from kernel code, so
>>>> add the comments.
>>>>
>>>> Add the coments of btrfs_lookup_block_group() too, this one works
>>>> like kernel side.
>>>>
>>>> Signed-off-by: Su Yue <Damenly_Su@gmx.com>
>>>> ---
>>>>    extent-tree.c | 6 ++++++
>>>>    1 file changed, 6 insertions(+)
>>>>
>>>> diff --git a/extent-tree.c b/extent-tree.c
>>>> index d67e4098351f..f690ae999f37 100644
>>>> --- a/extent-tree.c
>>>> +++ b/extent-tree.c
>>>> @@ -164,6 +164,9 @@ err:
>>>>        return 0;
>>>>    }
>>>>
>>>> +/*
>>>> + * Return the block group that contains or after bytenr
>>>> + */
>>>
>>> What about "Return the block group thart starts at or after @bytenr" ?
>>>
>>
>> That's what documented in kernel already.
>> The thing I try to express is "contains".
>> For such a block group marked as B[n, m).
>> btrfs_lookup_first_block_group(x) (x > n && x < m). Kernel code will
>> return the block group next to B. However, progs side will return the
>> block group B.
>
> "Contains" indeed covers your example.
> But the "after @bytenr" part looks strange to me.
>
> Did you mean if @bytenr is not covered by any block group, then the next
> block group starts after @bytenr is returned?
>
Yes, that's precisely what I mean.

Thanks.
> Then the comment is good.
>
> Thanks,
> Qu
>
>>
>> Thanks.
>>> Thanks,
>>> Qu
>>>
>>>>    struct btrfs_block_group_cache *btrfs_lookup_first_block_group(struct
>>>>                                   btrfs_fs_info *info,
>>>>                                   u64 bytenr)
>>>> @@ -193,6 +196,9 @@ struct btrfs_block_group_cache
>>>> *btrfs_lookup_first_block_group(struct
>>>>        return block_group;
>>>>    }
>>>>
>>>> +/*
>>>> + * Return the block group that contains the given bytenr
>>>> + */
>>>>    struct btrfs_block_group_cache *btrfs_lookup_block_group(struct
>>>>                                 btrfs_fs_info *info,
>>>>                                 u64 bytenr)
>>>>
>>>
>
David Sterba Nov. 15, 2019, 4:17 p.m. UTC | #5
On Mon, Nov 11, 2019 at 06:17:17PM +0800, Su Yue wrote:
> >>>> + * Return the block group that contains or after bytenr
> >>>> + */
> >>>
> >>> What about "Return the block group thart starts at or after @bytenr" ?
> >>>
> >>
> >> That's what documented in kernel already.
> >> The thing I try to express is "contains".
> >> For such a block group marked as B[n, m).
> >> btrfs_lookup_first_block_group(x) (x > n && x < m). Kernel code will
> >> return the block group next to B. However, progs side will return the
> >> block group B.
> >
> > "Contains" indeed covers your example.
> > But the "after @bytenr" part looks strange to me.
> >
> > Did you mean if @bytenr is not covered by any block group, then the next
> > block group starts after @bytenr is returned?
> >
> Yes, that's precisely what I mean.

I'll use the wording as suggested by Qu. Thanks.
David Sterba Nov. 15, 2019, 4:20 p.m. UTC | #6
On Mon, Nov 11, 2019 at 04:42:25PM +0800, damenly.su@gmail.com wrote:
> From: Su Yue <Damenly_Su@gmx.com>
> 
> The progs side function btrfs_lookup_first_block_group() calls
> find_first_extent_bit() to find block group which contains bytenr
> or after the bytenr. This behavior differs from kernel code, so
> add the comments.
> 
> Add the coments of btrfs_lookup_block_group() too, this one works
> like kernel side.
> 
> Signed-off-by: Su Yue <Damenly_Su@gmx.com>

1 and 2 added to devel, thanks.

Patch
diff mbox series

diff --git a/extent-tree.c b/extent-tree.c
index d67e4098351f..f690ae999f37 100644
--- a/extent-tree.c
+++ b/extent-tree.c
@@ -164,6 +164,9 @@  err:
 	return 0;
 }
 
+/*
+ * Return the block group that contains or after bytenr
+ */
 struct btrfs_block_group_cache *btrfs_lookup_first_block_group(struct
 						       btrfs_fs_info *info,
 						       u64 bytenr)
@@ -193,6 +196,9 @@  struct btrfs_block_group_cache *btrfs_lookup_first_block_group(struct
 	return block_group;
 }
 
+/*
+ * Return the block group that contains the given bytenr
+ */
 struct btrfs_block_group_cache *btrfs_lookup_block_group(struct
 							 btrfs_fs_info *info,
 							 u64 bytenr)