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

[Qu Wenruo <wqu@suse.com>]

在 2026/7/28 18:47, Daniel Vacek 写道:
> On Tue, 28 Jul 2026 at 10:40, Qu Wenruo <wqu@suse.com> wrote:
>> 在 2026/7/28 17:35, Daniel Vacek 写道:
>>> On Tue, 28 Jul 2026 at 08:46, Qu Wenruo <wqu@suse.com> 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 <wqu@suse.com>
>>>> ---
>>>>    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?

The only case when that happen is when @len is 0, which is already 
rejected by fiemap_prep()

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