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

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.

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