Re: [PATCH 1/2] btrfs: use proper inclusive end in extent_fiemap()
Qu Wenruo <[email protected]> Tue, 28 Jul 2026 18:10:41 +0930
| Newsgroups | org.kernel.vger.linux-btrfs |
|---|---|
| Message-ID | <[email protected]> |
在 2026/7/28 17:35, Daniel Vacek 写道: > On Tue, 28 Jul 2026 at 08:46, 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; >> 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. For inclusive range_end, we either completely pass it by 1, or smaller than it. So that "==" will never be hit.> > --nX