All of lore.kernel.org
 help / color / mirror / Atom feed
From: Qu Wenruo <wqu@suse.com>
To: linux-btrfs@vger.kernel.org
Subject: [PATCH v6 01/15] btrfs: grab correct extent map for subpage compressed extent read
Date: Mon,  5 Jul 2021 10:00:56 +0800	[thread overview]
Message-ID: <20210705020110.89358-2-wqu@suse.com> (raw)
In-Reply-To: <20210705020110.89358-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.

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 9a023ae0f98b..19da933c5f1c 100644
--- a/fs/btrfs/compression.c
+++ b/fs/btrfs/compression.c
@@ -673,6 +673,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;
@@ -682,15 +683,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


  reply	other threads:[~2021-07-05  2:01 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-05  2:00 [PATCH v6 00/15] btrfs: add data write support for subpage Qu Wenruo
2021-07-05  2:00 ` Qu Wenruo [this message]
2021-07-08  6:50   ` [PATCH v6 01/15] btrfs: grab correct extent map for subpage compressed extent read Anand Jain
2021-07-08  7:06     ` Qu Wenruo
2021-07-09  9:13       ` Anand Jain
2021-07-05  2:00 ` [PATCH v6 02/15] btrfs: remove the GFP_HIGHMEM flag for compression code Qu Wenruo
2021-07-08 11:54   ` David Sterba
2021-07-08 12:11     ` Qu Wenruo
2021-07-05  2:00 ` [PATCH v6 03/15] btrfs: rework btrfs_decompress_buf2page() Qu Wenruo
2021-07-09 18:53   ` David Sterba
2021-07-09 22:03     ` Qu Wenruo
2021-07-09 19:26   ` David Sterba
2021-07-05  2:00 ` [PATCH v6 04/15] btrfs: rework lzo_decompress_bio() to make it subpage compatible Qu Wenruo
2021-07-09 20:37   ` David Sterba
2021-07-05  2:01 ` [PATCH v6 05/15] btrfs: extract relocation page read and dirty part into its own function Qu Wenruo
2021-07-05  2:01 ` [PATCH v6 06/15] btrfs: make relocate_one_page() to handle subpage case Qu Wenruo
2021-07-05  2:01 ` [PATCH v6 07/15] btrfs: fix wild subpage writeback which does not have ordered extent Qu Wenruo
2021-07-05  2:01 ` [PATCH v6 08/15] btrfs: disable inline extent creation for subpage Qu Wenruo
2021-07-05  2:01 ` [PATCH v6 09/15] btrfs: allow submit_extent_page() to do bio split " Qu Wenruo
2021-07-05  2:01 ` [PATCH v6 10/15] btrfs: reject raid5/6 fs " Qu Wenruo
2021-07-09  9:36   ` Anand Jain
2021-07-09 18:34     ` David Sterba
2021-07-05  2:01 ` [PATCH v6 11/15] btrfs: fix a crash caused by race between prepare_pages() and btrfs_releasepage() Qu Wenruo
2021-07-05  2:01 ` [PATCH v6 12/15] btrfs: fix a use-after-free bug in writeback subpage helper Qu Wenruo
2021-07-05  2:01 ` [PATCH v6 13/15] btrfs: fix a subpage false alert for relocating partial preallocated data extents Qu Wenruo
2021-07-05  2:01 ` [PATCH v6 14/15] btrfs: fix a subpage relocation data corruption Qu Wenruo
2021-07-05  2:01 ` [PATCH v6 15/15] btrfs: allow read-write for 4K sectorsize on 64K page size systems Qu Wenruo
2021-07-07  8:28 ` [PATCH v6 00/15] btrfs: add data write support for subpage Qu Wenruo
2021-07-07 17:41   ` Neal Gompa
2021-07-07 18:14     ` David Sterba
2021-07-07 23:19       ` Qu Wenruo
2021-07-08 11:27         ` David Sterba

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=20210705020110.89358-2-wqu@suse.com \
    --to=wqu@suse.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.