| Message ID | 8f23c433c686f87ae863071e1c16950c7b9ebd44.1713223082.git.wqu@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: fix btrfs_file_extent_item::ram_bytes of btrfs_split_ordered_extent() | expand |
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 --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);
[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(+)