[PATCH 0/2] btrfs: add basic alignment checks for extent_state

[PATCH 0/2] btrfs: add basic alignment checks for extent_state

[Qu Wenruo]

Commit 3f255ece2f1e ("btrfs: introduce extra sanity checks for extent
maps") introduced sanity checks for extent maps, but for extent state we
do not have any extra sanity checks.

This series will introduce the check in the 2nd patch, meanwhile the
first one is to fix an exposed unaligned access in extent_fiemap().

Qu Wenruo (2):
  btrfs: use proper inclusive end in extent_fiemap()
  btrfs: add validation for extent states

 fs/btrfs/extent-io-tree.c | 21 +++++++++++++++++++++
 fs/btrfs/fiemap.c         |  8 ++++----
 2 files changed, 25 insertions(+), 4 deletions(-)

-- 
2.54.0

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

[Qu Wenruo]

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

[PATCH 2/2] btrfs: add validation for extent states

[Qu Wenruo]

Extent maps have the extra validation since commit 3f255ece2f1e ("btrfs:
introduce extra sanity checks for extent maps"), but extent states do
not have a similar check.

Introduce a basic alignment check for the following call sites, so that
we can cover all extent maps inserted into the tree:

- insert_state_fast()
- insert_state()
- split_state()

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/extent-io-tree.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/fs/btrfs/extent-io-tree.c b/fs/btrfs/extent-io-tree.c
index c18ea5ef2974..4f1ef33f17ac 100644
--- a/fs/btrfs/extent-io-tree.c
+++ b/fs/btrfs/extent-io-tree.c
@@ -403,6 +403,21 @@ static void set_state_bits(struct extent_io_tree *tree,
 	state->state |= bits_to_set;
 }
 
+static void validate_extent_state(const struct extent_io_tree *tree,
+				  struct extent_state *state)
+{
+	u32 blocksize;
+
+	if (tree->owner != IO_TREE_INODE_IO)
+		return;
+
+	blocksize = btrfs_extent_io_tree_to_fs_info(tree)->sectorsize;
+	ASSERT(IS_ALIGNED(state->start, blocksize) &&
+	       IS_ALIGNED(state->end + 1, blocksize),
+	       "unaligned extent state, blocksize=%u start=%llu end=%llu state=0x%x",
+	       blocksize, state->start, state->end, state->state);
+}
+
 /*
  * Insert an extent_state struct into the tree.  'bits' are set on the
  * struct before it is inserted.
@@ -429,6 +444,8 @@ static struct extent_state *insert_state(struct extent_io_tree *tree,
 	const u64 end = state->end + 1;
 	const bool try_merge = !(bits & (EXTENT_LOCK_BITS | EXTENT_BOUNDARY));
 
+	validate_extent_state(tree, state);
+
 	set_state_bits(tree, state, bits, changeset);
 
 	node = &tree->state.rb_node;
@@ -481,6 +498,8 @@ static void insert_state_fast(struct extent_io_tree *tree,
 			      struct rb_node *parent, unsigned bits,
 			      struct extent_changeset *changeset)
 {
+	validate_extent_state(tree, state);
+
 	set_state_bits(tree, state, bits, changeset);
 	rb_link_node(&state->rb_node, parent, node);
 	rb_insert_color(&state->rb_node, &tree->state);
@@ -533,6 +552,8 @@ static int split_state(struct extent_io_tree *tree, struct extent_state *orig,
 		}
 	}
 
+	validate_extent_state(tree, orig);
+	validate_extent_state(tree, prealloc);
 	rb_link_node(&prealloc->rb_node, parent, node);
 	rb_insert_color(&prealloc->rb_node, &tree->state);
 
-- 
2.54.0

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

[Filipe Manana]

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

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

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

[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?

--nX

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

[Qu Wenruo]

在 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.

For inclusive range_end, we either completely pass it by 1, or smaller 
than it.

So that "==" will never be hit.>
> --nX

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

[Qu Wenruo]

在 2026/7/28 17:05, Filipe Manana 写道:
> On Tue, Jul 28, 2026 at 7:47 AM 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;
> 
> 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
>>
>>

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

[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?

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

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

[Qu Wenruo]

在 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
>>

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

[Daniel Vacek]

On Tue, 28 Jul 2026 at 11:17, Daniel Vacek <neelx@suse.com> wrote:
> 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?

Of course I am, as it's not divided, just rounded. For some reason I
imagined these were block numbers not bytes. My bad.
Sorry about the noise.

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