diff mbox series

[1/2] btrfs: set correct ram_bytes when splitting ordered extent

Message ID 8f23c433c686f87ae863071e1c16950c7b9ebd44.1713223082.git.wqu@suse.com (mailing list archive)
State New
Headers show
Series btrfs: fix btrfs_file_extent_item::ram_bytes of btrfs_split_ordered_extent() | expand

Commit Message

Qu Wenruo April 15, 2024, 11:24 p.m. UTC
[BUG]
When running generic/287, the following file extent items can be
generated:

        item 16 key (258 EXTENT_DATA 2682880) itemoff 15305 itemsize 53
                generation 9 type 1 (regular)
                extent data disk byte 1378414592 nr 462848
                extent data offset 0 nr 462848 ram 2097152
                extent compression 0 (none)

Note that file extent item is not a compressed one, but its ram_bytes is
way larger than its disk_num_bytes.

According to btrfs on-disk scheme, ram_bytes should match disk_num_bytes
if it's not a compressed one.

[CAUSE]
Since commit b73a6fd1b1ef ("btrfs: split partial dio bios before
submit"), for partial dio writes, we would split the ordered extent.

However the function btrfs_split_ordered_extent() doesn't update the
ram_bytes even it has already shrunk the disk_num_bytes.

Originally the function btrfs_split_ordered_extent() is only introduced
for zoned devices in commit d22002fd37bd ("btrfs: zoned: split ordered
extent when bio is sent"), but later commit b73a6fd1b1ef ("btrfs: split
partial dio bios before submit") makes non-zoned btrfs affected.

Thankfully for un-compressed file extent, we do not really utilize the
ram_bytes member, thus it won't cause any real problem.

[FIX]
Also update btrfs_ordered_extent::ram_bytes inside
btrfs_split_ordered_extent().

Fixes: b73a6fd1b1ef ("btrfs: split partial dio bios before submit")
Fixes: d22002fd37bd ("btrfs: zoned: split ordered extent when bio is sent")
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/ordered-data.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Filipe Manana April 16, 2024, 11:38 a.m. UTC | #1
On Tue, Apr 16, 2024 at 12:24 AM Qu Wenruo <wqu@suse.com> wrote:
>
> [BUG]
> When running generic/287, the following file extent items can be
> generated:
>
>         item 16 key (258 EXTENT_DATA 2682880) itemoff 15305 itemsize 53
>                 generation 9 type 1 (regular)
>                 extent data disk byte 1378414592 nr 462848
>                 extent data offset 0 nr 462848 ram 2097152
>                 extent compression 0 (none)
>
> Note that file extent item is not a compressed one, but its ram_bytes is
> way larger than its disk_num_bytes.
>
> According to btrfs on-disk scheme, ram_bytes should match disk_num_bytes
> if it's not a compressed one.
>
> [CAUSE]
> Since commit b73a6fd1b1ef ("btrfs: split partial dio bios before
> submit"), for partial dio writes, we would split the ordered extent.
>
> However the function btrfs_split_ordered_extent() doesn't update the
> ram_bytes even it has already shrunk the disk_num_bytes.
>
> Originally the function btrfs_split_ordered_extent() is only introduced
> for zoned devices in commit d22002fd37bd ("btrfs: zoned: split ordered
> extent when bio is sent"), but later commit b73a6fd1b1ef ("btrfs: split
> partial dio bios before submit") makes non-zoned btrfs affected.
>
> Thankfully for un-compressed file extent, we do not really utilize the
> ram_bytes member, thus it won't cause any real problem.
>
> [FIX]
> Also update btrfs_ordered_extent::ram_bytes inside
> btrfs_split_ordered_extent().
>
> Fixes: b73a6fd1b1ef ("btrfs: split partial dio bios before submit")
> Fixes: d22002fd37bd ("btrfs: zoned: split ordered extent when bio is sent")

It's confusing to have two commits here.

Given that the second one is the one that introduced the ordered
extent splitting function and that the first one just uses that
function, leaving only the second Fixes tag makes it clear and avoids
confusion. I.e. it's not the first commit that introduced the bug.

Otherwise it looks good, thanks.

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

> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>  fs/btrfs/ordered-data.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/fs/btrfs/ordered-data.c b/fs/btrfs/ordered-data.c
> index b749ba45da2b..c2a42bcde98e 100644
> --- a/fs/btrfs/ordered-data.c
> +++ b/fs/btrfs/ordered-data.c
> @@ -1188,6 +1188,7 @@ struct btrfs_ordered_extent *btrfs_split_ordered_extent(
>         ordered->disk_bytenr += len;
>         ordered->num_bytes -= len;
>         ordered->disk_num_bytes -= len;
> +       ordered->ram_bytes -= len;
>
>         if (test_bit(BTRFS_ORDERED_IO_DONE, &ordered->flags)) {
>                 ASSERT(ordered->bytes_left == 0);
> --
> 2.44.0
>
>
diff mbox series

Patch

diff --git a/fs/btrfs/ordered-data.c b/fs/btrfs/ordered-data.c
index b749ba45da2b..c2a42bcde98e 100644
--- a/fs/btrfs/ordered-data.c
+++ b/fs/btrfs/ordered-data.c
@@ -1188,6 +1188,7 @@  struct btrfs_ordered_extent *btrfs_split_ordered_extent(
 	ordered->disk_bytenr += len;
 	ordered->num_bytes -= len;
 	ordered->disk_num_bytes -= len;
+	ordered->ram_bytes -= len;
 
 	if (test_bit(BTRFS_ORDERED_IO_DONE, &ordered->flags)) {
 		ASSERT(ordered->bytes_left == 0);