Re: [PATCH v2 1/3] btrfs: use aligned range for locking in extent_fiemap()

On Tue, Jul 28, 2026 at 10:41 AM Qu Wenruo <[email protected]> wrote:
>
>
>
> 在 2026/7/28 19:08, Filipe Manana 写道:
> > On Tue, Jul 28, 2026 at 10:23 AM 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 an exclusive end into
> >> btrfs_lock_extent(), which will step into the next block unexpectedly.
> >>
> >> Fix @range_end to be inclusive, so that btrfs_lock_extent() and
> >> btrfs_unlock_extent() will lock/unlock the correct aligned range.
> >>
> >> 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 the problem is passing an exclusive end to btrfs_lock_extent(),
> > then rather than doing all these changes to this variables, which is
> > error prone since they are used in several places and generally we use
> > exclusive ends everywhere, I'd prefer to simply change the call to
> > btrfs_lock_extent() and btrfs_unlock_extent() to receive 'range_end -
> > 1'. That also makes the fix much clearer.
>
> Thanks for the advice, indeed that is much safer.

After that you can add:

Reviewed-by: Filipe Manana <[email protected]>

Thanks.

>
> Thanks,
> Qu>
> > Thanks.
> >
> >>          }
> >>
> >>          if (cache.cached && cache.offset + cache.len >= last_extent_end) {
> >> --
> >> 2.54.0
> >>
> >>
>