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

在 2026/7/28 17:05, Filipe Manana 写道:
> 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?

btrfs_lock_extent(), which is inserting extent_state that is not aligned 
and will be caught by the last patch.

Thanks,
Qu

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