| Message ID | 20191111084226.475957-1-Damenly_Su@gmx.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] btrfs-progs: add comments of block group lookup functions | expand |
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) >
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) >> >
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) >>> >>
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) >>>> >>> >
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.
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.
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)