diff mbox series

btrfs: fix COW handling in run_delalloc_nocow function

Message ID 20250414074610.2475801-1-davechen@synology.com (mailing list archive)
State New
Headers show
Series btrfs: fix COW handling in run_delalloc_nocow function | expand

Commit Message

davechen April 14, 2025, 7:46 a.m. UTC
In run_delalloc_nocow(), when the found btrfs_key's offset > cur_offset,
it indicates a gap between he current processing region and
the next file extent. The original code would directly jump to
the "must_cow" label, which implicitly increments the slot and
forces a fallback to COW. This behavior might skip an extent item and
result in an overestimated COW fallback range.

This patch modifies the logic so that when a gap is detected:
  - If no COW range is already being recorded (cow_start is unset),
    cow_start is set to cur_offset.
  - cur_offset is then advanced to the beginning of the next
    extent (extent_end).
  - Instead of jumping to "must_cow", control flows directly to
    "next_slot" so that the same extent item can be reexamined properly.

The change ensures that we accurately account for the extent gap and
avoid accidentally extending the range that needs to fallback to COW.

Signed-off-by: Dave Chen <davechen@synology.com>
---
 fs/btrfs/inode.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Comments

Filipe Manana April 14, 2025, 10:07 a.m. UTC | #1
On Mon, Apr 14, 2025 at 8:54 AM davechen <davechen@synology.com> wrote:
>
> In run_delalloc_nocow(), when the found btrfs_key's offset > cur_offset,
> it indicates a gap between he current processing region and

he -> the

> the next file extent. The original code would directly jump to
> the "must_cow" label, which implicitly increments the slot and

Implicitly? It's very explicit, we have the expression "path->slots[0]++".
So just remove the "implicitly" word.

> forces a fallback to COW. This behavior might skip an extent item and
> result in an overestimated COW fallback range.
>
> This patch modifies the logic so that when a gap is detected:
>   - If no COW range is already being recorded (cow_start is unset),
>     cow_start is set to cur_offset.
>   - cur_offset is then advanced to the beginning of the next
>     extent (extent_end).
>   - Instead of jumping to "must_cow", control flows directly to
>     "next_slot" so that the same extent item can be reexamined properly.
>
> The change ensures that we accurately account for the extent gap and
> avoid accidentally extending the range that needs to fallback to COW.
>
> Signed-off-by: Dave Chen <davechen@synology.com>
> ---
>  fs/btrfs/inode.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 5b842276573e..58457bdf984d 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -2160,7 +2160,10 @@ static noinline int run_delalloc_nocow(struct btrfs_inode *inode,
>                 if (found_key.offset > cur_offset) {
>                         extent_end = found_key.offset;
>                         extent_type = 0;
> -                       goto must_cow;
> +                       if (cow_start == (u64)-1)
> +                               cow_start = cur_offset;
> +                       cur_offset = extent_end;
> +                       goto next_slot;

Now that we jump to "next_slot" and not "must_cow" anymore, we should also:

1) remove the line above that sets extent_type to 0, no longer needed;

2) no need for setting extent_end either, we can just remove it and
set cur_offset to found_key.offset.

Otherwise it looks good, and with those minor changes made:

Reviewed-by: Filipe Manana <fdmanana@suse.com>

Thanks.

>                 }
>
>                 /*
> --
> 2.43.0
>
>
diff mbox series

Patch

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 5b842276573e..58457bdf984d 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -2160,7 +2160,10 @@  static noinline int run_delalloc_nocow(struct btrfs_inode *inode,
 		if (found_key.offset > cur_offset) {
 			extent_end = found_key.offset;
 			extent_type = 0;
-			goto must_cow;
+			if (cow_start == (u64)-1)
+				cow_start = cur_offset;
+			cur_offset = extent_end;
+			goto next_slot;
 		}
 
 		/*