| Message ID | 0bd7715ef04abbfca4b97137b5b197333f2eb227.1719291793.git.wqu@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: detect and fix the ram_bytes mismatch | expand |
On Tue, Jun 25, 2024 at 6:08 AM Qu Wenruo <wqu@suse.com> wrote: > > [BUG] > After adding extra checks on btrfs_file_extent_item::ram_bytes to > tree-checker, running fsstress with multiple threads can lead to It's irrelevant to mention multiple threads, that's not necessary to cause the problem. > tree-checker warning at write time, as we created file extent items with > ram_bytes. This last part of the sentence makes no sense "we created file extent items with ram_bytes" - they must always have ram_bytes. I think you meant to say "with an invalid ram_bytes value". > > All those offending file extents have offset 0, and ram_bytes matching > num_bytes, and smaller than disk_num_bytes. > > This would also trigger the recently enhanced btrfs-check, which would > catch such mismatch and report them as minor errors. mismatch -> mismatches > > [CAUSE] > When a folio/page is invalidated and it is part of a submitted OE, we > mark the OE truncated just to the beginning of the folio/page. > > And for truncated OE, we insert the file extent item with incorrect > value for ram_bytes (using num_bytes instead of the usual value). > > This is not a big deal for end users, as we do not utilize the ram_bytes > field for regular non-compressed extents. > This mismatch is just a small violation against on-disk format. > > [FIX] > Fix it by removing the override on btrfs_file_extent_item::ram_bytes. > > Signed-off-by: Qu Wenruo <wqu@suse.com> > --- > fs/btrfs/inode.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c > index d6c43120c5d3..45f77ae8963f 100644 > --- a/fs/btrfs/inode.c > +++ b/fs/btrfs/inode.c > @@ -3018,10 +3018,8 @@ static int insert_ordered_extent_file_extent(struct btrfs_trans_handle *trans, > btrfs_set_stack_file_extent_disk_num_bytes(&stack_fi, > oe->disk_num_bytes); > btrfs_set_stack_file_extent_offset(&stack_fi, oe->offset); > - if (test_bit(BTRFS_ORDERED_TRUNCATED, &oe->flags)) { > + if (test_bit(BTRFS_ORDERED_TRUNCATED, &oe->flags)) > num_bytes = oe->truncated_len; > - ram_bytes = num_bytes; > - } The code looks good, with those updates to the the changelog: Reviewed-by: Filipe Manana <fdmanana@suse.com> Thanks. > btrfs_set_stack_file_extent_num_bytes(&stack_fi, num_bytes); > btrfs_set_stack_file_extent_ram_bytes(&stack_fi, ram_bytes); > btrfs_set_stack_file_extent_compression(&stack_fi, oe->compress_type); > -- > 2.45.2 > >
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c index d6c43120c5d3..45f77ae8963f 100644 --- a/fs/btrfs/inode.c +++ b/fs/btrfs/inode.c @@ -3018,10 +3018,8 @@ static int insert_ordered_extent_file_extent(struct btrfs_trans_handle *trans, btrfs_set_stack_file_extent_disk_num_bytes(&stack_fi, oe->disk_num_bytes); btrfs_set_stack_file_extent_offset(&stack_fi, oe->offset); - if (test_bit(BTRFS_ORDERED_TRUNCATED, &oe->flags)) { + if (test_bit(BTRFS_ORDERED_TRUNCATED, &oe->flags)) num_bytes = oe->truncated_len; - ram_bytes = num_bytes; - } btrfs_set_stack_file_extent_num_bytes(&stack_fi, num_bytes); btrfs_set_stack_file_extent_ram_bytes(&stack_fi, ram_bytes); btrfs_set_stack_file_extent_compression(&stack_fi, oe->compress_type);
[BUG] After adding extra checks on btrfs_file_extent_item::ram_bytes to tree-checker, running fsstress with multiple threads can lead to tree-checker warning at write time, as we created file extent items with ram_bytes. All those offending file extents have offset 0, and ram_bytes matching num_bytes, and smaller than disk_num_bytes. This would also trigger the recently enhanced btrfs-check, which would catch such mismatch and report them as minor errors. [CAUSE] When a folio/page is invalidated and it is part of a submitted OE, we mark the OE truncated just to the beginning of the folio/page. And for truncated OE, we insert the file extent item with incorrect value for ram_bytes (using num_bytes instead of the usual value). This is not a big deal for end users, as we do not utilize the ram_bytes field for regular non-compressed extents. This mismatch is just a small violation against on-disk format. [FIX] Fix it by removing the override on btrfs_file_extent_item::ram_bytes. Signed-off-by: Qu Wenruo <wqu@suse.com> --- fs/btrfs/inode.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)