Re: [PATCH 1/2] btrfs: use proper inclusive end in extent_fiemap()

On Tue, 28 Jul 2026 at 11:17, Daniel Vacek <[email protected]> wrote:
> On Tue, 28 Jul 2026 at 10:40, Qu Wenruo <[email protected]> wrote:
> > 在 2026/7/28 17:35, Daniel Vacek 写道:
> > > On Tue, 28 Jul 2026 at 08:46, Qu Wenruo <[email protected]> wrote:
> > >> The @end parameter for all extent io tree helpers is inclusive, but
> > >> the call site in extent_fiemap() is passing exclusive end.
> > >>
> > >> Fix it to follow the common pattern.
> > >>
> > >> Fixes: ac3c0d36a2a2 ("btrfs: make fiemap more efficient and accurate reporting extent sharedness")
> > >> Signed-off-by: Qu Wenruo <[email protected]>
> > >> ---
> > >>   fs/btrfs/fiemap.c | 8 ++++----
> > >>   1 file changed, 4 insertions(+), 4 deletions(-)
> > >>
> > >> diff --git a/fs/btrfs/fiemap.c b/fs/btrfs/fiemap.c
> > >> index ba6a360074c0..2f2ae0b76799 100644
> > >> --- a/fs/btrfs/fiemap.c
> > >> +++ b/fs/btrfs/fiemap.c
> > >> @@ -657,7 +657,7 @@ static int extent_fiemap(struct btrfs_inode *inode,
> > >>
> > >>   restart:
> > >>          range_start = round_down(start, sectorsize);
> > >> -       range_end = round_up(start + len, sectorsize);
> > >> +       range_end = round_up(start + len, sectorsize) - 1;
> > >>          prev_extent_end = range_start;
> > >>
> > >>          btrfs_lock_extent(&inode->io_tree, range_start, range_end, &cached_state);
> > >> @@ -710,7 +710,7 @@ static int extent_fiemap(struct btrfs_inode *inode,
> > >>
> > >>                  /* We have in implicit hole (NO_HOLES feature enabled). */
> > >>                  if (prev_extent_end < key.offset) {
> > >> -                       const u64 hole_end = min(key.offset, range_end) - 1;
> > >> +                       const u64 hole_end = min(key.offset - 1, range_end);
> > >>
> > >>                          ret = fiemap_process_hole(inode, fieinfo, &cache,
> > >>                                                    &delalloc_cached_state,
> > >> @@ -812,10 +812,10 @@ static int extent_fiemap(struct btrfs_inode *inode,
> > >>          if (!stopped && prev_extent_end < range_end) {
> > >>                  ret = fiemap_process_hole(inode, fieinfo, &cache,
> > >>                                            &delalloc_cached_state, backref_ctx,
> > >> -                                         0, 0, 0, prev_extent_end, range_end - 1);
> > >> +                                         0, 0, 0, prev_extent_end, range_end);
> > >>                  if (ret < 0)
> > >>                          goto out_unlock;
> > >> -               prev_extent_end = range_end;
> > >> +               prev_extent_end = range_end + 1;
> > >>          }
> > >>
> > >>          if (cache.cached && cache.offset + cache.len >= last_extent_end) {
> > >> --
> > >> 2.54.0
> > >
> > > I don't really get why you left out these:
> > >
> > > @@ -680,7 +680,7 @@ static int extent_fiemap(struct btrfs_inode *inode,
> > >                  goto check_eof_delalloc;
> > >          }
> > >
> > > -       while (prev_extent_end < range_end) {
> > > +       while (prev_extent_end <= range_end) {
> > >                  struct extent_buffer *leaf = path->nodes[0];
> > >                  struct btrfs_file_extent_item *ei;
> > >                  struct btrfs_key key;
> > > @@ -809,13 +809,13 @@ static int extent_fiemap(struct btrfs_inode *inode,
> > >          }
> > >
> > >   check_eof_delalloc:
> > > -       if (!stopped && prev_extent_end < range_end) {
> > > +       if (!stopped && prev_extent_end <= range_end) {
> > >                  ret = fiemap_process_hole(inode, fieinfo, &cache,
> > >                                            &delalloc_cached_state, backref_ctx,
> > >
> > > Am I missing something?
> >
> > Because we won't reach the == case at all.
> >
> > The start is always increased by a value aligned to sectorsize.
>
> I understand round_down() possibly decreases not increases. So if
> start and start + len - 1 fall into the same block you get the ==
> case. Am I wrong?

Of course I am, as it's not divided, just rounded. For some reason I
imagined these were block numbers not bytes. My bad.
Sorry about the noise.

> > For inclusive range_end, we either completely pass it by 1, or smaller
> > than it.
> >
> > So that "==" will never be hit.>
> > > --nX
> >