From: Qu Wenruo <wqu@suse.com>
To: linux-btrfs@vger.kernel.org
Cc: Anand Jain <anand.jain@oracle.com>, Qu Wenruo <wqu@suse.com>
Subject: [PATCH v7 04/17] btrfs: grab correct extent map for subpage compressed extent read
Date: Mon, 12 Jul 2021 16:30:14 +0800 [thread overview]
Message-ID: <20210712083027.212734-5-wqu@suse.com> (raw)
In-Reply-To: <20210712083027.212734-1-wqu@suse.com>
[BUG]
When subpage compressed read write support is enabled, btrfs/038 always
fail with EIO.
A simplified script can easily trigger the problem:
mkfs.btrfs -f -s 4k $dev
mount $dev $mnt -o compress=lzo
xfs_io -f -c "truncate 118811" $mnt/foo
xfs_io -c "pwrite -S 0x0d -b 39987 92267 39987" $mnt/foo > /dev/null
sync
btrfs subvolume snapshot -r $mnt $mnt/mysnap1
xfs_io -c "pwrite -S 0x3e -b 80000 200000 80000" $mnt/foo > /dev/null
sync
xfs_io -c "pwrite -S 0xdc -b 10000 250000 10000" $mnt/foo > /dev/null
xfs_io -c "pwrite -S 0xff -b 10000 300000 10000" $mnt/foo > /dev/null
sync
btrfs subvolume snapshot -r $mnt $mnt/mysnap2
cat $mnt/mysnap2/foo
# Above cat will fail due to EIO
[CAUSE]
The problem is in btrfs_submit_compressed_read().
When it tries to grab the extent map of the read range, it uses the
following call:
em = lookup_extent_mapping(em_tree,
page_offset(bio_first_page_all(bio)),
fs_info->sectorsize);
The problem is in the page_offset(bio_first_page_all(bio)) part.
The offending inode has the following file extent layout
item 10 key (257 EXTENT_DATA 131072) itemoff 15639 itemsize 53
generation 8 type 1 (regular)
extent data disk byte 13680640 nr 4096
extent data offset 0 nr 4096 ram 4096
extent compression 0 (none)
item 11 key (257 EXTENT_DATA 135168) itemoff 15586 itemsize 53
generation 8 type 1 (regular)
extent data disk byte 0 nr 0
item 12 key (257 EXTENT_DATA 196608) itemoff 15533 itemsize 53
generation 8 type 1 (regular)
extent data disk byte 13676544 nr 4096
extent data offset 0 nr 53248 ram 86016
extent compression 2 (lzo)
And the bio passed in has the following parameters:
page_offset(bio_first_page_all(bio)) = 131072
bio_first_bvec_all(bio)->bv_offset = 65536
If we use page_offset(bio_first_page_all(bio) without adding bv_offset,
we will get an extent map for file offset 131072, not 196608.
This means we read uncompressed data from disk, and later decompression
will definitely fail.
[FIX]
Take bv_offset into consideration when trying to grab an extent map.
And add an ASSERT() to ensure we're really getting a compressed extent.
Thankfully this won't affect anything but subpage, thus we wonly need to
ensure this patch get merged before we enabled basic subpage support.
Reviewed-by: Anand Jain <anand.jain@oracle.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
fs/btrfs/compression.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
index f17be6398d2c..22945189287b 100644
--- a/fs/btrfs/compression.c
+++ b/fs/btrfs/compression.c
@@ -682,6 +682,7 @@ blk_status_t btrfs_submit_compressed_read(struct inode *inode, struct bio *bio,
struct page *page;
struct bio *comp_bio;
u64 cur_disk_byte = bio->bi_iter.bi_sector << 9;
+ u64 file_offset;
u64 em_len;
u64 em_start;
struct extent_map *em;
@@ -691,15 +692,17 @@ blk_status_t btrfs_submit_compressed_read(struct inode *inode, struct bio *bio,
em_tree = &BTRFS_I(inode)->extent_tree;
+ file_offset = bio_first_bvec_all(bio)->bv_offset +
+ page_offset(bio_first_page_all(bio));
+
/* we need the actual starting offset of this extent in the file */
read_lock(&em_tree->lock);
- em = lookup_extent_mapping(em_tree,
- page_offset(bio_first_page_all(bio)),
- fs_info->sectorsize);
+ em = lookup_extent_mapping(em_tree, file_offset, fs_info->sectorsize);
read_unlock(&em_tree->lock);
if (!em)
return BLK_STS_IOERR;
+ ASSERT(em->compress_type != BTRFS_COMPRESS_NONE);
compressed_len = em->block_len;
cb = kmalloc(compressed_bio_size(fs_info, compressed_len), GFP_NOFS);
if (!cb)
--
2.32.0
next prev parent reply other threads:[~2021-07-12 8:34 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-07-12 8:30 [PATCH v7 00/17] btrfs: add data write support for subpage Qu Wenruo
2021-07-12 8:30 ` [PATCH v7 01/17] btrfs: properly reset @this_bio_flag in btrfs_do_readpage() to avoid inheriting old bio flags to next extent Qu Wenruo
2021-07-16 13:59 ` Anand Jain
2021-07-12 8:30 ` [PATCH v7 02/17] btrfs: fix NULL pointer dereference when reading two compressed extent inside the same page Qu Wenruo
2021-07-12 8:30 ` [PATCH v7 03/17] btrfs: disable compressed readahead for subpage Qu Wenruo
2021-07-12 8:30 ` Qu Wenruo [this message]
2021-07-12 8:30 ` [PATCH v7 05/17] btrfs: rework btrfs_decompress_buf2page() Qu Wenruo
2021-07-12 8:30 ` [PATCH v7 06/17] btrfs: rework lzo_decompress_bio() to make it subpage compatible Qu Wenruo
2021-07-12 8:30 ` [PATCH v7 07/17] btrfs: extract relocation page read and dirty part into its own function Qu Wenruo
2021-07-12 8:30 ` [PATCH v7 08/17] btrfs: make relocate_one_page() to handle subpage case Qu Wenruo
2021-07-12 8:30 ` [PATCH v7 09/17] btrfs: fix wild subpage writeback which does not have ordered extent Qu Wenruo
2021-07-12 8:30 ` [PATCH v7 10/17] btrfs: disable inline extent creation for subpage Qu Wenruo
2021-07-12 8:30 ` [PATCH v7 11/17] btrfs: allow submit_extent_page() to do bio split " Qu Wenruo
2021-07-12 8:30 ` [PATCH v7 12/17] btrfs: reject raid5/6 fs " Qu Wenruo
2021-07-13 0:39 ` Anand Jain
2021-07-12 8:30 ` [PATCH v7 13/17] btrfs: fix a crash caused by race between prepare_pages() and btrfs_releasepage() Qu Wenruo
2021-07-12 8:30 ` [PATCH v7 14/17] btrfs: fix a use-after-free bug in writeback subpage helper Qu Wenruo
2021-07-12 8:30 ` [PATCH v7 15/17] btrfs: fix a subpage false alert for relocating partial preallocated data extents Qu Wenruo
2021-07-12 8:30 ` [PATCH v7 16/17] btrfs: fix a subpage relocation data corruption Qu Wenruo
2021-07-12 8:30 ` [PATCH v7 17/17] btrfs: allow read-write for 4K sectorsize on 64K page size systems Qu Wenruo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210712083027.212734-5-wqu@suse.com \
--to=wqu@suse.com \
--cc=anand.jain@oracle.com \
--cc=linux-btrfs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox