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

On Tue, Jul 28, 2026 at 7:47 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 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;

Here it was an exclusive end.

>         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);

Here we guaranteed hole_end is not exclusive, by subtracting 1 from
the result of min(), and both arguments passed to min() are exclusive.

>
>                         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);

And here range_end was exclusive, we subtracted 1 to make it inclusive.

>                 if (ret < 0)
>                         goto out_unlock;
> -               prev_extent_end = range_end;
> +               prev_extent_end = range_end + 1;

Here range_end was exclusive so there was no need to sum 1.

In other words, this change doesn't fix anything; it's just rewriting
the calculations in a different way to reach the same results.

Where exactly do you think you saw a wrong end offset being used?

Thanks.

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