]> git.ipfire.org Git - thirdparty/kernel/stable.git/commitdiff
btrfs: handle csum lookup errors properly on reads
authorJosef Bacik <josef@toxicpanda.com>
Fri, 18 Feb 2022 15:03:23 +0000 (10:03 -0500)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Fri, 8 Apr 2022 12:06:36 +0000 (14:06 +0200)
[ Upstream commit 1784b7d502a94b561eae58249adde5f72c26eb3c ]

Currently any error we get while trying to lookup csums during reads
shows up as a missing csum, and then on the read completion side we
print an error saying there was a csum mismatch and we increase the
device corruption count.

However we could have gotten an EIO from the lookup.  We could also be
inside of a memory constrained container and gotten a ENOMEM while
trying to do the read.  In either case we don't want to make this look
like a file system corruption problem, we want to make it look like the
actual error it is.  Capture any negative value, convert it to the
appropriate blk_status_t, free the csum array if we have one and bail.

Note: a possible improvement would be to make the relocation code look
up the owning inode and see if it's marked as NODATASUM and set
EXTENT_NODATASUM there, that way if there's corruption and there isn't a
checksum when we want it we can fail here rather than later.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
fs/btrfs/file-item.c

index 91ae1caa1bdb5bb622c6e8c93f162c33b61f375b..a68d9cab12ead4ffe5588e16fef1cd498c40d56a 100644 (file)
@@ -366,6 +366,7 @@ blk_status_t btrfs_lookup_bio_sums(struct inode *inode, struct bio *bio, u8 *dst
 {
        struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
        struct extent_io_tree *io_tree = &BTRFS_I(inode)->io_tree;
+       struct btrfs_bio *bbio = NULL;
        struct btrfs_path *path;
        const u32 sectorsize = fs_info->sectorsize;
        const u32 csum_size = fs_info->csum_size;
@@ -375,6 +376,7 @@ blk_status_t btrfs_lookup_bio_sums(struct inode *inode, struct bio *bio, u8 *dst
        u8 *csum;
        const unsigned int nblocks = orig_len >> fs_info->sectorsize_bits;
        int count = 0;
+       blk_status_t ret = BLK_STS_OK;
 
        if (!fs_info->csum_root || (BTRFS_I(inode)->flags & BTRFS_INODE_NODATASUM))
                return BLK_STS_OK;
@@ -397,7 +399,7 @@ blk_status_t btrfs_lookup_bio_sums(struct inode *inode, struct bio *bio, u8 *dst
                return BLK_STS_RESOURCE;
 
        if (!dst) {
-               struct btrfs_bio *bbio = btrfs_bio(bio);
+               bbio = btrfs_bio(bio);
 
                if (nblocks * csum_size > BTRFS_BIO_INLINE_CSUM_SIZE) {
                        bbio->csum = kmalloc_array(nblocks, csum_size, GFP_NOFS);
@@ -453,21 +455,27 @@ blk_status_t btrfs_lookup_bio_sums(struct inode *inode, struct bio *bio, u8 *dst
 
                count = search_csum_tree(fs_info, path, cur_disk_bytenr,
                                         search_len, csum_dst);
-               if (count <= 0) {
-                       /*
-                        * Either we hit a critical error or we didn't find
-                        * the csum.
-                        * Either way, we put zero into the csums dst, and skip
-                        * to the next sector.
-                        */
+               if (count < 0) {
+                       ret = errno_to_blk_status(count);
+                       if (bbio)
+                               btrfs_bio_free_csum(bbio);
+                       break;
+               }
+
+               /*
+                * We didn't find a csum for this range.  We need to make sure
+                * we complain loudly about this, because we are not NODATASUM.
+                *
+                * However for the DATA_RELOC inode we could potentially be
+                * relocating data extents for a NODATASUM inode, so the inode
+                * itself won't be marked with NODATASUM, but the extent we're
+                * copying is in fact NODATASUM.  If we don't find a csum we
+                * assume this is the case.
+                */
+               if (count == 0) {
                        memset(csum_dst, 0, csum_size);
                        count = 1;
 
-                       /*
-                        * For data reloc inode, we need to mark the range
-                        * NODATASUM so that balance won't report false csum
-                        * error.
-                        */
                        if (BTRFS_I(inode)->root->root_key.objectid ==
                            BTRFS_DATA_RELOC_TREE_OBJECTID) {
                                u64 file_offset;
@@ -488,7 +496,7 @@ blk_status_t btrfs_lookup_bio_sums(struct inode *inode, struct bio *bio, u8 *dst
        }
 
        btrfs_free_path(path);
-       return BLK_STS_OK;
+       return ret;
 }
 
 int btrfs_lookup_csums_range(struct btrfs_root *root, u64 start, u64 end,